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?
