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

Reply via email to