usb_control_msg bugged?

[sukoy]

Hi,

Pi3B+
.firmware_revision 4cf061b4500b87c26c721db4d847593452270cff
4.19.65-v7+ f1c1b67b26ed3cb789037d1c844d4105deaa3cfd

I got some troubles with usb, to better understand start from scratch.

I found a lot of cameras with the default param bugged, here is an example: v4l2-ctl -d0 --all

brightness 0x00980900 (int)    : min=-64 max=64 step=1 default=-8193 value=0
                       contrast 0x00980901 (int)    : min=0 max=64 step=1 default=57343 value=32
                     saturation 0x00980902 (int)    : min=0 max=128 step=1 default=57343 value=105
                            hue 0x00980903 (int)    : min=-40 max=40 step=1 default=-8193 value=0
 white_balance_temperature_auto 0x0098090c (bool)   : default=1 value=1
                          gamma 0x00980910 (int)    : min=72 max=500 step=1 default=57343 value=100
                           gain 0x00980913 (int)    : min=0 max=100 step=1 default=57343 value=0
           power_line_frequency 0x00980918 (menu)   : min=0 max=2 default=1 value=1
      white_balance_temperature 0x0098091a (int)    : min=2800 max=6500 step=1 default=57343 value=4600 flags=inactive
                      sharpness 0x0098091b (int)    : min=0 max=6 step=1 default=57343 value=3
         backlight_compensation 0x0098091c (int)    : min=0 max=2 step=1 default=57343 value=1
                  exposure_auto 0x009a0901 (menu)   : min=0 max=3 default=0 value=3
              exposure_absolute 0x009a0902 (int)    : min=1 max=5000 step=1 default=93 value=93 flags=inactive
         exposure_auto_priority 0x009a0903 (bool)   : default=0 value=1

As you can see some default are corrupted with a common pattern -8193 or 57343 (it depends if default is signed or not ). Going around I found a lot of people with the exactly the same problem, but unfortunately no solution. The same camera works well on other systems with different kernel versions, I suspect the problems is on pi.
During last 2 days I found the right time to start to investigate. Honestly I was quite sure the problem was related to the uvc video driver, but it isn't. After digging in the uvc driver I found a buffer overflow caused maybe by usb stack.
In uvc_ctrl.c:950 getting the resolution param corrupt the default param, the resolution controller is 2 byte long but for some reasons something writes other 2 additional byte 0xff 0xdf and because default param come after resolution, it is written with those bad values. Other curious aspect, some params like exposure_absolute are declared as 4 byte long, in this case the overflow is masked but It is still there. The function on that line called is uvc_query_ctrl, I spent a lot of time there, trying to find something that doesn't work but unfortunately for you :P I didn't found anything. The problem seems to be in uvc_video:44, usb_control_msg, it is a urb blocking call.
To reproduce, find a common usb camera, attach to pi and check with v4l2-ctl -d0 --all if default is corrupted or not. In case you get, some bad values, like me, put some useful printf in uvc_video.c:37, before calling urb usb function
uvc_printk(KERN_INFO, "%s:%d data:%x data[0]=%x data[1]:%x data[2]=%x data[3]:%x size:%d\n", __FILE__, __LINE__, data, ((u8*)data)[0], ((u8*)data)[1], ((u8*)data)[2], ((u8*)data)[3], size);
and after the same print, uvc_video.c:51
uvc_printk(KERN_INFO, "%s:%d data:%x data[0]=%x data[1]:%x data[2]=%x data[3]:%x size:%d\n", __FILE__, __LINE__, data, ((u8*)data)[0], ((u8*)data)[1], ((u8*)data)[2], ((u8*)data)[3], size);

You will get a log like this:

/home/pi/linux/drivers/media/usb/uvc/uvc_video.c:39 data:b2befe88 data[0]=0 data[1]:0 data[2]=0 data[3]:0 size:2
[10182.313089] uvcvideo: /home/pi/linux/drivers/media/usb/uvc/uvc_video.c:53 data:b2befe88 data[0]=1 data[1]:0 data[2]=ff data[3]:df size:2

despite the size is 2, other 2 additional bytes are written with some bad values.
My 2cents, I suspect the usb bug is largely diffused.

Let me know if you need additional information, honestly I haven't time to dig in usb stack to catch the overflow.

Thanks

[P33M]

You're right - with a random webcam on a pi4 vs pi3b+, I get corrupt default values on Pi3b+ but not a Pi4.
What it should be:

                     brightness 0x00980900 (int)    : min=0 max=100 step=1 default=30 value=30
                       contrast 0x00980901 (int)    : min=0 max=100 step=1 default=57 value=57
                     saturation 0x00980902 (int)    : min=0 max=4 step=1 default=2 value=2
                          gamma 0x00980910 (int)    : min=1 max=8 step=1 default=3 value=3
           power_line_frequency 0x00980918 (menu)   : min=0 max=2 default=1 value=1
                      sharpness 0x0098091b (int)    : min=0 max=100 step=1 default=10 value=10
         backlight_compensation 0x0098091c (int)    : min=0 max=1 step=1 default=1 value=1

What it reports on 3B+:

                     brightness 0x00980900 (int)    : min=0 max=100 step=1 default=-8193 value=30
                       contrast 0x00980901 (int)    : min=0 max=100 step=1 default=57343 value=57
                     saturation 0x00980902 (int)    : min=0 max=4 step=1 default=57343 value=2
                          gamma 0x00980910 (int)    : min=1 max=8 step=1 default=57343 value=3
           power_line_frequency 0x00980918 (menu)   : min=0 max=2 default=1 value=1
                      sharpness 0x0098091b (int)    : min=0 max=100 step=1 default=57343 value=10
         backlight_compensation 0x0098091c (int)    : min=0 max=1 step=1 default=57343 value=1

[P33M]

From an analyzer trace, when doing a sudo v4l2-ctl -d 0 --all a sequence of control transfers is performed, all with 2-byte responses. We know that the dwc otg core used on previous Pi models requires a 4-byte aligned buffer - I wonder if received packets shorter than 4 bytes cause a 4-byte memory write.

[sukoy]

From an analyzer trace, when doing a sudo v4l2-ctl -d 0 --all a sequence of control transfers is performed, all with 2-byte responses. We know that the dwc otg core used on previous Pi models requires a 4-byte aligned buffer - I wonder if received packets shorter than 4 bytes cause a 4-byte memory write.

My experience told me that the bug is there, in usb low level driver, the usb stack is stable and solid. Despite if low level function works on 4 byte aligned buffers, the design of code should protect against the buffer overflow in any case, fire an error in case the function can't proceed or copy on local buffer as workaround.

[P33M]

Using dwc2 instead of dwc_otg has a similar, but different symptom:

                     brightness 0x00980900 (int)    : min=0 max=20478 step=1 default=-8193 value=30
                       contrast 0x00980901 (int)    : min=0 max=20478 step=1 default=57343 value=57
                     saturation 0x00980902 (int)    : min=0 max=20478 step=1 default=57343 value=2
                          gamma 0x00980910 (int)    : min=1 max=57343 step=1 default=57343 value=3
           power_line_frequency 0x00980918 (menu)   : min=0 max=2 default=0 value=1
                      sharpness 0x0098091b (int)    : min=0 max=20478 step=1 default=57343 value=10
         backlight_compensation 0x0098091c (int)    : min=0 max=20478 step=1 default=57343 value=1

[P33M]

The hardware dribbles extra bytes into memory for small IN transfers. If I force usage of the "align_buf" mechanism for the data stage of a control transfer, then I get correct readback from v4l2-ctl as the actual number of transfer bytes are memcpy'd into the buffer provided by the application.

[sukoy]

well done, when do you plan to release a new kernel version?

[popcornmix]

Probably will be an rpi-update kernel tomorrow.

[P33M]

Fixing dwc2 is going to be a bit more involved - the upstream driver only ever uses the align_buf for split-transaction IN transfers and just barfs on non-aligned buffers otherwise.

[popcornmix]

The fix is in latest rpi-update kernel.

[sukoy]

ok it works, please close

Thanks