Re: [PATCH v4.1 02/10] asm/nospec, array_ptr: sanitize speculative array de-references

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 22, 2018 at 10:37 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, Jan 22, 2018 at 10:07 AM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>> On Sun, Jan 21, 2018 at 2:40 AM, Russell King - ARM Linux
>> <linux@xxxxxxxxxxxxxxx> 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?

We could at least have a BUILD_BUG_ON when 'size' is a
builtin_contstant and greater than LONG_MAX. Alternatively we could
require archs to provide the equivalent of the x86 array_ptr_mask()
that does not have the LONG_MAX limitation?



[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux