Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <e86106b6-b7a5-ee6f-4865-151353c68ec7@gmail.com>
Date:   Fri, 25 Mar 2022 09:35:46 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH] media: uvc_video: fix bit overflow in uvc_probe_video
Content-Language: en-US
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     mchehab@kernel.org, linux-media@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220324091308.44799-1-hbh25y@gmail.com>
 <Yjx3laKM9Ah+1PRC@pendragon.ideasonboard.com>
From:   Hangyu Hua <hbh25y@gmail.com>
In-Reply-To: <Yjx3laKM9Ah+1PRC@pendragon.ideasonboard.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@gmail.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) {
>