linux-kernel - Re: [PATCH] x86: correct fpu emulation access to ldt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALCETrWqr_UV3vZNppjcaUDe+AKc3QcK4fDXMHbGc0Mk_QESog@mail.gmail.com>
Date:	Wed, 5 Aug 2015 11:24:00 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Juergen Gross <jgross@...e.com>
Cc:	billm@...bpc.org.au,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86: correct fpu emulation access to ldt

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