Date: Mon, 22 Jan 2018 10:37:58 -0800 From: Linus Torvalds <torvalds@...ux-foundation.org> To: Dan Williams <dan.j.williams@...el.com> Cc: Russell King - ARM Linux <linux@...linux.org.uk>, Thomas Gleixner <tglx@...utronix.de>, linux-arch <linux-arch@...r.kernel.org>, Kernel Hardening <kernel-hardening@...ts.openwall.com>, Peter Zijlstra <peterz@...radead.org>, Catalin Marinas <catalin.marinas@....com>, X86 ML <x86@...nel.org>, Will Deacon <will.deacon@....com>, Ingo Molnar <mingo@...hat.com>, Greg KH <gregkh@...uxfoundation.org>, "H. Peter Anvin" <hpa@...or.com>, Alan Cox <alan@...ux.intel.com> Subject: Re: [PATCH v4.1 02/10] asm/nospec, array_ptr: sanitize speculative array de-references On Mon, Jan 22, 2018 at 10:07 AM, Dan Williams <dan.j.williams@...el.com> wrote: > On Sun, Jan 21, 2018 at 2:40 AM, Russell King - ARM Linux > <linux@...linux.org.uk> wrote: >> >> I'd suggest changing the description to something like: >> >> * If 0 <= idx < size, return a mask of ~0UL, otherwise return zero. Yes. HOWEVER. We should make it clear that the signedness of the comparisons is undefined, and that 'size' should always be positive for the above to be well-specified. Why? Because one valid implementation of the above is: idx U< size ? ~0UL : 0 as a single unsigned comparison (I'm using "U<" as a "unsigned less than"). But an equally valid implementation of it might be idx S>= 0 && idx S< size ? ~0UL : 0 which does two signed comparisons instead. And they are not exactly the same. They are the same _IFF_ 'size' is positive in 'long'. But if size is an unsigned value so big as to be negative in 'long', they give different results for 'idx'. In fact, the ALU-only version actually did a third version of the above that only uses "signed comparison against zero": idx S> 0 && (size - idx - 1) S>= 0 ? ~0UL : 0 which again is equivalent only when 'size' is positive in 'long'. Note that 'idx' itself can have any range (that's kind of the _point_, after all: checking that idx is in some range). But the logic only works for a positive array size. Which honestly is not really a limitation, but it's worth spelling out, I think. In particular, if you have a user pointer, and a 3G:1G split in user:kernel address space, you camn *not* do something like uptr = array_ptr(NULL, userptr, TASK_SIZE); to get a sanitized user pointer, because TASK_SIZE is not positive in 'long'. Hmm? Linus
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.