Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1200013 > unrolled thread

[PATCH] x86: correct fpu emulation access to ldt

Back to article view | Back to linux.kernel

Contents

  [PATCH] x86: correct fpu emulation access to ldt Juergen Gross <jgross@suse.com> - 2015-08-04 17:10 +0200
    Re: [PATCH] x86: correct fpu emulation access to ldt Andy Lutomirski <luto@amacapital.net> - 2015-08-04 20:10 +0200
      Re: [PATCH] x86: correct fpu emulation access to ldt Juergen Gross <jgross@suse.com> - 2015-08-05 11:20 +0200
        Re: [PATCH] x86: correct fpu emulation access to ldt Andy Lutomirski <luto@amacapital.net> - 2015-08-05 20:30 +0200
          Re: [PATCH] x86: correct fpu emulation access to ldt Juergen Gross <jgross@suse.com> - 2015-08-06 05:40 +0200
            Re: [PATCH] x86: correct fpu emulation access to ldt Andy Lutomirski <luto@amacapital.net> - 2015-08-06 06:40 +0200

#1200013 — [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 <jgross@suse.com>
---
 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 <asm/uaccess.h>
 #include <asm/traps.h>
 #include <asm/desc.h>
+#include <asm/mmu_context.h>
 #include <asm/user.h>
 #include <asm/fpu/internal.h>
 
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 <asm/uaccess.h>
 #include <asm/desc.h>
+#include <asm/mmu_context.h>
 
 #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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1200201

On Tue, Aug 4, 2015 at 8:02 AM, Juergen Gross <jgross@suse.com> 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 <jgross@suse.com>

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200567

On 08/04/2015 08:01 PM, Andy Lutomirski wrote:
> On Tue, Aug 4, 2015 at 8:02 AM, Juergen Gross <jgross@suse.com> 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 <jgross@suse.com>
>
> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201050

On Wed, Aug 5, 2015 at 2:11 AM, Juergen Gross <jgross@suse.com> wrote:
> On 08/04/2015 08:01 PM, Andy Lutomirski wrote:
>>
>> On Tue, Aug 4, 2015 at 8:02 AM, Juergen Gross <jgross@suse.com> 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 <jgross@suse.com>
>>
>>
>> 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: stable@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201411

On 08/05/2015 08:24 PM, Andy Lutomirski wrote:
> On Wed, Aug 5, 2015 at 2:11 AM, Juergen Gross <jgross@suse.com> wrote:
>> On 08/04/2015 08:01 PM, Andy Lutomirski wrote:
>>>
>>> On Tue, Aug 4, 2015 at 8:02 AM, Juergen Gross <jgross@suse.com> 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 <jgross@suse.com>
>>>
>>>
>>> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201423

On Aug 5, 2015 8:35 PM, "Juergen Gross" <jgross@suse.com> wrote:
>
> On 08/05/2015 08:24 PM, Andy Lutomirski wrote:
>>
>> On Wed, Aug 5, 2015 at 2:11 AM, Juergen Gross <jgross@suse.com> wrote:
>>>
>>> On 08/04/2015 08:01 PM, Andy Lutomirski wrote:
>>>>
>>>>
>>>> On Tue, Aug 4, 2015 at 8:02 AM, Juergen Gross <jgross@suse.com> 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 <jgross@suse.com>
>>>>
>>>>
>>>>
>>>> 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.

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web