If the request length of UVC XU is 1 (even though this is illegal), due
to 'data' may be the non-zero value, UVC_GET_LEN could potentially result
in a length that is not 1 because of the high byte is not zero. In order
to ensure that 2-byte data array is set to 0, 'kmalloc' is modified to 'kzalloc'.
Signed-off-by: Jerry Liu <[email protected]>
---
drivers/media/usb/uvc/uvc_ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 5e9d3da862dd..054bc14f7a58 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2088,7 +2088,7 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
u8 *data;
int ret;
- data = kmalloc(2, GFP_KERNEL);
+ data = kzalloc(2, GFP_KERNEL);
if (data == NULL)
return -ENOMEM;
--
2.25.1
Hi Jerry,
Thank you for the patch.
On Fri, Sep 15, 2023 at 09:12:14AM -0700, Jerry Liu wrote:
> If the request length of UVC XU is 1 (even though this is illegal), due
> to 'data' may be the non-zero value, UVC_GET_LEN could potentially result
> in a length that is not 1 because of the high byte is not zero. In order
> to ensure that 2-byte data array is set to 0, 'kmalloc' is modified to 'kzalloc'.
I don't think this can happen. The call to uvc_query_ctrl(UVC_GET_LEN)
is given a length of 2. If the device responds with less than two bytes,
the function will return an error, and uvc_ctrl_fill_xu_info() will
propagate the error to the caller, without accessing the data array.
>
> Signed-off-by: Jerry Liu <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 5e9d3da862dd..054bc14f7a58 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2088,7 +2088,7 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
> u8 *data;
> int ret;
>
> - data = kmalloc(2, GFP_KERNEL);
> + data = kzalloc(2, GFP_KERNEL);
> if (data == NULL)
> return -ENOMEM;
>
--
Regards,
Laurent Pinchart
Hi Laurent,
Thanks for your comment!
I'm sorry I have a confusing description for that.
You're right it's sure to return the error if it gets an error length
from UVC. But I think it still can work despite receiving the length is 1.
In uvc_query_ctrl, it will return an error but the value is 1 not '-EPIPE',
I think even though the length is less than 1, it still continues to execute
the XU command.
However, I found it will receive the wrong data from uvc_query_ctrl because
it only assigns 1-byte, not 2-byte value to the data array.
For example:
if data array is not allocated with zero bytes:
asssigned value of
data[0] | data[1] 1 byte length data[0] | data[1]
--------------------- --------------------> ---------------------
0xcc | 0xcc 0x01 | 0xcc
then in uvc_ctrl_fill_xu_info, 'info->size' will get wrong size from data array.
Somtimes, the data array is allocated with zero bytes:
asssigned value of
data[0] | data[1] 1 byte length data[0] | data[1]
--------------------- --------------------> ---------------------
0x00 | 0x00 0x01 | 0x00
In this case, 'info->size' will get correct size from data array.
On Fri, Sep 15, 2023 at 10:04:49PM +0300, Laurent Pinchart wrote:
> Hi Jerry,
>
> Thank you for the patch.
>
> On Fri, Sep 15, 2023 at 09:12:14AM -0700, Jerry Liu wrote:
> > If the request length of UVC XU is 1 (even though this is illegal), due
> > to 'data' may be the non-zero value, UVC_GET_LEN could potentially result
> > in a length that is not 1 because of the high byte is not zero. In order
> > to ensure that 2-byte data array is set to 0, 'kmalloc' is modified to 'kzalloc'.
>
> I don't think this can happen. The call to uvc_query_ctrl(UVC_GET_LEN)
> is given a length of 2. If the device responds with less than two bytes,
> the function will return an error, and uvc_ctrl_fill_xu_info() will
> propagate the error to the caller, without accessing the data array.
>
> >
> > Signed-off-by: Jerry Liu <[email protected]>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 5e9d3da862dd..054bc14f7a58 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -2088,7 +2088,7 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
> > u8 *data;
> > int ret;
> >
> > - data = kmalloc(2, GFP_KERNEL);
> > + data = kzalloc(2, GFP_KERNEL);
> > if (data == NULL)
> > return -ENOMEM;
> >
>
> --
> Regards,
>
> Laurent Pinchart