Re: [PATCH] media/usb/siano: Fix endpoint type checking in smsusb

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

 



On Mon, Aug 19, 2024 at 10:15:11AM +0200, Mauro Carvalho Chehab wrote:
> This patch is duplicated of this one:
> 
> https://patchwork.linuxtv.org/project/linux-media/patch/20240409143634.33230-1-n.zhandarovich@xxxxxxxxxx/
> 
> The part I didn't like with such approach is that it checks only for
> bulk endpoints. Most media devices have also isoc. Now, I'm not sure
> about Siano devices. There are 3 different major chipsets supported
> by this driver (sm1000, sm11xx, sm2xxx). Among them, sm1000 has one
> USB ID for cold boot, and, once firmware is loaded, it gains another
> USB ID for a a warm boot.

Are you sure about all this?  The one source file in 
drivers/media/usb/siano refers only to bulk endpoints, and the files in 
drivers/media/common/siano don't refer to endpoints or URBs at all.  
Nothing in either directory refers to isochronous endpoints.  Is there 
some other place I should be looking?

Also, while there are many constants in those files whose names start 
with SMS1, there aren't any whose names start with SMS2 or SM2 (or their 
lowercase equivalents).  And the Kconfig help text mentions only Siano 
SMS1xxx.

> Your patch and the previously submitted one are not only checking
> for the direction, but it is also discarding isoc endpoints.
> Applying a change like that without testing with real hardware of
> those three types just to make fuzz testing happy, sounded a little 
> bit risky to my taste.
> 
> I would be more willing to pick it if the check would either be
> tested on real hardware or if the logic would be changed to
> accept either bulk or isoc endpoints, just like the current code.

If the driver did apply to devices that used isochronous transfers, the 
ideal approach would be to check the endpoint type against the device 
type.  However, as it stands this doesn't seem to be necessary.

Alan Stern




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux