From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754076AbbHFDfW (ORCPT ); Wed, 5 Aug 2015 23:35:22 -0400 Received: from mx2.suse.de ([195.135.220.15]:56379 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751867AbbHFDfV (ORCPT ); Wed, 5 Aug 2015 23:35:21 -0400 Subject: Re: [PATCH] x86: correct fpu emulation access to ldt To: Andy Lutomirski References: <1438700560-1778-1-git-send-email-jgross@suse.com> <55C1D33F.20803@suse.com> Cc: billm@melbpc.org.au, "linux-kernel@vger.kernel.org" From: Juergen Gross Message-ID: <55C2D5F5.4010809@suse.com> Date: Thu, 6 Aug 2015 05:35:17 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/05/2015 08:24 PM, Andy Lutomirski wrote: > On Wed, Aug 5, 2015 at 2:11 AM, Juergen Gross wrote: >> On 08/04/2015 08:01 PM, Andy Lutomirski wrote: >>> >>> On Tue, Aug 4, 2015 at 8:02 AM, Juergen Gross wrote: >>>> >>>> Commit 14805442532c ("x86/ldt: Make modify_ldt synchronous") introduced >>>> a new struct ldt_struct anchored at mm->context.ldt. >>>> >>>> Adapt the x86 fpu emulation to use that new structure. >>>> >>>> Signed-off-by: Juergen Gross >>> >>> >>> Whoops! >>> >>> Does this need to Cc: stable? >> >> >> Probably. >> >>> Also, want to make it slightly fancier so we can drop the dependency >>> on CONFIG_MODIFY_LDT_SYSCALL? >> >> >> Something like: >> >> -#define LDT_DESCRIPTOR(s) (((struct desc_struct >> *)current->mm->context.ldt)[(s) >> 3]) >> +#ifdef CONFIG_MODIFY_LDT_SYSCALL >> +#define LDT_DESCRIPTOR(s) (current->mm->context.ldt->entries[(s) >> 3]) >> +#else >> +#define LDT_DESCRIPTOR(s) ((struct desc_struct){{{ .a = 0, .b = 0, }}}) > > Careful! I think that akpm uses some ancient gcc version that can't > compile that. Maybe have a global empty segment somewhere that this > returns, or just ifdef out the two call sites. > > Also, I don't believe this for a second: > > /* s is always from a cpu register, and the cpu does bounds checking > * during register load --> no further bounds checks needed */ > #define LDT_DESCRIPTOR(s) (((struct desc_struct > *)current->mm->context.ldt)[(s) >> 3]) > > "What the comment means is that s always came from a cpu register at > some point in the recent past (assuming that no lazy segment save > logic is in effect) and we cross our fingers and hope that we never > end up accessing out of bounds if the LDT isn't the same as it was at > the time of the fault we're handling." > > Sigh. > > Maybe the best approach would be to replace LDT_DESCRIPTOR with an > actual function that returns a struct desc_struct. If it's out of > bounds or !CONFIG_MODIFY_LDT_SYSCALL, return zeros. Otherwise return > the descriptor. Yeah, seems to be the better approach. > >> +#endif >> >> I'd need to specify the corresponding patch as a prerequisite for stable >> I guess? How to do this before it is picked by Linus? > > Send a v2 with Cc: stable@vger.kernel.org # [commit hash you depend > on]. Presumably Ingo will pick it up, not Linus. I know how to specify a prerequisite. I just wasn't sure which commit hash to use, as up to now I've only one from your tree and I guessed that wouldn't do it. Juergen