Re: [Xen-devel] UBSAN report in find_next_bit()

["Jan Beulich" <JBeulich@suse.com>]

>>> On 24.06.19 at 18:24, <andrew.cooper3@citrix.com> wrote:
>> else if ( (node = next_node(node, nodemask)) >= MAX_NUMNODES )
>>     node = first_node(nodemask);
> 
> On x86, MAX_NUMNODES is 64, and this part of get_free_buddy() loops over
> nodes {0..63}.  next_node() expands to find_next_bit(..., node+1) which
> passes offset == size on the final iteration.
> 
> find_next_bit() has an optimisation for bitmaps of 64 or fewer bits
> which does:
> 
>> else if ( __builtin_constant_p(size) && s__ <= BITS_PER_LONG )
>>     r__ = o__ + __scanbit(*(const unsigned long *)(a__) >> o__, s__);
> 
> UBSAN takes objection to the shift, which in this case is a shift by 64.
> 
> The code in __find_next_bit() makes it clear that offset == size is a
> valid condition, which would suggest that the bug is with the optimisation.

Oh, in particular the ASSERT() there is indeed very clear.

> However, this conclusion contradicts the views of c/s b20079da9 which
> decided that offset == size is not a valid condition.

And that was based on how x86'es find_next{,_zero}_bit() as well
as ...

> ARM64's find_next_bit() explicitly copes with offset >= size, and while
> I don't speak ARM asm well enough to work out whether
> _find_first_bit_le() copes with offset == size, the vgic.c code
> definitely expects it to function in this way.

... Arm32's _find_next{,_zero}_bit_le. You've named the issue the x86
logic has. Arm32's, afaict, will read one byte past the array when offset
and size match and are a multiple of 8.

> As a result, I think the reasoning in c/s b20079da9 is false, and that
> change needs re-adjusting.  I also think that x86's optimisation for
> size == 64 should be considered buggy and fixed.  TBH, I'm not sure the
> optimisation is worthwhile having in the first place.

The question though is whether, alongside offset == size potentially
being meant to be valid, offset > size is to be treated like such, too.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel