Re: [PATCH] media: atomisp: Fix buffer overrun in gmin_get_var_int()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 5/27/23 18:55, Andy Shevchenko wrote:
> On Sat, May 27, 2023 at 6:51 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>>
>> Not all functions used in gmin_get_var_int() update len to the actual
>> length of the returned string. So len may still have its initial value
>> of the length of val[] when "val[len] = 0;" is run to ensure 0 termination.
>>
>> If this happens we end up writing one beyond the bounds of val[], fix this.
>>
>> Note this is a quick fix for this since the entirety of
>> atomisp_gmin_platform.c will be removed once all atomisp sensor
>> drivers have been moved over to runtime-pm + v4l2-async device
>> registration.
> 
> ...
> 
>> Closes: https://lore.kernel.org/linux-media/26f37e19-c240-4d77-831d-ef3f1a4dd51d@kili.mountain/
> 
> Is this a new official tag? (Just to my surprise)

Yes, I was surprised too, checkpatch.pl now wants a Closes: tag
after a Reported-by: one, rather then a Link: tag.

> 
> ...
> 
>> -       char val[CFG_VAR_NAME_MAX];
>> -       size_t len = sizeof(val);
>> +       char val[CFG_VAR_NAME_MAX + 1];
>> +       size_t len = CFG_VAR_NAME_MAX;
> 
> Why not sizeof() - 1? At least it will be a single point when change
> can be made and not breaking again in a similar way the code.

I wanted to make the buffer one byte bigger to make room for
the terminating 0, not 1 bute smaller.

Regards,

Hams





[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux