From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752542AbbHNSqm (ORCPT ); Fri, 14 Aug 2015 14:46:42 -0400 Received: from mail-ob0-f178.google.com ([209.85.214.178]:36723 "EHLO mail-ob0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750808AbbHNSql (ORCPT ); Fri, 14 Aug 2015 14:46:41 -0400 MIME-Version: 1.0 In-Reply-To: References: <20150814071500.GA2678@gmail.com> From: Andy Lutomirski Date: Fri, 14 Aug 2015 11:46:20 -0700 Message-ID: Subject: Re: [GIT PULL] x86 fixes To: Linus Torvalds Cc: Ingo Molnar , Juergen Gross , Andy Lutomirski , Linux Kernel Mailing List , Thomas Gleixner , "H. Peter Anvin" , Peter Zijlstra , Brian Gerst , Denys Vlasenko , Andrew Morton Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 14, 2015 at 11:25 AM, Linus Torvalds wrote: > On Fri, Aug 14, 2015 at 12:15 AM, Ingo Molnar wrote: >> >> Please pull the latest x86-urgent-for-linus git tree from: > > Nope. It seems to be an unmitigated disaster, as far as I can tell. > >> +static inline struct desc_struct FPU_get_ldt_descriptor(unsigned seg) >> +{ >> + static struct desc_struct zero_desc; >> + struct desc_struct ret = zero_desc; >> + >> +#ifdef CONFIG_MODIFY_LDT_SYSCALL >> + seg >>= 3; > > So this seems to take the actual segment selector, and look it up in > the LDT. (Why only the LDT?) > Beats the crap out of me. Presumably it's because no one ever cared what happened if the selectors were in the GDT. This makes me suspect that no one has ever tested math emulation in combination with set_thread_area. > However: > >> + descriptor = FPU_get_ldt_descriptor(segment); > > as far as I can tell, the "segment" here is the segment _register_ > number, not the segment selector. > > The segment selector is in "addr->selector", and furthermore I'm not > at all convinced that it is in the LDT to begin with. I'd expect the > common case to be that it's in the GDT, in fact. But what do I know.. > > Anyway, I may be embarrassingly wrong, and if I am, feel free to shout > bad words at me, but that code seems to be utter crap. > > Not that the old code was particularly good either, but at least that > PM_REG_(segment) that *used* to exist there would translate segment > register numbers into actual selector values (even if it would get the > FS case wrong). Oh, crap. You're right, and I'm embarrassed that I missed that. > > Now that said, I doubt anybody cares. Since we don't support the > original 80386, the only way to ever trigger FP emulation is by having > a 486SX or possibly a couple of even rarer clone chips. So it's not > like the fact that the code is completely wrong and crap actually > *matters*, but I still refuse to pull stuff that seems to be so > completely screwed up. > > And this commit is even marked as "reviewed". Are you guys really > seeing something that I'm not? Am I somehow wrong in thinking it's > entirely broken? I think it's only slightly broken. This bit: if ((FPU_CS & 4) != 4) { /* Must be in the LDT */ /* Can only handle segmented addressing via the LDT for now, and it must be 16 bit */ printk("FPU emulator: Unsupported addressing mode\n"); math_abort(FPU_info, SIGILL); } code_descriptor = FPU_get_ldt_descriptor(FPU_CS); is buggy, but no buggier than the old code. FPU_CS is thing: #define FPU_CS (*(unsigned short *) &(FPU_info->regs->cs)) so it's only the pm_address thing that's broken. Sorry :( --Andy