Re: [PATCH] fs/proc/vmcore: a few cleanups for vmcore_add_device_dump

[Su Hui <suhui@nfschina.com>]

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