Hi Prabhakar, On Tue, 19 May 2026 at 15:44, Lad, Prabhakar <[email protected]> wrote: > On Tue, May 19, 2026 at 2:34 PM Geert Uytterhoeven <[email protected]> > wrote: > > On Tue, 19 May 2026 at 15:30, Prabhakar <[email protected]> wrote: > > > From: Lad Prabhakar <[email protected]> > > > > > > Fix an counter tracking in mmc_test_alloc_mem() that causes a kernel panic > > > during error unwinding. > > > > > > The `struct mmc_test_mem` uses the `__counted_by(cnt)` annotation on its > > > flexible array member `arr`. While kzalloc_flex() initially sets the > > > counter field (`cnt`) to `max_segs`, the allocation loop needs to track > > > how many elements have actually been populated. > > > > > > Previously, leaving `mem->cnt` at `max_segs` meant that if the loop failed > > > midway (e.g., "Failed to map sg list"), the error unwinding path in > > > mmc_test_free_mem() would attempt to clean up uninitialized trailing > > > array slots. This resulted in passing NULL pointers to __free_pages(), > > > triggering a kernel panic: > > > > > > [ 66.172845] mmc0: Failed to map sg list > > > [ 66.176722] Unable to handle kernel NULL pointer dereference at > > > virtual address 0000000000000000 > > > ... > > > [ 66.432747] Call trace: > > > [ 66.435191] ___free_pages+0x1c/0xc4 (P) > > > [ 66.439119] __free_pages+0x14/0x20 > > > [ 66.442608] mmc_test_area_cleanup+0x58/0x84 [mmc_test] > > > > > > Fix this by explicitly resetting `mem->cnt` to 0 immediately after > > > allocation. Then, move the existing `mem->cnt` increment so that it occurs > > > prior to populating each array slot, using `mem->cnt - 1` for the actual > > > assignment index. This guarantees that the counter accurately tracks > > > initialized entries for safe error cleanup, while dynamically expanding > > > the `__counted_by` validation boundary ahead of each flexible array write. > > > > > > Additionally, rewrite the cleanup loop in mmc_test_free_mem() to use a > > > standard forward for-loop. This addresses the unsafe post-decrement logic > > > in the original `while (mem->cnt--)` loop which evaluated and decremented > > > the counter field before indexing the array, and avoids a potential > > > integer > > > underflow/wrap-around of the counter field if the cleanup path is invoked > > > when `mem->cnt` is 0. > > > > > > Fixes: c3126dccfd7b ("mmc: mmc_test: use kzalloc_flex") > > > Signed-off-by: Lad Prabhakar <[email protected]> > > > --- > > > v1->v2: > > > - Started with cnt = 0 and incremented before assignment to ensure > > > accurate tracking of initialized entries in mmc_test_alloc_mem(). > > > - In mmc_test_free_mem(), replaced the while loop with a forward for-loop > > > to > > > safely iterate over initialized entries without risking underflow. > > > - Updated commit message to clarify the issue and the fix. > > > > Thanks for your patch! > > > > > --- a/drivers/mmc/core/mmc_test.c > > > +++ b/drivers/mmc/core/mmc_test.c > > > @@ -318,9 +318,8 @@ static void mmc_test_free_mem(struct mmc_test_mem > > > *mem) > > > { > > > if (!mem) > > > return; > > > - while (mem->cnt--) > > > - __free_pages(mem->arr[mem->cnt].page, > > > - mem->arr[mem->cnt].order); > > > + for (unsigned int i = 0; i < mem->cnt; i++) > > > + __free_pages(mem->arr[i].page, mem->arr[i].order); > > > kfree(mem); > > > } > > > > > > @@ -356,6 +355,7 @@ static struct mmc_test_mem > > > *mmc_test_alloc_mem(unsigned long min_sz, > > > mem = kzalloc_flex(*mem, arr, max_segs); > > > if (!mem) > > > return NULL; > > > + mem->cnt = 0; > > > > This is not needed, as it is set to zero by kzalloc_flex(). > > > Actually, kzalloc_flex() automatically sets mem->cnt to max_segs > because cnt is annotated with __counted_by. Because of that implicit > initialization, we need this explicit reset to get it back to zero.
Only when your compiler supports it[1]. OMG... When I commented on the LWN.net article[2], I considered only the case where the compiler is too old, and the counter stays at zero when the user forgets to initialize it explicitly. Now we have the opposite case, where we need the counter to stay at zero :-( [1] https://elixir.bootlin.com/linux/v7.0.9/source/include/linux/compiler_types.h#L549 [2] https://lwn.net/Articles/1063295/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
