[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPcyv4iY9PWULZfGnKnTiD4tVxHSw4QfSwUBV4d=1KfnUjbgWw@mail.gmail.com>
Date: Mon, 22 Jan 2018 10:07:28 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Russell King - ARM Linux <linux@...linux.org.uk>
Cc: 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>,
Linus Torvalds <torvalds@...ux-foundation.org>, Alan Cox <alan@...ux.intel.com>
Subject: Re: [PATCH v4.1 02/10] asm/nospec, array_ptr: sanitize speculative
array de-references
On Sun, Jan 21, 2018 at 2:40 AM, Russell King - ARM Linux
<linux@...linux.org.uk> wrote:
> On Sat, Jan 20, 2018 at 01:06:09PM -0800, Dan Williams wrote:
>> +/*
>> + * If idx is negative or if idx > size then bit 63 is set in the mask,
>> + * and the value of ~(-1L) is zero. When the mask is zero, bounds check
>> + * failed, array_ptr will return NULL.
>
> The more times I see this the more times I'm unhappy with this comment:
>
> 1. does this really mean "idx > size" or "idx >= size" ? The code
> implements the latter not the former.
>
> 2. is "bit 63" relevant here - what if longs are 32-bit? "the top bit"
> or "the sign bit" would be better.
>
> 3. "and the value of ~(-1L) is zero." So does this mean that when
> 0 <= idx < size, somehow the rules of logic change and ~(-1L)
> magically becomes no longer zero!
>
> I'd suggest changing the description to something like:
>
> * If 0 <= idx < size, return a mask of ~0UL, otherwise return zero.
>
> or:
>
> * When idx is out of bounds (iow, is negative or idx >= sz), the sign
> * bit will be set. Extend the sign bit to all bits and invert, giving
> * a result of zero for an out of bounds idx, or ~0UL if within bounds.
>
> depending on how deeply you want to describe what's going on here.
Sounds good Russell, I'll fold the longer description in for the next
version to save the next person needing to decode this technique.
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.
