Hi Prabhakar,
On Tue, 19 May 2026 at 12:05, Lad, Prabhakar <[email protected]> wrote:
> On Mon, May 18, 2026 at 12:08 PM Geert Uytterhoeven
> <[email protected]> wrote:
> > On Wed, 13 May 2026 at 22:13, Prabhakar <[email protected]> wrote:
> > > From: Lad Prabhakar <[email protected]>
> > >
> > > Fix logic issues introduced by the kzalloc_flex() conversion in
> > > mmc_test_alloc_mem() due to interaction with the __counted_by
> > > annotation on the flexible array.
> > >
> > > Bounds-checking sanitizers rely on the counter field reflecting the
> > > allocated array size before any array access occurs. However, use
> > > mem->cnt both as the allocation size and as the runtime insertion
> > > index, causing incorrect indexing and potentially invalid bounds
> > > tracking.
> > >
> > > Initialize mem->cnt to the maximum allocated number of segments
> > > immediately after kzalloc_flex(), then use a separate local index
> > > variable to track successfully allocated entries. Update mem->cnt to
> > > the actual number of initialized elements before returning or entering
> > > the cleanup path.
> > >
> > > Also rewrite mmc_test_free_mem() to use a forward for-loop, improving
> > > readability and ensuring only initialized entries are freed.
> > >
> > > Fixes: c3126dccfd7b ("mmc: mmc_test: use kzalloc_flex")
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > --- a/drivers/mmc/core/mmc_test.c
> > > +++ b/drivers/mmc/core/mmc_test.c
> > > @@ -316,11 +316,13 @@ static int mmc_test_buffer_transfer(struct
> > > mmc_test_card *test,
> > >
> > > static void mmc_test_free_mem(struct mmc_test_mem *mem)
> > > {
> > > + unsigned int idx;
> > > +
> > > if (!mem)
> > > return;
> > > - while (mem->cnt--)
> > > - __free_pages(mem->arr[mem->cnt].page,
> > > - mem->arr[mem->cnt].order);
> > > + for (idx = 0; idx < mem->cnt; idx++)
> >
> > for (unsigned int i; ...)?
> >
> Ok.
>
> > > + __free_pages(mem->arr[idx].page,
> > > + mem->arr[idx].order);
> > > kfree(mem);
> > > }
> > >
> > > @@ -341,6 +343,7 @@ static struct mmc_test_mem
> > > *mmc_test_alloc_mem(unsigned long min_sz,
> > > unsigned long page_cnt = 0;
> > > unsigned long limit = nr_free_buffer_pages() >> 4;
> > > struct mmc_test_mem *mem;
> > > + unsigned int idx = 0;
> > >
> > > if (max_page_cnt > limit)
> > > max_page_cnt = limit;
> > > @@ -356,6 +359,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 = max_segs;
> > >
> > > while (max_page_cnt) {
> > > struct page *page;
> > > @@ -375,23 +379,26 @@ 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[idx].page = page;
> > > + mem->arr[idx].order = order;
> > > + idx += 1;
> >
> > While looking rather ugly, I think starting with mem->cnt at zero,
> > and updating it in each step like
> >
> > mem->cnt++;
> > mem->arr[mem->cnt - 1].page = page;
> > mem->arr[mem->cnt - 1].order = order;
> >
> > would still be better, as it makes the dependency between mem->cnt and
> > the size of mem->arr[] clearer (located closer to each other), and ...
> >
> >
> Ok, I will start with mem->cnt at zero; with this I can drop changes
> in mmc_test_free_mem().
I don't think you can drop these changes, as mmc_test_free_mem()
does mem->cnt-- _before_ accessing mem->arr[mem->cnt].
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
