[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+55aFyqAGRE4+QteYyvNyMzepFXHVVBn=A07Adj-yBs=tS_Ww@mail.gmail.com>
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.
