linux-kernel - Re: [PATCH] fs/proc/vmcore: a few cleanups for vmcore_add_device

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: <a90315db-e2d9-4a86-9be5-6ea88f5c94a2@nfschina.com>
Date: Tue, 24 Jun 2025 10:14:55 +0800
From: Su Hui <suhui@...china.com>
To: Dan Carpenter <dan.carpenter@...aro.org>, Baoquan He <bhe@...hat.com>
Cc: akpm@...ux-foundation.org, vgoyal@...hat.com, dyoung@...hat.com,
 kexec@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-fsdevel@...r.kernel.org, kernel-janitors@...r.kernel.org,
 Su Hui <suhui@...china.com>
Subject: Re: [PATCH] fs/proc/vmcore: a few cleanups for vmcore_add_device_dump


On 2025/6/23 23:22, Dan Carpenter wrote:
> On Mon, Jun 23, 2025 at 10:36:45PM +0800, Baoquan He wrote:
>> On 06/23/25 at 06:47pm, Su Hui wrote:
>>> There are three cleanups for vmcore_add_device_dump(). Adjust data_size's
>>> type from 'size_t' to 'unsigned int' for the consistency of data->size.
>> It's unclear to me why size_t is not suggested here. Isn't it assigned
>> a 'sizeof() + data->size' in which size_t should be used?

Oh, sorry for this, I missed some things.

1497         data_size = roundup(sizeof(struct vmcoredd_header) + 
data->size,
1498                             PAGE_SIZE);
1499
1500         /* Allocate buffer for driver's to write their dumps */
1501         buf = vmcore_alloc_buf(data_size);
             [...]
1515
1516         dump->buf = buf;
1517         dump->size = data_size;
                  ^^^^^^^^^^^^^^^^^^^^^
If data_size is 64 bit and assume data_size is bigger than 32bit, 
dump->size will overflow.
Should we adjust dump->size's type to size_t? Or maybe it's impossible 
for data_size bigger
than 32bit?
> Yeah...  That's a good point.  People should generally default to size_t
> for sizes.  It really does prevent a lot of integer overflow bugs.  In
> this case data->size is not controlled by the user, but if it were
> then that would be an integer overflow on 32bit systems and not on
> 64bit systems, until we start declaring sizes as unsigned int and
> then all the 32bit bugs start affecting everyone.
Agreed, sorry for my fault again.
I will remove the 'unsigned int' in v2 patch.
Thanks for your suggestions!

regards,
Su Hui