[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240819182456.5e13315a@foz.lan>
Date: Mon, 19 Aug 2024 18:24:56 +0200
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Greg KH <gregkh@...uxfoundation.org>, Mauro Carvalho Chehab
<mchehab@...nel.org>, syzbot
<syzbot+85e3ddbf0ddbfbc85f1e@...kaller.appspotmail.com>,
linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
linux-usb@...r.kernel.org, syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] media/usb/siano: Fix endpoint type checking in smsusb
Em Mon, 19 Aug 2024 10:32:05 -0400
Alan Stern <stern@...land.harvard.edu> escreveu:
> 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@fintech.ru/
> >
> > 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.
The initial driver had support only for SM1000 and SM11xx. There is a small
note there about the sm1000 devices there (I guess this is the chipset
number of Stellar, but my memories might be tricking me), but without
a real association with the chipset number:
/* This device is only present before firmware load */
{ USB_DEVICE(0x187f, 0x0010),
.driver_info = SMS1XXX_BOARD_SIANO_STELLAR_ROM },
/* This device pops up after firmware load */
{ USB_DEVICE(0x187f, 0x0100),
.driver_info = SMS1XXX_BOARD_SIANO_STELLAR },
Years later, support for sm2xxx was added.
Those two boards, for instance (see drivers/media/common/siano/sms-cards.c)
are variants of sm2xxx (one of them is sm2270, if I'm not mistaken) that
supports Brazilian TV standard:
[SMS1XXX_BOARD_SIANO_PELE] = {
.name = "Siano Pele Digital Receiver",
.type = SMS_PELE,
.default_mode = DEVICE_MODE_ISDBT_BDA,
},
[SMS1XXX_BOARD_SIANO_RIO] = {
.name = "Siano Rio Digital Receiver",
.type = SMS_RIO,
.default_mode = DEVICE_MODE_ISDBT_BDA,
},
There are some boards there with a different version of sm22xx
that supports only DVB (can't remember anymore what boards).
Basically, the actual SMS device type is given by this enum:
enum sms_device_type_st {
SMS_UNKNOWN_TYPE = -1,
SMS_STELLAR = 0,
SMS_NOVA_A0,
SMS_NOVA_B0,
SMS_VEGA,
SMS_VENICE,
SMS_MING,
SMS_PELE,
SMS_RIO,
SMS_DENVER_1530,
SMS_DENVER_2160,
SMS_NUM_OF_DEVICE_TYPES /* This is just a count */
};
But I dunno if there are a 1:1 mapping between type and chipset
number. The above type names probably match some vendor internal
names, but we never had any tables associating them to a device number,
as the vendor never provided us such information.
Btw I vaguely remember I heard about a newer Siano chipsets (sm3xxx),
but never saw such devices.
-
Now, I'm not sure about what endpoints this specific driver exports, as
I'm lacking vendor's documentation. What I said is that almost all DVB
devices have isoc endpoints, but I dunno if this is the case of Siano.
Thanks,
Mauro
Powered by blists - more mailing lists