Messages in this thread | | | Date | Tue, 26 Jul 2022 10:39:50 -0700 | From | Dennis Zhou <> | Subject | Re: Linux 5.19-rc8 |
| |
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
| |