linux-kernel - Re: [PATCH] media: uvc_video: fix bit overflow in uvc_probe

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] [day] [month] [year] [list]

Date:   Fri, 25 Mar 2022 09:35:46 +0800
From:   Hangyu Hua <hbh25y@...il.com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     mchehab@...nel.org, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: uvc_video: fix bit overflow in uvc_probe_video

Thanks. I will do it.

On 2022/3/24 21:52, Laurent Pinchart wrote:
> Hi Hangyu,
> 
> Thank you for the patch.
> 
> On Thu, Mar 24, 2022 at 05:13:08PM +0800, Hangyu Hua wrote:
>> probe->dwMaxPayloadTransferSize is a 32bit value, but bandwidth is 16bit. This
>> may lead to a bit overflow.
>>
>> Signed-off-by: Hangyu Hua <hbh25y@...il.com>
>> ---
>>   drivers/media/usb/uvc/uvc_video.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
>> index 1b4cc934109e..cc4878373aa7 100644
>> --- a/drivers/media/usb/uvc/uvc_video.c
>> +++ b/drivers/media/usb/uvc/uvc_video.c
>> @@ -383,7 +383,7 @@ int uvc_probe_video(struct uvc_streaming *stream,
>>   	struct uvc_streaming_control *probe)
>>   {
>>   	struct uvc_streaming_control probe_min, probe_max;
>> -	u16 bandwidth;
>> +	u32 bandwidth;
>>   	unsigned int i;
>>   	int ret;
>>   
>> @@ -422,7 +422,7 @@ int uvc_probe_video(struct uvc_streaming *stream,
>>   			break;
>>   
>>   		bandwidth = probe->dwMaxPayloadTransferSize;
>> -		if (bandwidth <= stream->maxpsize)
>> +		if (bandwidth <= (u32)stream->maxpsize)
> 
> The cast to u32 isn't needed, it's implicit.
> 
> This could actually be written
> 
> 	if (probe->dwMaxPayloadTransferSize <= stream->maxpsize)
> 
> The bandwidth local variable can be dropped. Could you submit a v2 with
> this change ?
> 
>>   			break;
>>   
>>   		if (stream->dev->quirks & UVC_QUIRK_PROBE_MINMAX) {
>