Re: [PATCH] mtd: Add check for kcalloc()
From: Jiasheng Jiang
Date: Mon Feb 03 2025 - 21:37:41 EST
Hi Christophe,
On Mon, Feb 3, 2025 at 1:04 PM Christophe JAILLET
<christophe.jaillet@xxxxxxxxxx> wrote:
>
> Le 03/02/2025 à 09:32, Miquel Raynal a écrit :
> > On 02/02/2025 at 20:54:56 GMT, Jiasheng Jiang <jiashengjiangcool@xxxxxxxxx> wrote:
> >
> >> Add a check for kcalloc() and print an error message if it fails.
> >
> > Checking, yes, but logging, no. IIRC the core does it already if required.
> >
> >> Fixes: 78c08247b9d3 ("mtd: Support kmsg dumper based on pstore/blk")
> >
> > Cc: stable is missing
> >
> >> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@xxxxxxxxx>
> >> ---
> >> drivers/mtd/mtdpstore.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/mtd/mtdpstore.c b/drivers/mtd/mtdpstore.c
> >> index 7ac8ac901306..368997155c07 100644
> >> --- a/drivers/mtd/mtdpstore.c
> >> +++ b/drivers/mtd/mtdpstore.c
> >> @@ -423,6 +423,11 @@ static void mtdpstore_notify_add(struct mtd_info *mtd)
> >> longcnt = BITS_TO_LONGS(div_u64(mtd->size, mtd->erasesize));
> >> cxt->badmap = kcalloc(longcnt, sizeof(long), GFP_KERNEL);
> >>
> >> + if (!cxt->rmmap || !cxt->usedmap || !cxt->badmap) {
> >> + dev_err(&mtd->dev, "failed to allocate memory for map\n");
> >> + return;
> >> + }
> >> +
> >> /* just support dmesg right now */
> >> cxt->dev.flags = PSTORE_FLAGS_DMESG;
> >> cxt->dev.zone.read = mtdpstore_read;
> >
> > What if register_pstore_device() fails? Your only partially fixing the
> > problem if you don't handle the free()s there as well.
>
> ... and if an allocation fails above, then some kfree() also needs to be
> called.
>
Thanks, I have add kfree() to deal with each allocation in my v2.
> And, unrelated to your patch, these kcalloc()/kfree() could be some
> bitmap_zalloc()/bitmap_free(). (but i'm not sure it really worth the effort)
I think it works well currently, so it may not be necessary to do the
replacements.
>
> CJ
>
> >
> > Thanks,
> > Miquèl
> >
> >
>
-Jiasheng