drivers/mmc/core/mmc_test.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
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.
v1: https://lore.kernel.org/all/20260513201315.3186621-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
---
drivers/mmc/core/mmc_test.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
index ab38e4c45a8d..3c7e8a0704bb 100644
--- 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;
while (max_page_cnt) {
struct page *page;
@@ -375,9 +375,9 @@ static struct mmc_test_mem *mmc_test_alloc_mem(unsigned long min_sz,
goto out_free;
break;
}
- mem->arr[mem->cnt].page = page;
- mem->arr[mem->cnt].order = order;
mem->cnt += 1;
+ mem->arr[mem->cnt - 1].page = page;
+ mem->arr[mem->cnt - 1].order = order;
if (max_page_cnt <= (1UL << order))
break;
max_page_cnt -= 1UL << order;
--
2.54.0Hi Prabhakar,
On Tue, 19 May 2026 at 15:30, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> 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 -- geert@linux-m68k.org
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
Hi Geert,
Thank you for the review.
On Tue, May 19, 2026 at 2:34 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, 19 May 2026 at 15:30, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > 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.
Cheers,
Prabhakar
On 5/19/26 07:44, Lad, Prabhakar wrote:
> Hi Geert,
>
> Thank you for the review.
>
> On Tue, May 19, 2026 at 2:34 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>
>> Hi Prabhakar,
>>
>> On Tue, 19 May 2026 at 15:30, Prabhakar <prabhakar.csengg@gmail.com> wrote:
>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>
>>> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>> 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.
An auxiliary variable could be used to avoid having to update the
counter too early[1][2].
I think it'll eventually become best practice to defer updating the
counter until after the flexible array has been fully initialized, or
after every major update that requires changing its boundaries.
-Gustavo
[1] https://git.kernel.org/linus/ea9e148c803b24eb
[2] https://embeddedor.com/blog/2024/06/18/how-to-use-the-new-counted_by-attribute-in-c-and-linux/
(I'm in the process of updating this blogpost with some *alloc_flex() examples and more.)
Hi Gustavo,
On Tue, 19 May 2026 at 20:44, Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
> On 5/19/26 07:44, Lad, Prabhakar wrote:
> > On Tue, May 19, 2026 at 2:34 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >> On Tue, 19 May 2026 at 15:30, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> >>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>
> >>> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>> 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.
>
> An auxiliary variable could be used to avoid having to update the
> counter too early[1][2].
Thank you!
In light of this, Prabhakar's v1[3] is the better solution.
But perhaps that the addition of "mem->cnt = max_segs;" after the
kzalloc_flex() should be dropped, as it is done automatically by
compilers that support __counted_by, but is irrelevant for compilers
that do not support it (the correct value is set at the end anyway).
Gustavo, what do you think?
> I think it'll eventually become best practice to defer updating the
> counter until after the flexible array has been fully initialized, or
> after every major update that requires changing its boundaries.
In case the update is a _decrease_.
If the update is an _increase_, it must be done first?
> [1] https://git.kernel.org/linus/ea9e148c803b24eb
> [2] https://embeddedor.com/blog/2024/06/18/how-to-use-the-new-counted_by-attribute-in-c-and-linux/
> (I'm in the process of updating this blogpost with some *alloc_flex() examples and more.)
[3] "[PATCH] mmc: mmc_test: Fix __counted_by handling after
kzalloc_flex() conversion"
https://lore.kernel.org/20260513201315.3186621-1-prabhakar.mahadev-lad.rj@bp.renesas.com
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
Hi Prabhakar,
On Tue, 19 May 2026 at 15:44, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> On Tue, May 19, 2026 at 2:34 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, 19 May 2026 at 15:30, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > > 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 -- geert@linux-m68k.org
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
Hi Geert,
On Tue, May 19, 2026 at 2:55 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, 19 May 2026 at 15:44, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > On Tue, May 19, 2026 at 2:34 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, 19 May 2026 at 15:30, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > ---
> > > > 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 :-(
>
Yeah, it definitely introduces some tricky, asymmetrical behavior
depending on the toolchain.
Cheers,
Prabhakar
> [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 -- geert@linux-m68k.org
>
> 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
© 2016 - 2026 Red Hat, Inc.