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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <30ad77af-4a7b-4a15-9c0b-b0c70d9e1643@wanadoo.fr>
Date: Mon, 3 Feb 2025 19:04:34 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Miquel Raynal <miquel.raynal@...tlin.com>,
 Jiasheng Jiang <jiashengjiangcool@...il.com>
Cc: richard@....at, vigneshr@...com, gmpy.liaowx@...il.com, kees@...nel.org,
 linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mtd: Add check for kcalloc()

Le 03/02/2025 à 09:32, Miquel Raynal a écrit :
> On 02/02/2025 at 20:54:56 GMT, Jiasheng Jiang <jiashengjiangcool@...il.com> 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@...il.com>
>> ---
>>   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
> 
>