Hi Hans On Fri, 31 Dec 2021 at 10:59, Hans Petter Selasky <hps@xxxxxxxxxxx> wrote: > > 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? https://elixir.bootlin.com/linux/latest/source/drivers/media/usb/uvc/uvc_video.c#L281 and https://elixir.bootlin.com/linux/latest/source/drivers/media/usb/uvc/uvc_video.c#L291 > > --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 > > > > > > > -- Ricardo Ribalda
