lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANiDSCsDrDQty1T6DM6Au-bptOrJT9x49a=pGOawph1v6iisJw@mail.gmail.com>
Date: Mon, 7 Apr 2025 17:48:18 +0200
From: Ricardo Ribalda <ribalda@...omium.org>
To: Hans de Goede <hdegoede@...hat.com>
Cc: chenchangcheng <ccc194101@....com>, laurent.pinchart@...asonboard.com, 
	mchehab@...nel.org, linux-media@...r.kernel.org, linux-kernel@...r.kernel.org, 
	chenchangcheng <chenchangcheng@...inos.cn>
Subject: Re: [PATCH v6] media: uvcvideo: Fix bandwidth issue for Alcor camera

Hi

On Mon, 7 Apr 2025 at 15:39, Hans de Goede <hdegoede@...hat.com> wrote:
>
> Hi,
>
> On 24-Mar-25 03:21, chenchangcheng wrote:
> > From: chenchangcheng <chenchangcheng@...inos.cn>
> >
> > Some broken device return wrong dwMaxPayloadTransferSize fields,
> > as follows:
> > [  218.632537] [pid:20427,cpu6,guvcview,8]uvcvideo: Device requested 2752512 B/frame bandwidth.
> > [  218.632598] [pid:20427,cpu6,guvcview,9]uvcvideo: No fast enough alt setting for requested bandwidth.
> >
> > The maximum packet size of the device is 3 * 1024, according to the
> > logs above, the device needs to apply for a bandwidth of 0x2a0000.
> >
> > Bus 001 Device 008: ID 1b17:6684 Alcor Corp. Slave camera
> > Device Descriptor:
> >   bLength                18
> >   bDescriptorType         1
> >   bcdUSB               2.00
> >   bDeviceClass          239 Miscellaneous Device
> >   bDeviceSubClass         2
> >   bDeviceProtocol         1 Interface Association
> >   bMaxPacketSize0        64
> >   idVendor           0x1b17
> >   idProduct          0x6684
> >   bcdDevice            1.05
> >   iManufacturer           1 Alcor Corp.
> >   iProduct                2 Slave camera
> >   iSerial                 0
> >   bNumConfigurations      1
> >   Configuration Descriptor:
> >     bLength                 9
> >     bDescriptorType         2
> >     wTotalLength       0x02ad
> >     bNumInterfaces          2
> >     bConfigurationValue     1
> >     iConfiguration          0
> >     bmAttributes         0x80
> >       (Bus Powered)
> >     MaxPower              200mA
> >     Interface Association:
> >       bLength                 8
> >       bDescriptorType        11
> >       bFirstInterface         0
> >       bInterfaceCount         2
> >       bFunctionClass         14 Video
> >       bFunctionSubClass       3 Video Interface Collection
> >       bFunctionProtocol       0
> >       iFunction               4 Slave camera
> >     Interface Descriptor:
> >       bLength                 9
> >       bDescriptorType         4
> >       bInterfaceNumber        0
> >       bAlternateSetting       0
> >       bNumEndpoints           1
> >       bInterfaceClass        14 Video
> >       bInterfaceSubClass      1 Video Control
> >       bInterfaceProtocol      0
> >       iInterface              4 Slave camera
> >       VideoControl Interface Descriptor:
> >
> >       ....
> >
> >       Endpoint Descriptor:
> >         bLength                 7
> >         bDescriptorType         5
> >         bEndpointAddress     0x81  EP 1 IN
> >         bmAttributes            3
> >           Transfer Type            Interrupt
> >           Synch Type               None
> >           Usage Type               Data
> >         wMaxPacketSize     0x0010  1x 16 bytes
> >         bInterval               7
> >     Interface Descriptor:
> >       bLength                 9
> >       bDescriptorType         4
> >       bInterfaceNumber        1
> >       bAlternateSetting       0
> >       bNumEndpoints           0
> >       bInterfaceClass        14 Video
> >       bInterfaceSubClass      2 Video Streaming
> >       bInterfaceProtocol      0
> >       iInterface              0
> >       VideoStreaming Interface Descriptor:
> >         bLength                            14
> >         bDescriptorType                    36
> >         bDescriptorSubtype                  1 (INPUT_HEADER)
> >         bNumFormats                         1
> >         wTotalLength                   0x01ef
> >         bEndPointAddress                  130
> >         bmInfo                              0
> >         bTerminalLink                       3
> >         bStillCaptureMethod                 2
> >         bTriggerSupport                     1
> >         bTriggerUsage                       0
> >         bControlSize                        1
> >         bmaControls( 0)                     0
> >       VideoStreaming Interface Descriptor:
> >         bLength                            11
> >         bDescriptorType                    36
> >         bDescriptorSubtype                  6 (FORMAT_MJPEG)
> >         bFormatIndex                        1
> >         bNumFrameDescriptors                9
> >         bFlags                              1
> >           Fixed-size samples: Yes
> >         bDefaultFrameIndex                  1
> >         bAspectRatioX                       0
> >         bAspectRatioY                       0
> >         bmInterlaceFlags                 0x00
> >           Interlaced stream or variable: No
> >           Fields per frame: 1 fields
> >           Field 1 first: No
> >           Field pattern: Field 1 only
> >         bCopyProtect                        0
> >       VideoStreaming Interface Descriptor:
> >         bLength                            50
> >         bDescriptorType                    36
> >         bDescriptorSubtype                  7 (FRAME_MJPEG)
> >         bFrameIndex                         1
> >         bmCapabilities                   0x00
> >           Still image unsupported
> >         wWidth                           1920
> >         wHeight                          1080
> >         dwMinBitRate                248832000
> >         dwMaxBitRate                1492992000
> >         dwMaxVideoFrameBufferSize     6220800
> >         dwDefaultFrameInterval         333333
> >         bFrameIntervalType                  6
> >         dwFrameInterval( 0)            333333
> >         dwFrameInterval( 1)            400000
> >         dwFrameInterval( 2)            500000
> >         dwFrameInterval( 3)            666666
> >         dwFrameInterval( 4)           1000000
> >         dwFrameInterval( 5)           2000000
> >
> >       ......
> >
> >       VideoStreaming Interface Descriptor:
> >         bLength                            42
> >         bDescriptorType                    36
> >         bDescriptorSubtype                  3 (STILL_IMAGE_FRAME)
> >         bEndpointAddress                    0
> >         bNumImageSizePatterns               9
> >         wWidth( 0)                       1920
> >         wHeight( 0)                      1080
> >         wWidth( 1)                       2048
> >         wHeight( 1)                      1536
> >         wWidth( 2)                       1280
> >         wHeight( 2)                       720
> >         wWidth( 3)                       2592
> >         wHeight( 3)                      1944
> >         wWidth( 4)                       1280
> >         wHeight( 4)                      1024
> >         wWidth( 5)                       1280
> >         wHeight( 5)                       960
> >         wWidth( 6)                       1600
> >         wHeight( 6)                      1200
> >         wWidth( 7)                        800
> >         wHeight( 7)                       600
> >         wWidth( 8)                        640
> >         wHeight( 8)                       480
> >         bNumCompressionPatterns             0
> >       VideoStreaming Interface Descriptor:
> >         bLength                             6
> >         bDescriptorType                    36
> >         bDescriptorSubtype                 13 (COLORFORMAT)
> >         bColorPrimaries                     1 (BT.709,sRGB)
> >         bTransferCharacteristics            1 (BT.709)
> >         bMatrixCoefficients                 4 (SMPTE 170M (BT.601))
> >     Interface Descriptor:
> >       bLength                 9
> >       bDescriptorType         4
> >       bInterfaceNumber        1
> >       bAlternateSetting       1
> >       bNumEndpoints           1
> >       bInterfaceClass        14 Video
> >       bInterfaceSubClass      2 Video Streaming
> >       bInterfaceProtocol      0
> >       iInterface              0
> >       Endpoint Descriptor:
> >         bLength                 7
> >         bDescriptorType         5
> >         bEndpointAddress     0x82  EP 2 IN
> >         bmAttributes            5
> >           Transfer Type            Isochronous
> >           Synch Type               Asynchronous
> >           Usage Type               Data
> >         wMaxPacketSize     0x1400  3x 1024 bytes
> >         bInterval               1
> >     Interface Descriptor:
> >       bLength                 9
> >       bDescriptorType         4
> >       bInterfaceNumber        1
> >       bAlternateSetting       2
> >       bNumEndpoints           1
> >       bInterfaceClass        14 Video
> >       bInterfaceSubClass      2 Video Streaming
> >       bInterfaceProtocol      0
> >       iInterface              0
> >       Endpoint Descriptor:
> >         bLength                 7
> >         bDescriptorType         5
> >         bEndpointAddress     0x82  EP 2 IN
> >         bmAttributes            5
> >           Transfer Type            Isochronous
> >           Synch Type               Asynchronous
> >           Usage Type               Data
> >         wMaxPacketSize     0x0b84  2x 900 bytes
> >         bInterval               1
> >     Interface Descriptor:
> >       bLength                 9
> >       bDescriptorType         4
> >       bInterfaceNumber        1
> >       bAlternateSetting       3
> >       bNumEndpoints           1
> >       bInterfaceClass        14 Video
> >       bInterfaceSubClass      2 Video Streaming
> >       bInterfaceProtocol      0
> >       iInterface              0
> >       Endpoint Descriptor:
> >         bLength                 7
> >         bDescriptorType         5
> >         bEndpointAddress     0x82  EP 2 IN
> >         bmAttributes            5
> >           Transfer Type            Isochronous
> >           Synch Type               Asynchronous
> >           Usage Type               Data
> >         wMaxPacketSize     0x0c00  2x 1024 bytes
> >         bInterval               1
> >     Interface Descriptor:
> >       bLength                 9
> >       bDescriptorType         4
> >       bInterfaceNumber        1
> >       bAlternateSetting       4
> >       bNumEndpoints           1
> >       bInterfaceClass        14 Video
> >       bInterfaceSubClass      2 Video Streaming
> >       bInterfaceProtocol      0
> >       iInterface              0
> >       Endpoint Descriptor:
> >         bLength                 7
> >         bDescriptorType         5
> >         bEndpointAddress     0x82  EP 2 IN
> >         bmAttributes            5
> >           Transfer Type            Isochronous
> >           Synch Type               Asynchronous
> >           Usage Type               Data
> >         wMaxPacketSize     0x0c00  2x 1024 bytes
> >         bInterval               1
> > Device Qualifier (for other device speed):
> >   bLength                10
> >   bDescriptorType         6
> >   bcdUSB               2.00
> >   bDeviceClass          239 Miscellaneous Device
> >   bDeviceSubClass         2
> >   bDeviceProtocol         1 Interface Association
> >   bMaxPacketSize0        64
> >   bNumConfigurations      1
> > Device Status:     0x0000
> >   (Bus Powered)
> >
> > Signed-off-by: chenchangcheng <chenchangcheng@...inos.cn>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@...hat.com>
>
> Ricardo, do you have any more remarks about this patch ?

LGTM, I forgot to send the trailer sorry

Reviewed-by: Ricardo Ribalda <ribalda@...omium.org>

Not sure if Laurent wants the whole lsusb -v as the commit message os
as a cover letter.

Both work for me.

>
>
> Regards,
>
> Hans
>
>
> > ---
> >  drivers/media/usb/uvc/uvc_driver.c |  9 +++++++++
> >  drivers/media/usb/uvc/uvc_video.c  | 10 ++++++++++
> >  drivers/media/usb/uvc/uvcvideo.h   |  1 +
> >  3 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index deadbcea5e22..9b1dedf9773b 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -3023,6 +3023,15 @@ static const struct usb_device_id uvc_ids[] = {
> >         .bInterfaceSubClass   = 1,
> >         .bInterfaceProtocol   = 0,
> >         .driver_info          = UVC_INFO_QUIRK(UVC_QUIRK_STATUS_INTERVAL) },
> > +     /* Alcor Corp. Slave camera */
> > +     { .match_flags          = USB_DEVICE_ID_MATCH_DEVICE
> > +                             | USB_DEVICE_ID_MATCH_INT_INFO,
> > +       .idVendor             = 0x1b17,
> > +       .idProduct            = 0x6684,
> > +       .bInterfaceClass      = USB_CLASS_VIDEO,
> > +       .bInterfaceSubClass   = 1,
> > +       .bInterfaceProtocol   = 0,
> > +       .driver_info          = UVC_INFO_QUIRK(UVC_QUIRK_OVERFLOW_BANDWIDTH) },
> >       /* MSI StarCam 370i */
> >       { .match_flags          = USB_DEVICE_ID_MATCH_DEVICE
> >                               | USB_DEVICE_ID_MATCH_INT_INFO,
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index e3567aeb0007..56f23c363870 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -262,6 +262,16 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream,
> >
> >               ctrl->dwMaxPayloadTransferSize = bandwidth;
> >       }
> > +
> > +     if (stream->intf->num_altsetting > 1 &&
> > +         ctrl->dwMaxPayloadTransferSize > stream->maxpsize &&
> > +         stream->dev->quirks & UVC_QUIRK_OVERFLOW_BANDWIDTH) {
> > +             dev_warn(&stream->intf->dev,
> > +                      "the max payload transmission size (%d) exceededs the size of the ep max packet (%d). Using the max size.\n",
> > +                      ctrl->dwMaxPayloadTransferSize,
> > +                      stream->maxpsize);
> > +             ctrl->dwMaxPayloadTransferSize = stream->maxpsize;
> > +     }
> >  }
> >
> >  static size_t uvc_video_ctrl_size(struct uvc_streaming *stream)
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index 5e388f05f3fc..8b43d725c259 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -77,6 +77,7 @@
> >  #define UVC_QUIRK_DISABLE_AUTOSUSPEND        0x00008000
> >  #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000
> >  #define UVC_QUIRK_MJPEG_NO_EOF               0x00020000
> > +#define UVC_QUIRK_OVERFLOW_BANDWIDTH 0x00040000
> >
> >  /* Format flags */
> >  #define UVC_FMT_FLAG_COMPRESSED              0x00000001
> >
> > base-commit: 4701f33a10702d5fc577c32434eb62adde0a1ae1
>
>


-- 
Ricardo Ribalda

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ