Re: Linux 5.19-rc8

Dennis Zhou
2022-07-26T17:40:38Z

Hello,

On Mon, Jul 25, 2022 at 10:55:18AM -0700, Linus Torvalds wrote:
> On Mon, Jul 25, 2022 at 9:11 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > BUG: KFENCE: out-of-bounds read in _find_next_bit_le+0x10/0x48
>
> Ok, I was hoping somebody more ARMy would look at this, particularly
> since there is no call trace beyond the actual fault.
>
> So it shows that it happens in _find_next_bit_le(), but not who called it.
>
> It does show "who allocated the page", and I can see the message that
> is printed afterwards, so it comes from that
>
> static void __init test_bitmap_printlist(void)
>
> function, so I guess we know the call chain:
>
> test_bitmap_printlist ->
> bitmap_print_to_pagebuf ->
> scnprintf "%*pbl\n" ->
> pointer ->
> bitmap_list_string ->
> for_each_set_bitrange
>
> and I think I see what's wrong in there. That thing does
>
> (b) = find_next_bit((addr), (size), (e) + 1), \
> (e) = find_next_zero_bit((addr), (size), (b) + 1))
>
> for the end of the range, and looking at the oops, the instruction
> that oopses is
>
> ldrb r3, [r0, r2, lsr #3]
>
> where 'r2' is the bit position, and 'r0' is the start of the bitmap.
>
> And:
>
> > r10: 00000000 r9 : 0000002d r8 : ef59d000
> > r7 : c0e55514 r6 : c2215000 r5 : 00008000 r4 : 00008000
> > r3 : 845cac12 r2 : 00008001 r1 : 00008000 r0 : ef59d000
>
> Lookie here: r1 contains the size, and r2 is past the end of the size.
>
> So pick your poison: either the bug is in
>
> (a) the bitmap region iterators shouldn't even ask for past-the-end results
>
> I've added Dennis Zhou who did that first
> bitmap_for_each_set_region() in commit e837dfde15a4 ("bitmap:
> genericize percpu bitmap region iterators"), and Yuri Norov who
> renamed and moved it to for_each_set_bitrange() in commit ec288a2cf7ca
> ("bitmap: unify find_bit operations").
>

It seems like this is mostly taken care of by migrating arm to use the
generic implementations, but I just want to cover our basis here.

Are we okay with adding the contract find_*_bit() operations must handle
asking for past size properly? FWIW, we'd have to modify most of the
iterators in find.h.

Thanks,
Dennis