Re: [PATCH] Make sure we parse really received data.

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

 



On 12/23/21 20:09, Ricardo Ribalda wrote:
Hi Hans

Thanks for your patch

On Thu, 23 Dec 2021 at 16:42, Hans Petter Selasky <hps@xxxxxxxxxxx> wrote:

Hi,

USB control requests may return less data than we ask for.
Found using valgrind and webcamd on FreeBSD.

If the usb control request returns less data, then the checks for
ret!=size will trigger.

Am I missing something obvious?


Hi,

USB control transfers are allowed to be short terminated! But there is no flag to error out on short terminated control transfers, from what I can see. This is sometimes used for reading strings. You setup a fixed 255 byte buffer, and then simply issue the control read string request. The length you get back is the actual string length.

> If the usb control request returns less data, then the checks for
> ret!=size will trigger.

Can you point in the code where this check is?

--HPS

Best regards



==15522== Conditional jump or move depends on uninitialised value(s)
==15522==    at 0x662EF4: uvc_fixup_video_ctrl (uvc_video.c:135)
==15522==    by 0x662EF4: uvc_get_video_ctrl (uvc_video.c:297)
==15522==    by 0x6640B0: uvc_video_init (uvc_video.c:2078)
==15522==    by 0x65E79D: uvc_register_video (uvc_driver.c:2258)
==15522==    by 0x65E79D: uvc_register_terms (uvc_driver.c:2300)
==15522==    by 0x65E79D: uvc_register_chains (uvc_driver.c:2321)
==15522==    by 0x65E79D: uvc_probe (uvc_driver.c:2463)
==15522==    by 0x3C8F46: usb_linux_probe_p (linux_usb.c:449)
==15522==    by 0x75B4B2: main (webcamd.c:1021)
==15522==  Uninitialised value was created by a heap allocation
==15522==    at 0x4853844: malloc (in
/usr/local/libexec/valgrind/vgpreload_memcheck-amd64-freebsd.so)
==15522==    by 0x3BC8A4: kmalloc (linux_func.c:1807)
==15522==    by 0x662C8C: uvc_get_video_ctrl (uvc_video.c:229)
==15522==    by 0x6640B0: uvc_video_init (uvc_video.c:2078)
==15522==    by 0x65E79D: uvc_register_video (uvc_driver.c:2258)
==15522==    by 0x65E79D: uvc_register_terms (uvc_driver.c:2300)
==15522==    by 0x65E79D: uvc_register_chains (uvc_driver.c:2321)
==15522==    by 0x65E79D: uvc_probe (uvc_driver.c:2463)
==15522==    by 0x3C8F46: usb_linux_probe_p (linux_usb.c:449)
==15522==    by 0x75B4B2: main (webcamd.c:1021)

Signed-off-by: Hans Petter Selasky <hps@xxxxxxxxxxx>
---
   drivers/media/usb/uvc/uvc_video.c | 6 +++++-
   1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index 9f37eaf28ce7..6233703f9a50 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -258,7 +258,11 @@ static int uvc_get_video_ctrl(struct uvc_streaming
*stream,
                         query == UVC_GET_DEF)
                 return -EIO;

-       data = kmalloc(size, GFP_KERNEL);
+       /*
+        * Make sure we parse really received data
+        * by allocating a zeroed buffer.
+        */
+       data = kzalloc(size, GFP_KERNEL);
         if (data == NULL)
                 return -ENOMEM;

--
2.34.1







[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux