| 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
| ||
|
Message-ID: <DM8PR02MB81986250F1FC52F950ACD923E3089@DM8PR02MB8198.namprd02.prod.outlook.com>
Date: Mon, 7 Mar 2022 01:24:35 +0000
From: "Linyu Yuan (QUIC)" <quic_linyyuan@...cinc.com>
To: "mka@...omium.org" <mka@...omium.org>,
"Linyu Yuan (QUIC)" <quic_linyyuan@...cinc.com>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"Tao Wang (Consultant) (QUIC)" <quic_wat@...cinc.com>,
"balbi@...nel.org" <balbi@...nel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"dianders@...omium.org" <dianders@...omium.org>,
"frowand.list@...il.com" <frowand.list@...il.com>,
"hadess@...ess.net" <hadess@...ess.net>,
"krzk@...nel.org" <krzk@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"mathias.nyman@...el.com" <mathias.nyman@...el.com>,
"michal.simek@...inx.com" <michal.simek@...inx.com>,
"peter.chen@...nel.org" <peter.chen@...nel.org>,
"ravisadineni@...omium.org" <ravisadineni@...omium.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"rogerq@...nel.org" <rogerq@...nel.org>,
"stern@...land.harvard.edu" <stern@...land.harvard.edu>,
"swboyd@...omium.org" <swboyd@...omium.org>
Subject: RE: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver
> From: mka@...omium.org <mka@...omium.org>
> Sent: Saturday, March 5, 2022 12:48 AM
> To: Linyu Yuan (QUIC) <quic_linyyuan@...cinc.com>
> Cc: gregkh@...uxfoundation.org; Tao Wang (Consultant) (QUIC)
> <quic_wat@...cinc.com>; balbi@...nel.org; devicetree@...r.kernel.org;
> dianders@...omium.org; frowand.list@...il.com; hadess@...ess.net;
> krzk@...nel.org; linux-kernel@...r.kernel.org; linux-usb@...r.kernel.org;
> mathias.nyman@...el.com; michal.simek@...inx.com;
> peter.chen@...nel.org; ravisadineni@...omium.org; robh+dt@...nel.org;
> rogerq@...nel.org; stern@...land.harvard.edu; swboyd@...omium.org
> Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add
> onboard_usb_hub driver
>
> On Wed, Mar 02, 2022 at 05:14:30AM +0000, Linyu Yuan (QUIC) wrote:
> > > From: mka@...omium.org <mka@...omium.org>
> > > Sent: Wednesday, March 2, 2022 12:33 AM
> > > To: Linyu Yuan (QUIC) <quic_linyyuan@...cinc.com>
> > > Cc: gregkh@...uxfoundation.org; Tao Wang (Consultant) (QUIC)
> > > <quic_wat@...cinc.com>; balbi@...nel.org;
> devicetree@...r.kernel.org;
> > > dianders@...omium.org; frowand.list@...il.com; hadess@...ess.net;
> > > krzk@...nel.org; linux-kernel@...r.kernel.org; linux-
> usb@...r.kernel.org;
> > > mathias.nyman@...el.com; michal.simek@...inx.com;
> > > peter.chen@...nel.org; ravisadineni@...omium.org;
> robh+dt@...nel.org;
> > > rogerq@...nel.org; stern@...land.harvard.edu;
> swboyd@...omium.org
> > > Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add
> > > onboard_usb_hub driver
> > >
> > > > In my opinion, if it need update VID/PID table in this driver to support a
> > > new HUB,
> > > > we can parse VID/PID from device tree and create dynamic VID/PID
> entry
> > > to id_table of onboard_hub_usbdev_driver.
> > > >
> > > > Hope you can understand what I said.
> > >
> > > Not really.
> > >
> > > I doubt that what you are suggesting would work. The easiest thing
> > > to convince people would probably be to send a patch (based on this
> > > one) with a working implementation of your idea.
> >
> > I show my idea, but not test,
> >
> > diff --git a/drivers/usb/misc/onboard_usb_hub.c
> b/drivers/usb/misc/onboard_usb_hub.c
> > index e280409..1811317 100644
> > --- a/drivers/usb/misc/onboard_usb_hub.c
> > +++ b/drivers/usb/misc/onboard_usb_hub.c
> > @@ -10,6 +10,7 @@
> > #include <linux/init.h>
> > #include <linux/kernel.h>
> > #include <linux/list.h>
> > +#include <linux/slab.h>
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > #include <linux/of.h>
> > @@ -173,6 +174,58 @@ static void onboard_hub_remove_usbdev(struct
> onboard_hub *hub, const struct usb_
> > mutex_unlock(&hub->lock);
> > }
> >
> > +#define MAX_HUB_NUM 4
> > +#define MAX_TABLE_SIZE (MAX_HUB_NUM * 2)
> > +static struct usb_device_id onboard_hub_id_table[MAX_TABLE_SIZE + 1];
> > +MODULE_DEVICE_TABLE(usb, onboard_hub_id_table);
> > +
> > +static void onboard_hub_add_idtable(__u16 vid, __u16 pid)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < MAX_TABLE_SIZE; i++) {
> > + if (!onboard_hub_id_table[i].idVendor)
> > + break;
> > +
> > + if (onboard_hub_id_table[i].idVendor == vid &&
> > + onboard_hub_id_table[i].idProduct == pid)
> > + return;
> > + }
> > + if (i == MAX_TABLE_SIZE)
> > + return;
> > +
> > + onboard_hub_id_table[i].idVendor = vid;
> > + onboard_hub_id_table[i].idProduct = pid;
> > + onboard_hub_id_table[i].match_flags =
> USB_DEVICE_ID_MATCH_DEVICE;
> > +}
> > +
> > +static int onboard_hub_parse_idtable(struct device_node *np)
> > +{
> > + int size = of_property_count_elems_of_size(np, "vidpid", sizeof(int));
> > + int ret, i;
> > + u16 *ids;
> > +
> > + if (!size || size % 2)
> > + return -EINVAL;
> > +
> > + ids = kzalloc(sizeof(u16) * size, GFP_KERNEL);
> > + if (!ids)
> > + return -ENOMEM;
> > +
> > + ret = of_property_read_u16_array(np, "vidpid", ids, size);
> > + if (ret) {
> > + kfree(ids);
> > + return -EINVAL;
> > + }
> > +
> > + for (i = 0; i < size; i+=2)
> > + onboard_hub_add_idtable(ids[i], ids[i+1]);
> > +
> > + kfree(ids);
> > +
> > + return 0;
> > +}
> > +
> > static ssize_t always_powered_in_suspend_show(struct device *dev,
> struct device_attribute *attr,
> > char *buf)
> > {
> > @@ -210,6 +263,10 @@ static int onboard_hub_probe(struct
> platform_device *pdev)
> > struct onboard_hub *hub;
> > int err;
> >
> > + err = onboard_hub_parse_idtable(dev->of_node);
> > + if (err)
> > + return err;
> > +
> > hub = devm_kzalloc(dev, sizeof(*hub), GFP_KERNEL);
> > if (!hub)
> > return -ENOMEM;
> > @@ -378,15 +435,6 @@ static void
> onboard_hub_usbdev_disconnect(struct usb_device *udev)
> > onboard_hub_remove_usbdev(hub, udev);
> > }
> >
> > -static const struct usb_device_id onboard_hub_id_table[] = {
> > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1
> */
> > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1
> */
> > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x0414) }, /* RTS5414 USB 3.2
> */
> > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1
> */
> > - {}
> > -};
> > -MODULE_DEVICE_TABLE(usb, onboard_hub_id_table);
> > -
> > static struct usb_device_driver onboard_hub_usbdev_driver = {
> > .name = "onboard-usb-hub",
> > .probe = onboard_hub_usbdev_probe,
>
> I see multiple issues with this approach:
>
> 1. The new device tree property 'vidpid'. It is (or should be) redundant
> with the compatible string, I very much doubt you could convince DT
> maintainers to add it.
> 2. You don't want to modify the driver to enabled support for new USB hubs.
> That means you would have to use a compatible string that is already in
> the driver, but which doesn't match the VID:PID of the hub. While this
> might work it's a hack.
> 3. If the USB hub is probed before the platform device it won't use this
> driver because the VID:PID isn't in the device id table.
This is another topic which I don't know if discuss or not,
What is power state requirement of hub for this driver ?
Do it expect hub power off when system enter linux kernel stage ?
If so, all this driver may not work as hub may enumerated before this driver load.
> 4. Possible race conditions when changing the device id table on the fly
If hub is default power off, there is no race.
Powered by blists - more mailing lists