Hi Geert, On Tue, May 19, 2026 at 11:09 AM Geert Uytterhoeven <[email protected]> wrote: > > 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]. > Ack.
Cheers, Prabhakar
