Re: [PATCH] mtd: Add check for kcalloc()

[Christophe JAILLET]

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.

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)

CJ

> Thanks,
> Miquèl