Hi Geert, Thank you for the review.
On Mon, May 18, 2026 at 12:08 PM Geert Uytterhoeven <[email protected]> wrote: > > Hi Prabhakar, > > 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]> > > Thanks for your patch! > > > --- 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(). Cheers, Prabhakar > > if (max_page_cnt <= (1UL << order)) > > break; > > max_page_cnt -= 1UL << order; > > page_cnt += 1UL << order; > > - if (mem->cnt >= max_segs) { > > + if (idx >= mem->cnt) { > > if (page_cnt < min_page_cnt) > > goto out_free; > > break; > > } > > } > > > > + mem->cnt = idx; > > + > > return mem; > > > > out_free: > > + mem->cnt = idx; > > ... as having to set mem->cnt twice looks rather fragile to me. > > > mmc_test_free_mem(mem); > > return NULL; > > } > > Regardless, as the patch looks correct to me: > Reviewed-by: Geert Uytterhoeven <[email protected]> > > 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
