Re: [PATCH] x86: correct fpu emulation access to ldt
On Aug 5, 2015 8:35 PM, "Juergen Gross" wrote: > > 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: sta...@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. Gotcha. I thought it was a strange question, and I obviously misunderstood. Use: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=x86/urgent&id=37868fe113ff2ba814b3b4eb12df214df555f8dc if you haven't spotted it yet. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: correct fpu emulation access to ldt
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: sta...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: correct fpu emulation access to ldt
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. > +#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: sta...@vger.kernel.org # [commit hash you depend on]. Presumably Ingo will pick it up, not Linus. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: correct fpu emulation access to ldt
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, }}}) +#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? Juergen -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: correct fpu emulation access to ldt
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? Also, want to make it slightly fancier so we can drop the dependency on CONFIG_MODIFY_LDT_SYSCALL? --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86: correct fpu emulation access to ldt
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 --- arch/x86/math-emu/fpu_entry.c | 1 + arch/x86/math-emu/fpu_system.h | 2 +- arch/x86/math-emu/get_address.c | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/math-emu/fpu_entry.c b/arch/x86/math-emu/fpu_entry.c index f37e84a..97820c24 100644 --- a/arch/x86/math-emu/fpu_entry.c +++ b/arch/x86/math-emu/fpu_entry.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include diff --git a/arch/x86/math-emu/fpu_system.h b/arch/x86/math-emu/fpu_system.h index 9ccecb6..7c7af3a 100644 --- a/arch/x86/math-emu/fpu_system.h +++ b/arch/x86/math-emu/fpu_system.h @@ -18,7 +18,7 @@ /* 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]) +#define LDT_DESCRIPTOR(s) (current->mm->context.ldt->entries[(s) >> 3]) #define SEG_D_SIZE(x) ((x).b & (3 << 21)) #define SEG_G_BIT(x) ((x).b & (1 << 23)) #define SEG_GRANULARITY(x) (((x).b & (1 << 23)) ? 4096 : 1) diff --git a/arch/x86/math-emu/get_address.c b/arch/x86/math-emu/get_address.c index 6ef5e99..4f83d09 100644 --- a/arch/x86/math-emu/get_address.c +++ b/arch/x86/math-emu/get_address.c @@ -21,6 +21,7 @@ #include #include +#include #include "fpu_system.h" #include "exception.h" -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/