Message ID | 20230915161213.42503-1-jerry.liu@technexion.com |
---|---|
State | New |
Headers | show |
Series | media: uvcvideo: Modified uvc_ctrl_fill_xu_info 'kmalloc' to 'kzalloc' | expand |
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 <jerry.liu@technexion.com> > > --- > > 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
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;
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 <jerry.liu@technexion.com> --- drivers/media/usb/uvc/uvc_ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)