Hi Prabhakar,
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().
>
> while (max_page_cnt) {
> struct page *page;
The rest LGTM.
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