Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On 3/19/24 10:17 PM, Janne Grunau wrote:
Hi,
sorry for the abysmal delay in response.
On Mon, Mar 18, 2024 at 03:17:33PM +0100, Marek Vasut wrote:
On 3/18/24 8:33 AM, Janne Grunau wrote:
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
This could be static void without return 0 at the end.
the return is there to break out of the while loop on parsing errors in a
single statement. This probably won't be necessary after using strsep and
sscanf in the parsing function but see below.
Ahh, now I see it. But then, shouldn't this return -ENODEV here already ?
It returns 0 so that parsing errors in the blocklist do not result
in blocking every USB device. That looked to me like the
less bad error behavior to me.
In unix , 0 is considered success and non-zero failure .
How about this:
- Return -EINVAL here (parsing failed)
If we return 0 / negated errors we do not need this function and we can
simply report the parsing error when usb_device_is_blocked() return
-EINVAL.
- Instead of 'return 1' in usb_device_is_blocked() return -ENODEV
- In usb_select_config(), check
if usb_device_is_blocked returned 0, no device blocked OK
if usb_device_is_blocked returned -ENODEV, device blocked,
return -ENODEV
if usb_device_is_blocked returned any other error, parsing error
return that error
I think the preferable option is to ignore parsing errors. If we
would propagate the error the result would be that every USB device is
ignored.
Good point.
What do you think ?
Fine by me, -EINVAL makes the parsing error reporting less awkward. Only
open question is what should happen on parsing errors.
I agree, ignore/skip the parsing errors and report to user that
something couldn't be parsed, that works.
locally modified and ready to resend once we agree on the behavior on
parsing errors
Thanks, I think we are in agreement.
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On Mon, Mar 18, 2024 at 03:17:33PM +0100, Marek Vasut wrote:
> On 3/18/24 8:33 AM, Janne Grunau wrote:
>
> > +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
> > +{
> > + printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
> > + blocklist);
> > + return 0;
>
> This could be static void without return 0 at the end.
> >>>
> >>> the return is there to break out of the while loop on parsing errors in a
> >>> single statement. This probably won't be necessary after using strsep and
> >>> sscanf in the parsing function but see below.
> >>
> >> Ahh, now I see it. But then, shouldn't this return -ENODEV here already ?
> >
> > It returns 0 so that parsing errors in the blocklist do not result
> > in blocking every USB device. That looked to me like the
> > less bad error behavior to me.
>
> In unix , 0 is considered success and non-zero failure .
>
> How about this:
>
> - Return -EINVAL here (parsing failed)
If we return 0 / negated errors we do not need this function and we can
simply report the parsing error when usb_device_is_blocked() return
-EINVAL.
> - Instead of 'return 1' in usb_device_is_blocked() return -ENODEV
> - In usb_select_config(), check
>if usb_device_is_blocked returned 0, no device blocked OK
>if usb_device_is_blocked returned -ENODEV, device blocked,
> return -ENODEV
>if usb_device_is_blocked returned any other error, parsing error
> return that error
I think the preferable option is to ignore parsing errors. If we
would propagate the error the result would be that every USB device is
ignored.
> What do you think ?
Fine by me, -EINVAL makes the parsing error reporting less awkward. Only
open question is what should happen on parsing errors.
locally modified and ready to resend once we agree on the behavior on
parsing errors
Janne
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On 3/18/24 8:33 AM, Janne Grunau wrote:
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
This could be static void without return 0 at the end.
the return is there to break out of the while loop on parsing errors in a
single statement. This probably won't be necessary after using strsep and
sscanf in the parsing function but see below.
Ahh, now I see it. But then, shouldn't this return -ENODEV here already ?
It returns 0 so that parsing errors in the blocklist do not result
in blocking every USB device. That looked to me like the
less bad error behavior to me.
In unix , 0 is considered success and non-zero failure .
How about this:
- Return -EINVAL here (parsing failed)
- Instead of 'return 1' in usb_device_is_blocked() return -ENODEV
- In usb_select_config(), check
if usb_device_is_blocked returned 0, no device blocked OK
if usb_device_is_blocked returned -ENODEV, device blocked,
return -ENODEV
if usb_device_is_blocked returned any other error, parsing error
return that error
What do you think ?
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+ ulong vid, pid;
+ char *end;
+ const char *block_str = env_get("usb_blocklist");
+ const char *cur = block_str;
+
+ /* parse "usb_blocklist" strictly */
+ while (cur && cur[0] != '\0') {
Have a look at strsep() , namely strsep(block_str, ","); This will split
the string up for you at "," delimiters.
Example is in drivers/dfu/dfu.c dfu_config_interfaces() .
strsep() is probably a good idea even if it alone won't make the code that much
simpler for strict parsing.
And then, on each token, you can try and run sscanf(token, "%04x:%04x",
vid, pid);, that will parse the token format for you. See e.g.
test/lib/sscanf.c for further examples.
That should simplify the parsing a lot.
It would but sscanf() is optional and is only selected by CONFIG_XEN so I
assumed there would be concerns over binary size increase if USB_HOST would
require sscanf.
Good point, lets postpone sscanf() . Also, looking at it, sscanf would
work for numbers, but not for the special characters. So ... do you want
to try tweaking this further with strsep() or shall we go with this
implementation ?
I started converting it to strsep. It makes the intent clearer but it doesn't
simplify the implementation much. strsep() has the disadvantage that
it can't work in place and needs to copy the string. If we go with strsep
I would look into parsing the list once at USB init time and use a list of
numeric vendor, product ID pairs at device probe time.
If have a slight preference for the current implementation (with ignore
or deny instead of blocklist) as long as the list remains short.
OK, we can always improve this later, I would also like this
functionality in soon-ish.
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
Hej, On Sun, Mar 17, 2024, at 22:39, E Shattow wrote: > Should be usb_denylist, since "block devices" is ambiguous meaning if > it is a list of data chunks (blocks) or if it blocks (deny). There is > no question what a denylist is. I briefly thought of that concluded that the spelling "blocklist" removed the ambiguity. I not sure about "denylist". It sounds weird to me in the context of probing USB devices. I'd rather use "ignorelist" as alternative. Janne
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On Mon, Mar 18, 2024, at 06:06, Marek Vasut wrote:
> On 3/17/24 7:15 PM, Janne Grunau wrote:
>> Hej,
>
> Hi,
>
>> On Sun, Mar 17, 2024, at 17:18, Marek Vasut wrote:
>>> On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote:
From: Janne Grunau
Add the environment variable "usb_blocklist" to prevent USB devices
listed in it from being used. This allows to ignore devices which
trigger bugs in u-boot's USB stack or are undesirable for other reasons.
Devices emulating keyboards are one example. U-boot currently supports
only one USB keyboard device. Most commonly, people run into this with
Yubikeys, so let's ignore those in the default environment.
Based on previous USB keyboard specific patches for the same purpose.
Link:
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Signed-off-by: Janne Grunau
---
common/usb.c | 56
+++
doc/usage/environment.rst | 12 ++
include/env_default.h | 11 ++
3 files changed, 79 insertions(+)
diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..73af5be066 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device
*dev, int addr, bool do_read,
return 0;
}
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
>>>
>>> This could be static void without return 0 at the end.
>>
>> the return is there to break out of the while loop on parsing errors in a
>> single statement. This probably won't be necessary after using strsep and
>> sscanf in the parsing function but see below.
>
> Ahh, now I see it. But then, shouldn't this return -ENODEV here already ?
It returns 0 so that parsing errors in the blocklist do not result
in blocking every USB device. That looked to me like the
less bad error behavior to me.
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+ ulong vid, pid;
+ char *end;
+ const char *block_str = env_get("usb_blocklist");
+ const char *cur = block_str;
+
+ /* parse "usb_blocklist" strictly */
+ while (cur && cur[0] != '\0') {
>>>
>>> Have a look at strsep() , namely strsep(block_str, ","); This will split
>>> the string up for you at "," delimiters.
>>>
>>> Example is in drivers/dfu/dfu.c dfu_config_interfaces() .
>>
>> strsep() is probably a good idea even if it alone won't make the code that
>> much simpler for strict parsing.
>>
>>> And then, on each token, you can try and run sscanf(token, "%04x:%04x",
>>> vid, pid);, that will parse the token format for you. See e.g.
>>> test/lib/sscanf.c for further examples.
>>>
>>> That should simplify the parsing a lot.
>>
>> It would but sscanf() is optional and is only selected by CONFIG_XEN so I
>> assumed there would be concerns over binary size increase if USB_HOST would
>> require sscanf.
>
> Good point, lets postpone sscanf() . Also, looking at it, sscanf would
> work for numbers, but not for the special characters. So ... do you want
> to try tweaking this further with strsep() or shall we go with this
> implementation ?
I started converting it to strsep. It makes the intent clearer but it doesn't
simplify the implementation much. strsep() has the disadvantage that
it can't work in place and needs to copy the string. If we go with strsep
I would look into parsing the list once at USB init time and use a list of
numeric vendor, product ID pairs at device probe time.
If have a slight preference for the current implementation (with ignore
or deny instead of blocklist) as long as the list remains short.
Janne
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On 3/17/24 7:15 PM, Janne Grunau wrote:
Hej,
Hi,
On Sun, Mar 17, 2024, at 17:18, Marek Vasut wrote:
On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote:
From: Janne Grunau
Add the environment variable "usb_blocklist" to prevent USB devices
listed in it from being used. This allows to ignore devices which
trigger bugs in u-boot's USB stack or are undesirable for other reasons.
Devices emulating keyboards are one example. U-boot currently supports
only one USB keyboard device. Most commonly, people run into this with
Yubikeys, so let's ignore those in the default environment.
Based on previous USB keyboard specific patches for the same purpose.
Link:
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Signed-off-by: Janne Grunau
---
common/usb.c | 56
+++
doc/usage/environment.rst | 12 ++
include/env_default.h | 11 ++
3 files changed, 79 insertions(+)
diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..73af5be066 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
int addr, bool do_read,
return 0;
}
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
This could be static void without return 0 at the end.
the return is there to break out of the while loop on parsing errors in a
single statement. This probably won't be necessary after using strsep and
sscanf in the parsing function but see below.
Ahh, now I see it. But then, shouldn't this return -ENODEV here already ?
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+ ulong vid, pid;
+ char *end;
+ const char *block_str = env_get("usb_blocklist");
+ const char *cur = block_str;
+
+ /* parse "usb_blocklist" strictly */
+ while (cur && cur[0] != '\0') {
Have a look at strsep() , namely strsep(block_str, ","); This will split
the string up for you at "," delimiters.
Example is in drivers/dfu/dfu.c dfu_config_interfaces() .
strsep() is probably a good idea even if it alone won't make the code that much
simpler for strict parsing.
And then, on each token, you can try and run sscanf(token, "%04x:%04x",
vid, pid);, that will parse the token format for you. See e.g.
test/lib/sscanf.c for further examples.
That should simplify the parsing a lot.
It would but sscanf() is optional and is only selected by CONFIG_XEN so I
assumed there would be concerns over binary size increase if USB_HOST would
require sscanf.
Good point, lets postpone sscanf() . Also, looking at it, sscanf would
work for numbers, but not for the special characters. So ... do you want
to try tweaking this further with strsep() or shall we go with this
implementation ?
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
Should be usb_denylist, since "block devices" is ambiguous meaning if
it is a list of data chunks (blocks) or if it blocks (deny). There is
no question what a denylist is.
On Sun, Mar 17, 2024 at 4:07 AM Janne Grunau via B4 Relay
wrote:
>
> From: Janne Grunau
>
> Add the environment variable "usb_blocklist" to prevent USB devices
> listed in it from being used. This allows to ignore devices which
> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
> Devices emulating keyboards are one example. U-boot currently supports
> only one USB keyboard device. Most commonly, people run into this with
> Yubikeys, so let's ignore those in the default environment.
>
> Based on previous USB keyboard specific patches for the same purpose.
>
> Link:
> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
> Signed-off-by: Janne Grunau
> ---
> common/usb.c | 56
> +++
> doc/usage/environment.rst | 12 ++
> include/env_default.h | 11 ++
> 3 files changed, 79 insertions(+)
>
> diff --git a/common/usb.c b/common/usb.c
> index 836506dcd9..73af5be066 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
> int addr, bool do_read,
> return 0;
> }
>
> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
> +{
> + printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
> + blocklist);
> + return 0;
> +}
> +
> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
> +{
> + ulong vid, pid;
> + char *end;
> + const char *block_str = env_get("usb_blocklist");
> + const char *cur = block_str;
> +
> + /* parse "usb_blocklist" strictly */
> + while (cur && cur[0] != '\0') {
> + vid = simple_strtoul(cur, , 0);
> + if (cur == end || end[0] != ':')
> + return usb_blocklist_parse_error(block_str,
> +cur - block_str);
> +
> + cur = end + 1;
> + pid = simple_strtoul(cur, , 0);
> + if (cur == end && end[0] != '*')
> + return usb_blocklist_parse_error(block_str,
> +cur - block_str);
> +
> + if (end[0] == '*') {
> + /* use out of range idProduct as wildcard indication
> */
> + pid = U16_MAX + 1;
> + end++;
> + }
> + if (end[0] != ',' && end[0] != '\0')
> + return usb_blocklist_parse_error(block_str,
> +cur - block_str);
> +
> + if (id_vendor == vid && (pid > U16_MAX || id_product == pid))
> {
> + printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
> + id_product);
> + debug("Ignoring USB device 0x%x:0x%x\n",id_vendor,
> + id_product);
> + return 1;
> + }
> + if (end[0] == '\0')
> + break;
> + cur = end + 1;
> + }
> +
> + return 0;
> +}
> +
> int usb_select_config(struct usb_device *dev)
> {
> unsigned char *tmpbuf = NULL;
> @@ -1099,6 +1150,11 @@ int usb_select_config(struct usb_device *dev)
> le16_to_cpus(>descriptor.idProduct);
> le16_to_cpus(>descriptor.bcdDevice);
>
> + /* ignore devices from usb_blocklist */
> + if (usb_device_is_blocked(dev->descriptor.idVendor,
> + dev->descriptor.idProduct))
> + return -ENODEV;
> +
> /*
> * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
> * about this first Get Descriptor request. If there are any other
> diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> index ebf75fa948..e42702adb2 100644
> --- a/doc/usage/environment.rst
> +++ b/doc/usage/environment.rst
> @@ -366,6 +366,18 @@ tftpwindowsize
> This means the count of blocks we can receive before
> sending ack to server.
>
> +usb_blocklist
> +Block USB devices from being bound to an USB device driver. This can be
> used
> +to ignore devices which causes crashes in u-boot's USB stack or are
> +undesirable for other reasons.
> +The default environment blocks Yubico devices since they emulate USB
> +keyboards. U-boot currently supports only a single USB keyboard device
> and
> +it is undesirable that a Yubikey is used as keyboard.
> +Devices are matched by idVendor and idProduct. The variable contains a
> comma
> +separated list of idVendor:idProduct pairs as hexadecimal numbers joined
> +by a colon. '*' functions as a wildcard for idProduct
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
Hej,
On Sun, Mar 17, 2024, at 17:18, Marek Vasut wrote:
> On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote:
>> From: Janne Grunau
>>
>> Add the environment variable "usb_blocklist" to prevent USB devices
>> listed in it from being used. This allows to ignore devices which
>> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
>> Devices emulating keyboards are one example. U-boot currently supports
>> only one USB keyboard device. Most commonly, people run into this with
>> Yubikeys, so let's ignore those in the default environment.
>>
>> Based on previous USB keyboard specific patches for the same purpose.
>>
>> Link:
>> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
>> Signed-off-by: Janne Grunau
>> ---
>> common/usb.c | 56
>> +++
>> doc/usage/environment.rst | 12 ++
>> include/env_default.h | 11 ++
>> 3 files changed, 79 insertions(+)
>>
>> diff --git a/common/usb.c b/common/usb.c
>> index 836506dcd9..73af5be066 100644
>> --- a/common/usb.c
>> +++ b/common/usb.c
>> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
>> int addr, bool do_read,
>> return 0;
>> }
>>
>> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
>> +{
>> +printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
>> + blocklist);
>> +return 0;
>
> This could be static void without return 0 at the end.
the return is there to break out of the while loop on parsing errors in a
single statement. This probably won't be necessary after using strsep and
sscanf in the parsing function but see below.
>> +}
>> +
>> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
>> +{
>> +ulong vid, pid;
>> +char *end;
>> +const char *block_str = env_get("usb_blocklist");
>> +const char *cur = block_str;
>> +
>> +/* parse "usb_blocklist" strictly */
>> +while (cur && cur[0] != '\0') {
>
> Have a look at strsep() , namely strsep(block_str, ","); This will split
> the string up for you at "," delimiters.
>
> Example is in drivers/dfu/dfu.c dfu_config_interfaces() .
strsep() is probably a good idea even if it alone won't make the code that much
simpler for strict parsing.
> And then, on each token, you can try and run sscanf(token, "%04x:%04x",
> vid, pid);, that will parse the token format for you. See e.g.
> test/lib/sscanf.c for further examples.
>
> That should simplify the parsing a lot.
It would but sscanf() is optional and is only selected by CONFIG_XEN so I
assumed there would be concerns over binary size increase if USB_HOST would
require sscanf.
Janne
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote:
From: Janne Grunau
Add the environment variable "usb_blocklist" to prevent USB devices
listed in it from being used. This allows to ignore devices which
trigger bugs in u-boot's USB stack or are undesirable for other reasons.
Devices emulating keyboards are one example. U-boot currently supports
only one USB keyboard device. Most commonly, people run into this with
Yubikeys, so let's ignore those in the default environment.
Based on previous USB keyboard specific patches for the same purpose.
Link:
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Signed-off-by: Janne Grunau
---
common/usb.c | 56 +++
doc/usage/environment.rst | 12 ++
include/env_default.h | 11 ++
3 files changed, 79 insertions(+)
diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..73af5be066 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
int addr, bool do_read,
return 0;
}
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
This could be static void without return 0 at the end.
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+ ulong vid, pid;
+ char *end;
+ const char *block_str = env_get("usb_blocklist");
+ const char *cur = block_str;
+
+ /* parse "usb_blocklist" strictly */
+ while (cur && cur[0] != '\0') {
Have a look at strsep() , namely strsep(block_str, ","); This will split
the string up for you at "," delimiters.
Example is in drivers/dfu/dfu.c dfu_config_interfaces() .
And then, on each token, you can try and run sscanf(token, "%04x:%04x",
vid, pid);, that will parse the token format for you. See e.g.
test/lib/sscanf.c for further examples.
That should simplify the parsing a lot.
[...]
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
On 3/17/24 12:34 PM, Janne Grunau wrote:
Hej,
On Sun, Mar 17, 2024, at 12:07, Janne Grunau via B4 Relay wrote:
From: Janne Grunau
Add the environment variable "usb_blocklist" to prevent USB devices
listed in it from being used. This allows to ignore devices which
trigger bugs in u-boot's USB stack or are undesirable for other reasons.
Devices emulating keyboards are one example. U-boot currently supports
only one USB keyboard device. Most commonly, people run into this with
Yubikeys, so let's ignore those in the default environment.
Based on previous USB keyboard specific patches for the same purpose.
Link:
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Signed-off-by: Janne Grunau
---
common/usb.c | 56
+++
doc/usage/environment.rst | 12 ++
include/env_default.h | 11 ++
3 files changed, 79 insertions(+)
diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..73af5be066 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device
*dev, int addr, bool do_read,
return 0;
}
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+ printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+ blocklist);
+ return 0;
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+ ulong vid, pid;
+ char *end;
+ const char *block_str = env_get("usb_blocklist");
+ const char *cur = block_str;
+
+ /* parse "usb_blocklist" strictly */
+ while (cur && cur[0] != '\0') {
+ vid = simple_strtoul(cur, , 0);
+ if (cur == end || end[0] != ':')
+ return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+ cur = end + 1;
+ pid = simple_strtoul(cur, , 0);
+ if (cur == end && end[0] != '*')
+ return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+ if (end[0] == '*') {
+ /* use out of range idProduct as wildcard indication */
+ pid = U16_MAX + 1;
+ end++;
+ }
+ if (end[0] != ',' && end[0] != '\0')
+ return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+ if (id_vendor == vid && (pid > U16_MAX || id_product == pid)) {
+ printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
+ id_product);
this is a leftover from testing, already removed locally
You could turn this into dev_dbg()
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
Hej,
On Sun, Mar 17, 2024, at 12:07, Janne Grunau via B4 Relay wrote:
> From: Janne Grunau
>
> Add the environment variable "usb_blocklist" to prevent USB devices
> listed in it from being used. This allows to ignore devices which
> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
> Devices emulating keyboards are one example. U-boot currently supports
> only one USB keyboard device. Most commonly, people run into this with
> Yubikeys, so let's ignore those in the default environment.
>
> Based on previous USB keyboard specific patches for the same purpose.
>
> Link:
> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
> Signed-off-by: Janne Grunau
> ---
> common/usb.c | 56
> +++
> doc/usage/environment.rst | 12 ++
> include/env_default.h | 11 ++
> 3 files changed, 79 insertions(+)
>
> diff --git a/common/usb.c b/common/usb.c
> index 836506dcd9..73af5be066 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device
> *dev, int addr, bool do_read,
> return 0;
> }
>
> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
> +{
> + printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
> +blocklist);
> + return 0;
> +}
> +
> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
> +{
> + ulong vid, pid;
> + char *end;
> + const char *block_str = env_get("usb_blocklist");
> + const char *cur = block_str;
> +
> + /* parse "usb_blocklist" strictly */
> + while (cur && cur[0] != '\0') {
> + vid = simple_strtoul(cur, , 0);
> + if (cur == end || end[0] != ':')
> + return usb_blocklist_parse_error(block_str,
> + cur - block_str);
> +
> + cur = end + 1;
> + pid = simple_strtoul(cur, , 0);
> + if (cur == end && end[0] != '*')
> + return usb_blocklist_parse_error(block_str,
> + cur - block_str);
> +
> + if (end[0] == '*') {
> + /* use out of range idProduct as wildcard indication */
> + pid = U16_MAX + 1;
> + end++;
> + }
> + if (end[0] != ',' && end[0] != '\0')
> + return usb_blocklist_parse_error(block_str,
> + cur - block_str);
> +
> + if (id_vendor == vid && (pid > U16_MAX || id_product == pid)) {
> + printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
> + id_product);
this is a leftover from testing, already removed locally
Janne
