[v3] usb: gadget: add WebUSB landing page support

Commit Message

Jó Ágila Bitsch Jan. 1, 2023, 4:59 p.m. UTC

There is a custom (non-USB IF) extension to the USB standard:

https://wicg.github.io/webusb/

This specification is published under the W3C Community Contributor
Agreement, which in particular allows to implement the specification
without any royalties.

The specification allows USB gadgets to announce an URL to landing
page and describes a Javascript interface for websites to interact
with the USB gadget, if the user allows it. It is currently
supported by Chromium-based browsers, such as Chrome, Edge and
Opera on all major operating systems including Linux.

This patch adds optional support for Linux-based USB gadgets
wishing to expose such a landing page.

During device enumeration, a host recognizes that the announced
USB version is at least 2.01, which means, that there are BOS
descriptors available. The device than announces WebUSB support
using a platform device capability. This includes a vendor code
under which the landing page URL can be retrieved using a
vendor-specific request.

Previously, the BOS descriptors would unconditionally include an
LPM related descriptor, as BOS descriptors were only ever sent
when the device was LPM capable. As this is no longer the case,
this patch puts this descriptor behind a lpm_capable condition.

Usage is modeled after os_desc descriptors:
echo 1 > webusb/use
echo "https://www.kernel.org" > webusb/landingPage

lsusb will report the device with the following lines:
  Platform Device Capability:
    bLength                24
    bDescriptorType        16
    bDevCapabilityType      5
    bReserved               0
    PlatformCapabilityUUID    {3408b638-09a9-47a0-8bfd-a0768815b665}
      WebUSB:
        bcdVersion    1.00
        bVendorCode      0
        iLandingPage     1 https://www.kernel.org

Signed-off-by: Jó Ágila Bitsch <[e-mail]>
---
v2 -> V3: improved commit message to include additional condition in desc_bos as per comment 
	  by Alan Stern
V1 -> V2: cleaned up coding style, made URL scheme comparison case insensitive, addressed review
          comments by Alan Stern
V0 -> V1: use sysfs_emit instead of sprintf and use lock in webusb_bcdVersion_store, addressed review
	  comments by Christophe JAILLET

 Documentation/ABI/testing/configfs-usb-gadget |  13 ++
 drivers/usb/gadget/composite.c                |  95 ++++++++++--
 drivers/usb/gadget/configfs.c                 | 145 ++++++++++++++++++
 include/linux/usb/composite.h                 |   6 +
 include/uapi/linux/usb/ch9.h                  |  33 ++++
 5 files changed, 282 insertions(+), 10 deletions(-)

Comments

Andrzej Pietrasiewicz Jan. 9, 2023, 5:49 p.m. UTC | #1

Hi Jó Ágila Bitsch,

W dniu 1.01.2023 o 17:59, Jó Ágila Bitsch pisze:
> 
> There is a custom (non-USB IF) extension to the USB standard:
> 
> https://wicg.github.io/webusb/
> 
> This specification is published under the W3C Community Contributor
> Agreement, which in particular allows to implement the specification
> without any royalties.
> 
> The specification allows USB gadgets to announce an URL to landing
> page and describes a Javascript interface for websites to interact
> with the USB gadget, if the user allows it. It is currently
> supported by Chromium-based browsers, such as Chrome, Edge and
> Opera on all major operating systems including Linux.
> 

I envision an "evil" scenario, where you don't own your USB device any more.
Instead you are required to pay a subscription to keep accessing it
because its driver is somewhere on the net. I purposedly have put
evil in quotes, because such a scenario might be considered preferred
by some people. Maybe there are advantages, too, like not having to worry
about correct drivers. Anyway, the kernel is about mechanism, not policy,
so I'm probably in no position to judge such things.

Please see below.

> This patch adds optional support for Linux-based USB gadgets
> wishing to expose such a landing page.
> 
> During device enumeration, a host recognizes that the announced
> USB version is at least 2.01, which means, that there are BOS
> descriptors available. The device than announces WebUSB support
> using a platform device capability. This includes a vendor code
> under which the landing page URL can be retrieved using a
> vendor-specific request.
> 
> Previously, the BOS descriptors would unconditionally include an
> LPM related descriptor, as BOS descriptors were only ever sent
> when the device was LPM capable. As this is no longer the case,
> this patch puts this descriptor behind a lpm_capable condition.
> 
> Usage is modeled after os_desc descriptors:
> echo 1 > webusb/use
> echo "https://www.kernel.org" > webusb/landingPage
> 
> lsusb will report the device with the following lines:
>    Platform Device Capability:
>      bLength                24
>      bDescriptorType        16
>      bDevCapabilityType      5
>      bReserved               0
>      PlatformCapabilityUUID    {3408b638-09a9-47a0-8bfd-a0768815b665}
>        WebUSB:
>          bcdVersion    1.00
>          bVendorCode      0
>          iLandingPage     1 https://www.kernel.org
> 
> Signed-off-by: Jó Ágila Bitsch <[e-mail]>
> ---
> v2 -> V3: improved commit message to include additional condition in desc_bos as per comment
> 	  by Alan Stern
> V1 -> V2: cleaned up coding style, made URL scheme comparison case insensitive, addressed review
>            comments by Alan Stern
> V0 -> V1: use sysfs_emit instead of sprintf and use lock in webusb_bcdVersion_store, addressed review
> 	  comments by Christophe JAILLET
> 
>   Documentation/ABI/testing/configfs-usb-gadget |  13 ++
>   drivers/usb/gadget/composite.c                |  95 ++++++++++--
>   drivers/usb/gadget/configfs.c                 | 145 ++++++++++++++++++
>   include/linux/usb/composite.h                 |   6 +
>   include/uapi/linux/usb/ch9.h                  |  33 ++++
>   5 files changed, 282 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget b/Documentation/ABI/testing/configfs-usb-gadget
> index b7943aa7e997..8399e0f0ed1c 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget
> +++ b/Documentation/ABI/testing/configfs-usb-gadget
> @@ -143,3 +143,16 @@ Description:
>   		qw_sign		an identifier to be reported as "OS String"
>   				proper
>   		=============	===============================================
> +
> +What:		/config/usb-gadget/gadget/webusb
> +Date:		Dec 2022
> +KernelVersion:	6.2
> +Description:
> +		This group contains "WebUSB" extension handling attributes.
> +
> +		=============	===============================================
> +		use		flag turning "WebUSB" support on/off
> +		bcdVersion	bcd WebUSB specification version number
> +		b_vendor_code	one-byte value used for custom per-device
> +		landing_page	UTF-8 encoded URL of the device's landing page
> +		=============	===============================================
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 403563c06477..9b209e69442d 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -14,6 +14,7 @@
>   #include <linux/device.h>
>   #include <linux/utsname.h>
>   #include <linux/bitfield.h>
> +#include <linux/uuid.h>
>   
>   #include <linux/usb/composite.h>
>   #include <linux/usb/otg.h>
> @@ -713,14 +714,16 @@ static int bos_desc(struct usb_composite_dev *cdev)
>   	 * A SuperSpeed device shall include the USB2.0 extension descriptor
>   	 * and shall support LPM when operating in USB2.0 HS mode.
>   	 */
> -	usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> -	bos->bNumDeviceCaps++;
> -	le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
> -	usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
> -	usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> -	usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
> -	usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
> -					    USB_BESL_SUPPORT | besl);
> +	if (cdev->gadget->lpm_capable) {
> +		usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> +		bos->bNumDeviceCaps++;
> +		le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
> +		usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
> +		usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> +		usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
> +		usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
> +							USB_BESL_SUPPORT | besl);
> +	}
>   
>   	/*
>   	 * The Superspeed USB Capability descriptor shall be implemented by all
> @@ -821,6 +824,40 @@ static int bos_desc(struct usb_composite_dev *cdev)
>   		}
>   	}
>   
> +	/*
> +	 * Following the specifaction at:
> +	 * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
> +	 */
> +	if (cdev->use_webusb) {
> +		struct usb_webusb_cap_descriptor *webusb_cap;
> +		/*
> +		 * little endian PlatformCapablityUUID for WebUSB:
> +		 * 3408b638-09a9-47a0-8bfd-a0768815b665
> +		 */
> +		uuid_t webusb_uuid = UUID_INIT(0x38b60834, 0xa909, 0xa047, 0x8b, 0xfd, 0xa0, 0x76,
> +						0x88, 0x15, 0xb6, 0x65);
> +
> +		webusb_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> +		bos->bNumDeviceCaps++;
> +
> +		le16_add_cpu(&bos->wTotalLength, USB_DT_WEBUSB_SIZE);
> +		webusb_cap->bLength = USB_DT_WEBUSB_SIZE;
> +		webusb_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> +		webusb_cap->bDevCapabilityType = USB_PLAT_DEV_CAP_TYPE;
> +		webusb_cap->bReserved = 0;
> +		export_uuid(webusb_cap->UUID, &webusb_uuid);
> +		if (cdev->bcd_webusb_version != 0)
> +			webusb_cap->bcdVersion = cpu_to_le16(cdev->bcd_webusb_version);
> +		else
> +			webusb_cap->bcdVersion = cpu_to_le16(0x0100);
> +
> +		webusb_cap->bVendorCode = cdev->b_webusb_vendor_code;
> +		if (strnlen(cdev->landing_page, sizeof(cdev->landing_page)) > 0)
> +			webusb_cap->iLandingPage = 1;
> +		else
> +			webusb_cap->iLandingPage = 0;
> +	}
> +
>   	return le16_to_cpu(bos->wTotalLength);
>   }
>   
> @@ -1744,7 +1781,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>   					cdev->desc.bcdUSB = cpu_to_le16(0x0210);
>   				}
>   			} else {
> -				if (gadget->lpm_capable)
> +				if (gadget->lpm_capable || cdev->use_webusb)
>   					cdev->desc.bcdUSB = cpu_to_le16(0x0201);
>   				else
>   					cdev->desc.bcdUSB = cpu_to_le16(0x0200);
> @@ -1779,7 +1816,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>   			break;
>   		case USB_DT_BOS:
>   			if (gadget_is_superspeed(gadget) ||
> -			    gadget->lpm_capable) {
> +			    gadget->lpm_capable || cdev->use_webusb) {
>   				value = bos_desc(cdev);
>   				value = min(w_length, (u16) value);
>   			}
> @@ -2013,6 +2050,44 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>   			goto check_value;
>   		}
>   
> +		/*
> +		 * webusb descriptor handling, following:
> +		 * https://wicg.github.io/webusb/#device-requests
> +		 */
> +		if (cdev->use_webusb &&
> +		    (ctrl->bRequestType & USB_TYPE_VENDOR) &&
> +		    ctrl->wIndex == /* WEBUSB_GET_URL*/ 2 &&
> +		    ctrl->bRequest == cdev->b_webusb_vendor_code) {
> +			/*
> +			 * specification of the url descriptor in WebUSB:
> +			 * https://wicg.github.io/webusb/#webusb-descriptors
> +			 */
> +			u8		*buf;
> +			unsigned int	landing_page_length;
> +
> +			buf = cdev->req->buf;
> +			buf[1] = /* WEBUSB_URL*/ 3;

Why not #define WEBUSB_URL?

> +
> +			landing_page_length = strnlen(cdev->landing_page, 252);
> +			if (strncasecmp("https://", cdev->landing_page, 8) == 0) {

Maybe it's me, but it would be easier for me to understand why the "8" if the
comparison was in reverse order of arguments, like this:

strncasecmp(cdev->landing_page, "https://", 8)

because we want the entirety of "https://" compared, not its substring, so the
limit kind of applies to the landing_page in the first place.
Maybe the "8" (and "7" below) can be #define'd, too?

> +				buf[2] = 0x01;

What is this magic 0x01?

> +				memcpy(buf+3, cdev->landing_page+8, landing_page_length-8);

spaces around arithmetic operators?

> +				buf[0] = landing_page_length - 8 + 3;
> +			} else if (strncasecmp("http://", cdev->landing_page, 7) == 0) {
> +				buf[2] = 0x00;

Magic 0x00?

> +				memcpy(buf+3, cdev->landing_page+7, landing_page_length-7);
> +				buf[0] = landing_page_length - 7 + 3;
> +			} else {
> +				buf[2] = 0xFF;

Magic 0xFF? (plus I'd expect lowercase hex digits).

There's "URL Prefixes" table in 4.3.1 URL Descriptor. Why not define 
URL_PREFIX_HTTP/HTTPS/NONE?

> +				memcpy(buf+3, cdev->landing_page, landing_page_length);
> +				buf[0] = landing_page_length + 3;
> +			}
> +
> +			value = buf[0];
> +
> +			goto check_value;
> +		}
> +
>   		VDBG(cdev,
>   			"non-core control req%02x.%02x v%04x i%04x l%d\n",
>   			ctrl->bRequestType, ctrl->bRequest,
> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> index 96121d1c8df4..2b7f01a9efff 100644
> --- a/drivers/usb/gadget/configfs.c
> +++ b/drivers/usb/gadget/configfs.c
> @@ -39,6 +39,7 @@ struct gadget_info {
>   	struct config_group configs_group;
>   	struct config_group strings_group;
>   	struct config_group os_desc_group;
> +	struct config_group webusb_group;
>   
>   	struct mutex lock;
>   	struct usb_gadget_strings *gstrings[MAX_USB_STRING_LANGS + 1];
> @@ -50,6 +51,11 @@ struct gadget_info {
>   	bool use_os_desc;
>   	char b_vendor_code;
>   	char qw_sign[OS_STRING_QW_SIGN_LEN];
> +	bool use_webusb;
> +	u16 bcd_webusb_version;
> +	u8 b_webusb_vendor_code;
> +	char landing_page[255];

A #define for the size?

> +
>   	spinlock_t spinlock;
>   	bool unbind;
>   };
> @@ -780,6 +786,131 @@ static void gadget_strings_attr_release(struct config_item *item)
>   USB_CONFIG_STRING_RW_OPS(gadget_strings);
>   USB_CONFIG_STRINGS_LANG(gadget_strings, gadget_info);
>   
> +static inline struct gadget_info *webusb_item_to_gadget_info(
> +		struct config_item *item)
> +{
> +	return container_of(to_config_group(item),
> +			struct gadget_info, webusb_group);
> +}
> +
> +static ssize_t webusb_use_show(struct config_item *item, char *page)
> +{
> +	return sysfs_emit(page, "%d\n",
> +			webusb_item_to_gadget_info(item)->use_webusb);
> +}
> +
> +static ssize_t webusb_use_store(struct config_item *item, const char *page,
> +				 size_t len)
> +{
> +	struct gadget_info *gi = webusb_item_to_gadget_info(item);
> +	int ret;
> +	bool use;
> +
> +	mutex_lock(&gi->lock);
> +	ret = kstrtobool(page, &use);
> +	if (!ret) {
> +		gi->use_webusb = use;
> +		ret = len;
> +	}
> +	mutex_unlock(&gi->lock);
> +
> +	return ret;
> +}
> +
> +static ssize_t webusb_bcdVersion_show(struct config_item *item, char *page)
> +{
> +	return sysfs_emit(page, "0x%04x\n",
> +					webusb_item_to_gadget_info(item)->bcd_webusb_version);
> +}
> +
> +static ssize_t webusb_bcdVersion_store(struct config_item *item,
> +		const char *page, size_t len)
> +{
> +	struct gadget_info *gi = webusb_item_to_gadget_info(item);
> +	u16 bcdVersion;
> +	int ret;
> +
> +	mutex_lock(&gi->lock);
> +	ret = kstrtou16(page, 0, &bcdVersion);
> +	if (ret)
> +		goto out;
> +	ret = is_valid_bcd(bcdVersion);
> +	if (ret)
> +		goto out;
> +
> +	gi->bcd_webusb_version = bcdVersion;
> +	ret = len;
> +
> +out:
> +	mutex_unlock(&gi->lock);
> +
> +	return ret;
> +}
> +
> +static ssize_t webusb_bVendorCode_show(struct config_item *item, char *page)
> +{
> +	return sysfs_emit(page, "0x%02x\n",
> +			webusb_item_to_gadget_info(item)->b_webusb_vendor_code);
> +}
> +
> +static ssize_t webusb_bVendorCode_store(struct config_item *item,
> +					   const char *page, size_t len)
> +{
> +	struct gadget_info *gi = webusb_item_to_gadget_info(item);
> +	int ret;
> +	u8 b_vendor_code;
> +
> +	mutex_lock(&gi->lock);
> +	ret = kstrtou8(page, 0, &b_vendor_code);
> +	if (!ret) {
> +		gi->b_webusb_vendor_code = b_vendor_code;
> +		ret = len;
> +	}
> +	mutex_unlock(&gi->lock);
> +
> +	return ret;
> +}
> +
> +static ssize_t webusb_landingPage_show(struct config_item *item, char *page)
> +{
> +	return sysfs_emit(page, "%s\n", webusb_item_to_gadget_info(item)->landing_page);
> +}
> +
> +static ssize_t webusb_landingPage_store(struct config_item *item, const char *page,
> +				     size_t len)
> +{
> +	struct gadget_info *gi = webusb_item_to_gadget_info(item);
> +	int l;
> +
> +	l = min(len, sizeof(gi->landing_page));
> +	if (page[l - 1] == '\n')
> +		--l;
> +
> +	mutex_lock(&gi->lock);
> +	memcpy(gi->landing_page, page, l);
> +	mutex_unlock(&gi->lock);
> +
> +	return len;
> +}
> +
> +CONFIGFS_ATTR(webusb_, use);
> +CONFIGFS_ATTR(webusb_, bVendorCode);
> +CONFIGFS_ATTR(webusb_, bcdVersion);
> +CONFIGFS_ATTR(webusb_, landingPage);
> +
> +static struct configfs_attribute *webusb_attrs[] = {
> +	&webusb_attr_use,
> +	&webusb_attr_bcdVersion,
> +	&webusb_attr_bVendorCode,
> +	&webusb_attr_landingPage,
> +	NULL,
> +};
> +
> +static struct config_item_type webusb_type = {
> +	.ct_attrs	= webusb_attrs,
> +	.ct_owner	= THIS_MODULE,
> +};
> +
>   static inline struct gadget_info *os_desc_item_to_gadget_info(
>   		struct config_item *item)
>   {
> @@ -1341,6 +1472,16 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
>   		gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
>   	}
>   
> +	if (gi->use_webusb) {
> +		cdev->use_webusb = true;
> +		cdev->bcd_webusb_version = gi->bcd_webusb_version;
> +		cdev->b_webusb_vendor_code = gi->b_webusb_vendor_code;
> +		memcpy(cdev->landing_page, gi->landing_page,
> +				strnlen(gi->landing_page,
> +						min(sizeof(cdev->landing_page),
> +							sizeof(gi->landing_page))));

checkpatch now allows 100 colums. Plus strnlen() looks indented too far to the
right.

The sizeofs are both 255. Are they ever expected to be different? Maybe not?
Then a #defined constant ensures they are equal. Then there's no need to find
a minimum among equal values.

Regards,

Andrzej

> +	}
> +
>   	if (gi->use_os_desc) {
>   		cdev->use_os_string = true;
>   		cdev->b_vendor_code = gi->b_vendor_code;
> @@ -1605,6 +1746,10 @@ static struct config_group *gadgets_make(
>   			&os_desc_type);
>   	configfs_add_default_group(&gi->os_desc_group, &gi->group);
>   
> +	config_group_init_type_name(&gi->webusb_group, "webusb",
> +			&webusb_type);
> +	configfs_add_default_group(&gi->webusb_group, &gi->group);
> +
>   	gi->composite.bind = configfs_do_nothing;
>   	gi->composite.unbind = configfs_do_nothing;
>   	gi->composite.suspend = NULL;
> diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
> index 43ac3fa760db..eb6fac5bbcde 100644
> --- a/include/linux/usb/composite.h
> +++ b/include/linux/usb/composite.h
> @@ -474,6 +474,12 @@ struct usb_composite_dev {
>   	struct usb_configuration	*os_desc_config;
>   	unsigned int			use_os_string:1;
>   
> +	/* WebUSB */
> +	u16				bcd_webusb_version;
> +	u8				b_webusb_vendor_code;
> +	char				landing_page[255];
> +	unsigned int			use_webusb:1;
> +
>   	/* private: */
>   	/* internals */
>   	unsigned int			suspended:1;
> diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
> index 31fcfa084e63..3a36550297bc 100644
> --- a/include/uapi/linux/usb/ch9.h
> +++ b/include/uapi/linux/usb/ch9.h
> @@ -947,6 +947,39 @@ struct usb_ss_container_id_descriptor {
>   
>   #define USB_DT_USB_SS_CONTN_ID_SIZE	20
>   
> +/*
> + * Platform Device Capability descriptor: Defines platform specific device
> + * capabilities
> + */
> +#define	USB_PLAT_DEV_CAP_TYPE	5
> +struct usb_plat_dev_cap_descriptor {
> +	__u8  bLength;
> +	__u8  bDescriptorType;
> +	__u8  bDevCapabilityType;
> +	__u8  bReserved;
> +	__u8  UUID[16];
> +} __attribute__((packed));
> +
> +#define USB_DT_USB_PLAT_DEV_CAP_SIZE	20
> +
> +/*
> + * WebUSB Platform Capability descriptor: A device announces support for the
> + * WebUSB command set by including the following Platform Descriptor in its
> + * Binary Object Store
> + * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
> + */
> +struct usb_webusb_cap_descriptor {
> +	__u8  bLength;
> +	__u8  bDescriptorType;
> +	__u8  bDevCapabilityType;
> +	__u8  bReserved;
> +	__u8  UUID[16];
> +	__u16 bcdVersion;
> +	__u8  bVendorCode;
> +	__u8  iLandingPage;
> +} __attribute__((packed));
> +#define USB_DT_WEBUSB_SIZE	(USB_DT_USB_PLAT_DEV_CAP_SIZE + 4)
> +
>   /*
>    * SuperSpeed Plus USB Capability descriptor: Defines the set of
>    * SuperSpeed Plus USB specific device level capabilities

Jó Ágila Bitsch Jan. 9, 2023, 11:19 p.m. UTC | #2

Hi Andrzej,

> > The specification allows USB gadgets to announce an URL to landing
> > page and describes a Javascript interface for websites to interact
> > with the USB gadget, if the user allows it. It is currently
> > supported by Chromium-based browsers, such as Chrome, Edge and
> > Opera on all major operating systems including Linux.
> >
>
> I envision an "evil" scenario, where you don't own your USB device any more.
> Instead you are required to pay a subscription to keep accessing it
> because its driver is somewhere on the net. I purposedly have put
> evil in quotes, because such a scenario might be considered preferred
> by some people. Maybe there are advantages, too, like not having to worry
> about correct drivers. Anyway, the kernel is about mechanism, not policy,
> so I'm probably in no position to judge such things.

Unfortunately, the "evil" devices you envision very much already exist.

When I was still working at university in the early 2010s, we were
working with a mobile EEG headset (>1000USD) with a USB receiver
dongle that would present itself as a USB HID device, but the
transferred frames would be AES encrypted, with the key based on a
proprietary transformation of the serial number of the device. At
first the library that decoded it was available as a binary blob for a
one-time payment, where you paid a hefty premium if you wanted access
to the raw readings. Later the manufacturer changed their business
model to subscription based, where the data had to flow through their
cloud service - with a recurring license fee. I don't know what
happened afterwards, as I left university.
This all was implemented based on libusb back then before people even
thought of WebUSB.

I'm sure there are other examples.

So, "evil" devices are already real and don't depend on WebUSB. The
motivating example "Web Drivers" from the specs
(https://wicg.github.io/webusb/#app-drivers) might potentially lead to
companies asking for subscription fees for their Web Driver, but as
long as there is no strong encryption in place, everyone would be free
to write an alternative driver. The mechanisms would not at all
preclude you from writing your own kernel or userspace driver using
e.g. libusb. A kernel driver would block access from the browser
though, as the browser cannot detach the kernel driver. (No such
command is currently defined in WebUSB.)  The JS implementation might
even guide you to understand aspects of the protocol used.

Citing from the motivating example in the design specs
(https://wicg.github.io/webusb/#app-updates-and-diagnostics):

> The landing page provides a way for the device manufacturer to direct the user to the right part of their website for help with their device.

The landing page mechanism in WebUSB improves discoverability for
devices, as Chrome and also lsusb directly point you to a webpage,
where the manufacturer or firmware producer can give more information
about the device. This webpage does not necessarily need to use a
WebUSB JS driver itself to access the device, but might just as well
point you to some git repository, where the hardware design and
firmware source are available.

For what it's worth, the pick-up of WebUSB I've seen has been mostly
in the maker scene with arduino, micro:bit and similar embedded
devices, where devices provide at least two interfaces: One for the OS
to claim and one for the Browser to claim.

Anyways, my biased opinion is that I would very much like a device to
tell me where I can find support documentation, updates and help also
when the device only implements standard interfaces that have kernel
drivers. I don't think that this URL will lead to more devices you can
only access with a subscription.

> Please see below.

Thanks for your excellent feedback. I answered inline.

> > This patch adds optional support for Linux-based USB gadgets
> > wishing to expose such a landing page.
> >
> > During device enumeration, a host recognizes that the announced
> > USB version is at least 2.01, which means, that there are BOS
> > descriptors available. The device than announces WebUSB support
> > using a platform device capability. This includes a vendor code
> > under which the landing page URL can be retrieved using a
> > vendor-specific request.
> >
> > Previously, the BOS descriptors would unconditionally include an
> > LPM related descriptor, as BOS descriptors were only ever sent
> > when the device was LPM capable. As this is no longer the case,
> > this patch puts this descriptor behind a lpm_capable condition.
> >
> > Usage is modeled after os_desc descriptors:
> > echo 1 > webusb/use
> > echo "https://www.kernel.org" > webusb/landingPage
> >
> > lsusb will report the device with the following lines:
> >    Platform Device Capability:
> >      bLength                24
> >      bDescriptorType        16
> >      bDevCapabilityType      5
> >      bReserved               0
> >      PlatformCapabilityUUID    {3408b638-09a9-47a0-8bfd-a0768815b665}
> >        WebUSB:
> >          bcdVersion    1.00
> >          bVendorCode      0
> >          iLandingPage     1 https://www.kernel.org
> >
> > Signed-off-by: Jó Ágila Bitsch <[e-mail]>
> > ---
> > v2 -> V3: improved commit message to include additional condition in desc_bos as per comment
> >         by Alan Stern
> > V1 -> V2: cleaned up coding style, made URL scheme comparison case insensitive, addressed review
> >            comments by Alan Stern
> > V0 -> V1: use sysfs_emit instead of sprintf and use lock in webusb_bcdVersion_store, addressed review
> >         comments by Christophe JAILLET
> >
> >   Documentation/ABI/testing/configfs-usb-gadget |  13 ++
> >   drivers/usb/gadget/composite.c                |  95 ++++++++++--
> >   drivers/usb/gadget/configfs.c                 | 145 ++++++++++++++++++
> >   include/linux/usb/composite.h                 |   6 +
> >   include/uapi/linux/usb/ch9.h                  |  33 ++++
> >   5 files changed, 282 insertions(+), 10 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/configfs-usb-gadget b/Documentation/ABI/testing/configfs-usb-gadget
> > index b7943aa7e997..8399e0f0ed1c 100644
> > --- a/Documentation/ABI/testing/configfs-usb-gadget
> > +++ b/Documentation/ABI/testing/configfs-usb-gadget
> > @@ -143,3 +143,16 @@ Description:
> >               qw_sign         an identifier to be reported as "OS String"
> >                               proper
> >               =============   ===============================================
> > +
> > +What:                /config/usb-gadget/gadget/webusb
> > +Date:                Dec 2022
> > +KernelVersion:       6.2
> > +Description:
> > +             This group contains "WebUSB" extension handling attributes.
> > +
> > +             =============   ===============================================
> > +             use             flag turning "WebUSB" support on/off
> > +             bcdVersion      bcd WebUSB specification version number
> > +             b_vendor_code   one-byte value used for custom per-device
> > +             landing_page    UTF-8 encoded URL of the device's landing page
> > +             =============   ===============================================
> > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> > index 403563c06477..9b209e69442d 100644
> > --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -14,6 +14,7 @@
> >   #include <linux/device.h>
> >   #include <linux/utsname.h>
> >   #include <linux/bitfield.h>
> > +#include <linux/uuid.h>
> >
> >   #include <linux/usb/composite.h>
> >   #include <linux/usb/otg.h>
> > @@ -713,14 +714,16 @@ static int bos_desc(struct usb_composite_dev *cdev)
> >        * A SuperSpeed device shall include the USB2.0 extension descriptor
> >        * and shall support LPM when operating in USB2.0 HS mode.
> >        */
> > -     usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> > -     bos->bNumDeviceCaps++;
> > -     le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
> > -     usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
> > -     usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> > -     usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
> > -     usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
> > -                                         USB_BESL_SUPPORT | besl);
> > +     if (cdev->gadget->lpm_capable) {
> > +             usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> > +             bos->bNumDeviceCaps++;
> > +             le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
> > +             usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
> > +             usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> > +             usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
> > +             usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
> > +                                                     USB_BESL_SUPPORT | besl);
> > +     }
> >
> >       /*
> >        * The Superspeed USB Capability descriptor shall be implemented by all
> > @@ -821,6 +824,40 @@ static int bos_desc(struct usb_composite_dev *cdev)
> >               }
> >       }
> >
> > +     /*
> > +      * Following the specifaction at:
> > +      * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
> > +      */
> > +     if (cdev->use_webusb) {
> > +             struct usb_webusb_cap_descriptor *webusb_cap;
> > +             /*
> > +              * little endian PlatformCapablityUUID for WebUSB:
> > +              * 3408b638-09a9-47a0-8bfd-a0768815b665
> > +              */
> > +             uuid_t webusb_uuid = UUID_INIT(0x38b60834, 0xa909, 0xa047, 0x8b, 0xfd, 0xa0, 0x76,
> > +                                             0x88, 0x15, 0xb6, 0x65);
> > +
> > +             webusb_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> > +             bos->bNumDeviceCaps++;
> > +
> > +             le16_add_cpu(&bos->wTotalLength, USB_DT_WEBUSB_SIZE);
> > +             webusb_cap->bLength = USB_DT_WEBUSB_SIZE;
> > +             webusb_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> > +             webusb_cap->bDevCapabilityType = USB_PLAT_DEV_CAP_TYPE;
> > +             webusb_cap->bReserved = 0;
> > +             export_uuid(webusb_cap->UUID, &webusb_uuid);
> > +             if (cdev->bcd_webusb_version != 0)
> > +                     webusb_cap->bcdVersion = cpu_to_le16(cdev->bcd_webusb_version);
> > +             else
> > +                     webusb_cap->bcdVersion = cpu_to_le16(0x0100);
> > +
> > +             webusb_cap->bVendorCode = cdev->b_webusb_vendor_code;
> > +             if (strnlen(cdev->landing_page, sizeof(cdev->landing_page)) > 0)
> > +                     webusb_cap->iLandingPage = 1;
> > +             else
> > +                     webusb_cap->iLandingPage = 0;
> > +     }
> > +
> >       return le16_to_cpu(bos->wTotalLength);
> >   }
> >
> > @@ -1744,7 +1781,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> >                                       cdev->desc.bcdUSB = cpu_to_le16(0x0210);
> >                               }
> >                       } else {
> > -                             if (gadget->lpm_capable)
> > +                             if (gadget->lpm_capable || cdev->use_webusb)
> >                                       cdev->desc.bcdUSB = cpu_to_le16(0x0201);
> >                               else
> >                                       cdev->desc.bcdUSB = cpu_to_le16(0x0200);
> > @@ -1779,7 +1816,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> >                       break;
> >               case USB_DT_BOS:
> >                       if (gadget_is_superspeed(gadget) ||
> > -                         gadget->lpm_capable) {
> > +                         gadget->lpm_capable || cdev->use_webusb) {
> >                               value = bos_desc(cdev);
> >                               value = min(w_length, (u16) value);
> >                       }
> > @@ -2013,6 +2050,44 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> >                       goto check_value;
> >               }
> >
> > +             /*
> > +              * webusb descriptor handling, following:
> > +              * https://wicg.github.io/webusb/#device-requests
> > +              */
> > +             if (cdev->use_webusb &&
> > +                 (ctrl->bRequestType & USB_TYPE_VENDOR) &&
> > +                 ctrl->wIndex == /* WEBUSB_GET_URL*/ 2 &&
> > +                 ctrl->bRequest == cdev->b_webusb_vendor_code) {
> > +                     /*
> > +                      * specification of the url descriptor in WebUSB:
> > +                      * https://wicg.github.io/webusb/#webusb-descriptors
> > +                      */
> > +                     u8              *buf;
> > +                     unsigned int    landing_page_length;
> > +
> > +                     buf = cdev->req->buf;
> > +                     buf[1] = /* WEBUSB_URL*/ 3;
>
> Why not #define WEBUSB_URL?

in a previous iteration, I had that exact #define in place, right
above. However, another reviewer found it pointless. Maybe I should
instead put the define into include/uapi/linux/usb/ch9.h

>
> > +
> > +                     landing_page_length = strnlen(cdev->landing_page, 252);
> > +                     if (strncasecmp("https://", cdev->landing_page, 8) == 0) {
>
> Maybe it's me, but it would be easier for me to understand why the "8" if the
> comparison was in reverse order of arguments, like this:
>
> strncasecmp(cdev->landing_page, "https://", 8)

I agree, this makes the 8 clearer.

> because we want the entirety of "https://" compared, not its substring, so the
> limit kind of applies to the landing_page in the first place.
> Maybe the "8" (and "7" below) can be #define'd, too?

depends on the name, but maybe something like

#define WEBUSB_PREFIX_HTTPS "https://"
#define WEBUSB_PREFIX_HTTPS_LENGTH 8
#define WEBUSB_PREFIX_HTTPS_SCHEME_ID 0x01

would work defined with the struct in include/uapi/linux/usb/ch9.h

>
> > +                             buf[2] = 0x01;
>
> What is this magic 0x01?

The field is specified in
https://wicg.github.io/webusb/#url-descriptor and would be the
WEBUSB_PREFIX_HTTPS_SCHEME_ID from above.
But yes, I should #define this.

>
> > +                             memcpy(buf+3, cdev->landing_page+8, landing_page_length-8);
>
> spaces around arithmetic operators?

will do


> > +                             buf[0] = landing_page_length - 8 + 3;
> > +                     } else if (strncasecmp("http://", cdev->landing_page, 7) == 0) {
> > +                             buf[2] = 0x00;
>
> Magic 0x00?

This would then be WEBUSB_PREFIX_HTTP_SCHEME_ID

>
> > +                             memcpy(buf+3, cdev->landing_page+7, landing_page_length-7);
> > +                             buf[0] = landing_page_length - 7 + 3;
> > +                     } else {
> > +                             buf[2] = 0xFF;
>
> Magic 0xFF? (plus I'd expect lowercase hex digits).

This would then be WEBUSB_PREFIX_NONE_SCHEME_ID

> There's "URL Prefixes" table in 4.3.1 URL Descriptor. Why not define
> URL_PREFIX_HTTP/HTTPS/NONE?

will do

> > +                             memcpy(buf+3, cdev->landing_page, landing_page_length);
> > +                             buf[0] = landing_page_length + 3;
> > +                     }
> > +
> > +                     value = buf[0];
> > +
> > +                     goto check_value;
> > +             }
> > +
> >               VDBG(cdev,
> >                       "non-core control req%02x.%02x v%04x i%04x l%d\n",
> >                       ctrl->bRequestType, ctrl->bRequest,
> > diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> > index 96121d1c8df4..2b7f01a9efff 100644
> > --- a/drivers/usb/gadget/configfs.c
> > +++ b/drivers/usb/gadget/configfs.c
> > @@ -39,6 +39,7 @@ struct gadget_info {
> >       struct config_group configs_group;
> >       struct config_group strings_group;
> >       struct config_group os_desc_group;
> > +     struct config_group webusb_group;
> >
> >       struct mutex lock;
> >       struct usb_gadget_strings *gstrings[MAX_USB_STRING_LANGS + 1];
> > @@ -50,6 +51,11 @@ struct gadget_info {
> >       bool use_os_desc;
> >       char b_vendor_code;
> >       char qw_sign[OS_STRING_QW_SIGN_LEN];
> > +     bool use_webusb;
> > +     u16 bcd_webusb_version;
> > +     u8 b_webusb_vendor_code;
> > +     char landing_page[255];
>
> A #define for the size?

will do

>
> > +
> >       spinlock_t spinlock;
> >       bool unbind;
> >   };
> > @@ -780,6 +786,131 @@ static void gadget_strings_attr_release(struct config_item *item)
> >   USB_CONFIG_STRING_RW_OPS(gadget_strings);
> >   USB_CONFIG_STRINGS_LANG(gadget_strings, gadget_info);
> >
> > +static inline struct gadget_info *webusb_item_to_gadget_info(
> > +             struct config_item *item)
> > +{
> > +     return container_of(to_config_group(item),
> > +                     struct gadget_info, webusb_group);
> > +}
> > +
> > +static ssize_t webusb_use_show(struct config_item *item, char *page)
> > +{
> > +     return sysfs_emit(page, "%d\n",
> > +                     webusb_item_to_gadget_info(item)->use_webusb);
> > +}
> > +
> > +static ssize_t webusb_use_store(struct config_item *item, const char *page,
> > +                              size_t len)
> > +{
> > +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
> > +     int ret;
> > +     bool use;
> > +
> > +     mutex_lock(&gi->lock);
> > +     ret = kstrtobool(page, &use);
> > +     if (!ret) {
> > +             gi->use_webusb = use;
> > +             ret = len;
> > +     }
> > +     mutex_unlock(&gi->lock);
> > +
> > +     return ret;
> > +}
> > +
> > +static ssize_t webusb_bcdVersion_show(struct config_item *item, char *page)
> > +{
> > +     return sysfs_emit(page, "0x%04x\n",
> > +                                     webusb_item_to_gadget_info(item)->bcd_webusb_version);
> > +}
> > +
> > +static ssize_t webusb_bcdVersion_store(struct config_item *item,
> > +             const char *page, size_t len)
> > +{
> > +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
> > +     u16 bcdVersion;
> > +     int ret;
> > +
> > +     mutex_lock(&gi->lock);
> > +     ret = kstrtou16(page, 0, &bcdVersion);
> > +     if (ret)
> > +             goto out;
> > +     ret = is_valid_bcd(bcdVersion);
> > +     if (ret)
> > +             goto out;
> > +
> > +     gi->bcd_webusb_version = bcdVersion;
> > +     ret = len;
> > +
> > +out:
> > +     mutex_unlock(&gi->lock);
> > +
> > +     return ret;
> > +}
> > +
> > +static ssize_t webusb_bVendorCode_show(struct config_item *item, char *page)
> > +{
> > +     return sysfs_emit(page, "0x%02x\n",
> > +                     webusb_item_to_gadget_info(item)->b_webusb_vendor_code);
> > +}
> > +
> > +static ssize_t webusb_bVendorCode_store(struct config_item *item,
> > +                                        const char *page, size_t len)
> > +{
> > +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
> > +     int ret;
> > +     u8 b_vendor_code;
> > +
> > +     mutex_lock(&gi->lock);
> > +     ret = kstrtou8(page, 0, &b_vendor_code);
> > +     if (!ret) {
> > +             gi->b_webusb_vendor_code = b_vendor_code;
> > +             ret = len;
> > +     }
> > +     mutex_unlock(&gi->lock);
> > +
> > +     return ret;
> > +}
> > +
> > +static ssize_t webusb_landingPage_show(struct config_item *item, char *page)
> > +{
> > +     return sysfs_emit(page, "%s\n", webusb_item_to_gadget_info(item)->landing_page);
> > +}
> > +
> > +static ssize_t webusb_landingPage_store(struct config_item *item, const char *page,
> > +                                  size_t len)
> > +{
> > +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
> > +     int l;
> > +
> > +     l = min(len, sizeof(gi->landing_page));
> > +     if (page[l - 1] == '\n')
> > +             --l;
> > +
> > +     mutex_lock(&gi->lock);
> > +     memcpy(gi->landing_page, page, l);
> > +     mutex_unlock(&gi->lock);
> > +
> > +     return len;
> > +}
> > +
> > +CONFIGFS_ATTR(webusb_, use);
> > +CONFIGFS_ATTR(webusb_, bVendorCode);
> > +CONFIGFS_ATTR(webusb_, bcdVersion);
> > +CONFIGFS_ATTR(webusb_, landingPage);
> > +
> > +static struct configfs_attribute *webusb_attrs[] = {
> > +     &webusb_attr_use,
> > +     &webusb_attr_bcdVersion,
> > +     &webusb_attr_bVendorCode,
> > +     &webusb_attr_landingPage,
> > +     NULL,
> > +};
> > +
> > +static struct config_item_type webusb_type = {
> > +     .ct_attrs       = webusb_attrs,
> > +     .ct_owner       = THIS_MODULE,
> > +};
> > +
> >   static inline struct gadget_info *os_desc_item_to_gadget_info(
> >               struct config_item *item)
> >   {
> > @@ -1341,6 +1472,16 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
> >               gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
> >       }
> >
> > +     if (gi->use_webusb) {
> > +             cdev->use_webusb = true;
> > +             cdev->bcd_webusb_version = gi->bcd_webusb_version;
> > +             cdev->b_webusb_vendor_code = gi->b_webusb_vendor_code;
> > +             memcpy(cdev->landing_page, gi->landing_page,
> > +                             strnlen(gi->landing_page,
> > +                                             min(sizeof(cdev->landing_page),
> > +                                                     sizeof(gi->landing_page))));
>
> checkpatch now allows 100 colums. Plus strnlen() looks indented too far to the
> right.

I will check this.

>
> The sizeofs are both 255. Are they ever expected to be different? Maybe not?
> Then a #defined constant ensures they are equal. Then there's no need to find
> a minimum among equal values.

Technically, if the url has the empty scheme, it can only be 252 bytes
long, since the maximum descriptor length is 255 because bLength is a
single byte, but 3 bytes are in the header of this descriptor, leaving
252 for the string.

However, if the landing page scheme is "https://", the maximum length
of this string could actually be 255 - 3 /*header*/ + 8 /* the length
of the prefix, which is stripped */ => 260 bytes.

I need to rework the validation here a bit. I would like to avoid for
userspace to explicitly set the scheme constants directly, as just
piping in the string seems so much more ergonomic.

>
> Regards,
>
> Andrzej

Thanks a lot for your review!

I hope my response to your initial concerns was not too long and windy.

I will post an updated version of my patch within the next few days.

Regards,
Jó


>
> > +     }
> > +
> >       if (gi->use_os_desc) {
> >               cdev->use_os_string = true;
> >               cdev->b_vendor_code = gi->b_vendor_code;
> > @@ -1605,6 +1746,10 @@ static struct config_group *gadgets_make(
> >                       &os_desc_type);
> >       configfs_add_default_group(&gi->os_desc_group, &gi->group);
> >
> > +     config_group_init_type_name(&gi->webusb_group, "webusb",
> > +                     &webusb_type);
> > +     configfs_add_default_group(&gi->webusb_group, &gi->group);
> > +
> >       gi->composite.bind = configfs_do_nothing;
> >       gi->composite.unbind = configfs_do_nothing;
> >       gi->composite.suspend = NULL;
> > diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
> > index 43ac3fa760db..eb6fac5bbcde 100644
> > --- a/include/linux/usb/composite.h
> > +++ b/include/linux/usb/composite.h
> > @@ -474,6 +474,12 @@ struct usb_composite_dev {
> >       struct usb_configuration        *os_desc_config;
> >       unsigned int                    use_os_string:1;
> >
> > +     /* WebUSB */
> > +     u16                             bcd_webusb_version;
> > +     u8                              b_webusb_vendor_code;
> > +     char                            landing_page[255];
> > +     unsigned int                    use_webusb:1;
> > +
> >       /* private: */
> >       /* internals */
> >       unsigned int                    suspended:1;
> > diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
> > index 31fcfa084e63..3a36550297bc 100644
> > --- a/include/uapi/linux/usb/ch9.h
> > +++ b/include/uapi/linux/usb/ch9.h
> > @@ -947,6 +947,39 @@ struct usb_ss_container_id_descriptor {
> >
> >   #define USB_DT_USB_SS_CONTN_ID_SIZE 20
> >
> > +/*
> > + * Platform Device Capability descriptor: Defines platform specific device
> > + * capabilities
> > + */
> > +#define      USB_PLAT_DEV_CAP_TYPE   5
> > +struct usb_plat_dev_cap_descriptor {
> > +     __u8  bLength;
> > +     __u8  bDescriptorType;
> > +     __u8  bDevCapabilityType;
> > +     __u8  bReserved;
> > +     __u8  UUID[16];
> > +} __attribute__((packed));
> > +
> > +#define USB_DT_USB_PLAT_DEV_CAP_SIZE 20
> > +
> > +/*
> > + * WebUSB Platform Capability descriptor: A device announces support for the
> > + * WebUSB command set by including the following Platform Descriptor in its
> > + * Binary Object Store
> > + * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
> > + */
> > +struct usb_webusb_cap_descriptor {
> > +     __u8  bLength;
> > +     __u8  bDescriptorType;
> > +     __u8  bDevCapabilityType;
> > +     __u8  bReserved;
> > +     __u8  UUID[16];
> > +     __u16 bcdVersion;
> > +     __u8  bVendorCode;
> > +     __u8  iLandingPage;
> > +} __attribute__((packed));
> > +#define USB_DT_WEBUSB_SIZE   (USB_DT_USB_PLAT_DEV_CAP_SIZE + 4)
> > +
> >   /*
> >    * SuperSpeed Plus USB Capability descriptor: Defines the set of
> >    * SuperSpeed Plus USB specific device level capabilities
>

Andrzej Pietrasiewicz Jan. 10, 2023, 1:28 p.m. UTC | #3

Hi Jó,

W dniu 10.01.2023 o 00:19, Jó Ágila Bitsch pisze:
> Hi Andrzej,
> 
>>> The specification allows USB gadgets to announce an URL to landing
>>> page and describes a Javascript interface for websites to interact
>>> with the USB gadget, if the user allows it. It is currently
>>> supported by Chromium-based browsers, such as Chrome, Edge and
>>> Opera on all major operating systems including Linux.
>>>
>>
>> I envision an "evil" scenario, where you don't own your USB device any more.
>> Instead you are required to pay a subscription to keep accessing it
>> because its driver is somewhere on the net. I purposedly have put
>> evil in quotes, because such a scenario might be considered preferred
>> by some people. Maybe there are advantages, too, like not having to worry
>> about correct drivers. Anyway, the kernel is about mechanism, not policy,
>> so I'm probably in no position to judge such things.
> 
> Unfortunately, the "evil" devices you envision very much already exist.
> 
> When I was still working at university in the early 2010s, we were
> working with a mobile EEG headset (>1000USD) with a USB receiver
> dongle that would present itself as a USB HID device, but the
> transferred frames would be AES encrypted, with the key based on a
> proprietary transformation of the serial number of the device. At
> first the library that decoded it was available as a binary blob for a
> one-time payment, where you paid a hefty premium if you wanted access
> to the raw readings. Later the manufacturer changed their business
> model to subscription based, where the data had to flow through their
> cloud service - with a recurring license fee. I don't know what
> happened afterwards, as I left university.
> This all was implemented based on libusb back then before people even
> thought of WebUSB.
> 

Thanks for honestly acknowledging their existence. It confirms that
what I expressed was not a "conspiracy theory", but a "conspiracy
practice" :D

> I'm sure there are other examples.
> 
> So, "evil" devices are already real and don't depend on WebUSB. The
> motivating example "Web Drivers" from the specs
> (https://wicg.github.io/webusb/#app-drivers) might potentially lead to
> companies asking for subscription fees for their Web Driver, but as
> long as there is no strong encryption in place, everyone would be free
> to write an alternative driver. The mechanisms would not at all
> preclude you from writing your own kernel or userspace driver using
> e.g. libusb. A kernel driver would block access from the browser
> though, as the browser cannot detach the kernel driver. (No such
> command is currently defined in WebUSB.)  The JS implementation might
> even guide you to understand aspects of the protocol used.
> 
> Citing from the motivating example in the design specs
> (https://wicg.github.io/webusb/#app-updates-and-diagnostics):
> 
>> The landing page provides a way for the device manufacturer to direct the user to the right part of their website for help with their device.
> 
> The landing page mechanism in WebUSB improves discoverability for
> devices, as Chrome and also lsusb directly point you to a webpage,
> where the manufacturer or firmware producer can give more information
> about the device. This webpage does not necessarily need to use a
> WebUSB JS driver itself to access the device, but might just as well
> point you to some git repository, where the hardware design and
> firmware source are available

I'm wondering why e.g. Firefox does not implement this? Chicken-and-egg
dilemma (no OS support at kernel level, so let's refrain from using it,
but then there's too few users, so let's not add OS support - this
circular depencency can be broken by applying your patch) or something
else?

> 
> For what it's worth, the pick-up of WebUSB I've seen has been mostly
> in the maker scene with arduino, micro:bit and similar embedded
> devices, where devices provide at least two interfaces: One for the OS
> to claim and one for the Browser to claim.
> 
> Anyways, my biased opinion is that I would very much like a device to
> tell me where I can find support documentation, updates and help also
> when the device only implements standard interfaces that have kernel
> drivers. I don't think that this URL will lead to more devices you can
> only access with a subscription.
> 
>> Please see below.
> 
> Thanks for your excellent feedback. I answered inline.
> 
>>> This patch adds optional support for Linux-based USB gadgets
>>> wishing to expose such a landing page.
>>>
>>> During device enumeration, a host recognizes that the announced
>>> USB version is at least 2.01, which means, that there are BOS
>>> descriptors available. The device than announces WebUSB support
>>> using a platform device capability. This includes a vendor code
>>> under which the landing page URL can be retrieved using a
>>> vendor-specific request.
>>>
>>> Previously, the BOS descriptors would unconditionally include an
>>> LPM related descriptor, as BOS descriptors were only ever sent
>>> when the device was LPM capable. As this is no longer the case,
>>> this patch puts this descriptor behind a lpm_capable condition.
>>>
>>> Usage is modeled after os_desc descriptors:
>>> echo 1 > webusb/use
>>> echo "https://www.kernel.org" > webusb/landingPage
>>>
>>> lsusb will report the device with the following lines:
>>>     Platform Device Capability:
>>>       bLength                24
>>>       bDescriptorType        16
>>>       bDevCapabilityType      5
>>>       bReserved               0
>>>       PlatformCapabilityUUID    {3408b638-09a9-47a0-8bfd-a0768815b665}
>>>         WebUSB:
>>>           bcdVersion    1.00
>>>           bVendorCode      0
>>>           iLandingPage     1 https://www.kernel.org
>>>
>>> Signed-off-by: Jó Ágila Bitsch <[e-mail]>
>>> ---
>>> v2 -> V3: improved commit message to include additional condition in desc_bos as per comment
>>>          by Alan Stern
>>> V1 -> V2: cleaned up coding style, made URL scheme comparison case insensitive, addressed review
>>>             comments by Alan Stern
>>> V0 -> V1: use sysfs_emit instead of sprintf and use lock in webusb_bcdVersion_store, addressed review
>>>          comments by Christophe JAILLET
>>>
>>>    Documentation/ABI/testing/configfs-usb-gadget |  13 ++
>>>    drivers/usb/gadget/composite.c                |  95 ++++++++++--
>>>    drivers/usb/gadget/configfs.c                 | 145 ++++++++++++++++++
>>>    include/linux/usb/composite.h                 |   6 +
>>>    include/uapi/linux/usb/ch9.h                  |  33 ++++
>>>    5 files changed, 282 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/configfs-usb-gadget b/Documentation/ABI/testing/configfs-usb-gadget
>>> index b7943aa7e997..8399e0f0ed1c 100644
>>> --- a/Documentation/ABI/testing/configfs-usb-gadget
>>> +++ b/Documentation/ABI/testing/configfs-usb-gadget
>>> @@ -143,3 +143,16 @@ Description:
>>>                qw_sign         an identifier to be reported as "OS String"
>>>                                proper
>>>                =============   ===============================================
>>> +
>>> +What:                /config/usb-gadget/gadget/webusb
>>> +Date:                Dec 2022
>>> +KernelVersion:       6.2
>>> +Description:
>>> +             This group contains "WebUSB" extension handling attributes.
>>> +
>>> +             =============   ===============================================
>>> +             use             flag turning "WebUSB" support on/off
>>> +             bcdVersion      bcd WebUSB specification version number
>>> +             b_vendor_code   one-byte value used for custom per-device
>>> +             landing_page    UTF-8 encoded URL of the device's landing page
>>> +             =============   ===============================================
>>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>>> index 403563c06477..9b209e69442d 100644
>>> --- a/drivers/usb/gadget/composite.c
>>> +++ b/drivers/usb/gadget/composite.c
>>> @@ -14,6 +14,7 @@
>>>    #include <linux/device.h>
>>>    #include <linux/utsname.h>
>>>    #include <linux/bitfield.h>
>>> +#include <linux/uuid.h>
>>>
>>>    #include <linux/usb/composite.h>
>>>    #include <linux/usb/otg.h>
>>> @@ -713,14 +714,16 @@ static int bos_desc(struct usb_composite_dev *cdev)
>>>         * A SuperSpeed device shall include the USB2.0 extension descriptor
>>>         * and shall support LPM when operating in USB2.0 HS mode.
>>>         */
>>> -     usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
>>> -     bos->bNumDeviceCaps++;
>>> -     le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
>>> -     usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
>>> -     usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>>> -     usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
>>> -     usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
>>> -                                         USB_BESL_SUPPORT | besl);
>>> +     if (cdev->gadget->lpm_capable) {
>>> +             usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
>>> +             bos->bNumDeviceCaps++;
>>> +             le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
>>> +             usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
>>> +             usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>>> +             usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
>>> +             usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
>>> +                                                     USB_BESL_SUPPORT | besl);
>>> +     }
>>>
>>>        /*
>>>         * The Superspeed USB Capability descriptor shall be implemented by all
>>> @@ -821,6 +824,40 @@ static int bos_desc(struct usb_composite_dev *cdev)
>>>                }
>>>        }
>>>
>>> +     /*
>>> +      * Following the specifaction at:
>>> +      * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
>>> +      */
>>> +     if (cdev->use_webusb) {
>>> +             struct usb_webusb_cap_descriptor *webusb_cap;
>>> +             /*
>>> +              * little endian PlatformCapablityUUID for WebUSB:
>>> +              * 3408b638-09a9-47a0-8bfd-a0768815b665
>>> +              */
>>> +             uuid_t webusb_uuid = UUID_INIT(0x38b60834, 0xa909, 0xa047, 0x8b, 0xfd, 0xa0, 0x76,
>>> +                                             0x88, 0x15, 0xb6, 0x65);
>>> +
>>> +             webusb_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
>>> +             bos->bNumDeviceCaps++;
>>> +
>>> +             le16_add_cpu(&bos->wTotalLength, USB_DT_WEBUSB_SIZE);
>>> +             webusb_cap->bLength = USB_DT_WEBUSB_SIZE;
>>> +             webusb_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>>> +             webusb_cap->bDevCapabilityType = USB_PLAT_DEV_CAP_TYPE;
>>> +             webusb_cap->bReserved = 0;
>>> +             export_uuid(webusb_cap->UUID, &webusb_uuid);
>>> +             if (cdev->bcd_webusb_version != 0)
>>> +                     webusb_cap->bcdVersion = cpu_to_le16(cdev->bcd_webusb_version);
>>> +             else
>>> +                     webusb_cap->bcdVersion = cpu_to_le16(0x0100);
>>> +
>>> +             webusb_cap->bVendorCode = cdev->b_webusb_vendor_code;
>>> +             if (strnlen(cdev->landing_page, sizeof(cdev->landing_page)) > 0)
>>> +                     webusb_cap->iLandingPage = 1;
>>> +             else
>>> +                     webusb_cap->iLandingPage = 0;
>>> +     }
>>> +
>>>        return le16_to_cpu(bos->wTotalLength);
>>>    }
>>>
>>> @@ -1744,7 +1781,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>>>                                        cdev->desc.bcdUSB = cpu_to_le16(0x0210);
>>>                                }
>>>                        } else {
>>> -                             if (gadget->lpm_capable)
>>> +                             if (gadget->lpm_capable || cdev->use_webusb)
>>>                                        cdev->desc.bcdUSB = cpu_to_le16(0x0201);
>>>                                else
>>>                                        cdev->desc.bcdUSB = cpu_to_le16(0x0200);
>>> @@ -1779,7 +1816,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>>>                        break;
>>>                case USB_DT_BOS:
>>>                        if (gadget_is_superspeed(gadget) ||
>>> -                         gadget->lpm_capable) {
>>> +                         gadget->lpm_capable || cdev->use_webusb) {
>>>                                value = bos_desc(cdev);
>>>                                value = min(w_length, (u16) value);
>>>                        }
>>> @@ -2013,6 +2050,44 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>>>                        goto check_value;
>>>                }
>>>
>>> +             /*
>>> +              * webusb descriptor handling, following:
>>> +              * https://wicg.github.io/webusb/#device-requests
>>> +              */
>>> +             if (cdev->use_webusb &&
>>> +                 (ctrl->bRequestType & USB_TYPE_VENDOR) &&
>>> +                 ctrl->wIndex == /* WEBUSB_GET_URL*/ 2 &&
>>> +                 ctrl->bRequest == cdev->b_webusb_vendor_code) {
>>> +                     /*
>>> +                      * specification of the url descriptor in WebUSB:
>>> +                      * https://wicg.github.io/webusb/#webusb-descriptors
>>> +                      */
>>> +                     u8              *buf;
>>> +                     unsigned int    landing_page_length;
>>> +
>>> +                     buf = cdev->req->buf;
>>> +                     buf[1] = /* WEBUSB_URL*/ 3;
>>
>> Why not #define WEBUSB_URL?
> 
> in a previous iteration, I had that exact #define in place, right
> above. However, another reviewer found it pointless. Maybe I should
> instead put the define into include/uapi/linux/usb/ch9.h
> 

Hmm. ch9.h had been called ch9.h for a reason. It corresponded to Chapter 9 of
the USB spec. Adding to it something that by design is outside the spec might
not be what we want. But I don't know.

>>
>>> +
>>> +                     landing_page_length = strnlen(cdev->landing_page, 252);
>>> +                     if (strncasecmp("https://", cdev->landing_page, 8) == 0) {
>>
>> Maybe it's me, but it would be easier for me to understand why the "8" if the
>> comparison was in reverse order of arguments, like this:
>>
>> strncasecmp(cdev->landing_page, "https://", 8)
> 
> I agree, this makes the 8 clearer.
> 
>> because we want the entirety of "https://" compared, not its substring, so the
>> limit kind of applies to the landing_page in the first place.
>> Maybe the "8" (and "7" below) can be #define'd, too?
> 
> depends on the name, but maybe something like
> 
> #define WEBUSB_PREFIX_HTTPS "https://"
> #define WEBUSB_PREFIX_HTTPS_LENGTH 8
> #define WEBUSB_PREFIX_HTTPS_SCHEME_ID 0x01
> 
> would work defined with the struct in include/uapi/linux/usb/ch9.h
> 
>>
>>> +                             buf[2] = 0x01;
>>
>> What is this magic 0x01?
> 
> The field is specified in
> https://wicg.github.io/webusb/#url-descriptor and would be the
> WEBUSB_PREFIX_HTTPS_SCHEME_ID from above.
> But yes, I should #define this.
> 
>>
>>> +                             memcpy(buf+3, cdev->landing_page+8, landing_page_length-8);
>>
>> spaces around arithmetic operators?
> 
> will do
> 
> 
>>> +                             buf[0] = landing_page_length - 8 + 3;
>>> +                     } else if (strncasecmp("http://", cdev->landing_page, 7) == 0) {
>>> +                             buf[2] = 0x00;
>>
>> Magic 0x00?
> 
> This would then be WEBUSB_PREFIX_HTTP_SCHEME_ID
> 
>>
>>> +                             memcpy(buf+3, cdev->landing_page+7, landing_page_length-7);
>>> +                             buf[0] = landing_page_length - 7 + 3;
>>> +                     } else {
>>> +                             buf[2] = 0xFF;
>>
>> Magic 0xFF? (plus I'd expect lowercase hex digits).
> 
> This would then be WEBUSB_PREFIX_NONE_SCHEME_ID
> 
>> There's "URL Prefixes" table in 4.3.1 URL Descriptor. Why not define
>> URL_PREFIX_HTTP/HTTPS/NONE?
> 
> will do
> 
>>> +                             memcpy(buf+3, cdev->landing_page, landing_page_length);
>>> +                             buf[0] = landing_page_length + 3;
>>> +                     }
>>> +
>>> +                     value = buf[0];
>>> +
>>> +                     goto check_value;
>>> +             }
>>> +
>>>                VDBG(cdev,
>>>                        "non-core control req%02x.%02x v%04x i%04x l%d\n",
>>>                        ctrl->bRequestType, ctrl->bRequest,
>>> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
>>> index 96121d1c8df4..2b7f01a9efff 100644
>>> --- a/drivers/usb/gadget/configfs.c
>>> +++ b/drivers/usb/gadget/configfs.c
>>> @@ -39,6 +39,7 @@ struct gadget_info {
>>>        struct config_group configs_group;
>>>        struct config_group strings_group;
>>>        struct config_group os_desc_group;
>>> +     struct config_group webusb_group;
>>>
>>>        struct mutex lock;
>>>        struct usb_gadget_strings *gstrings[MAX_USB_STRING_LANGS + 1];
>>> @@ -50,6 +51,11 @@ struct gadget_info {
>>>        bool use_os_desc;
>>>        char b_vendor_code;
>>>        char qw_sign[OS_STRING_QW_SIGN_LEN];
>>> +     bool use_webusb;
>>> +     u16 bcd_webusb_version;
>>> +     u8 b_webusb_vendor_code;
>>> +     char landing_page[255];
>>
>> A #define for the size?
> 
> will do
> 
>>
>>> +
>>>        spinlock_t spinlock;
>>>        bool unbind;
>>>    };
>>> @@ -780,6 +786,131 @@ static void gadget_strings_attr_release(struct config_item *item)
>>>    USB_CONFIG_STRING_RW_OPS(gadget_strings);
>>>    USB_CONFIG_STRINGS_LANG(gadget_strings, gadget_info);
>>>
>>> +static inline struct gadget_info *webusb_item_to_gadget_info(
>>> +             struct config_item *item)
>>> +{
>>> +     return container_of(to_config_group(item),
>>> +                     struct gadget_info, webusb_group);
>>> +}
>>> +
>>> +static ssize_t webusb_use_show(struct config_item *item, char *page)
>>> +{
>>> +     return sysfs_emit(page, "%d\n",
>>> +                     webusb_item_to_gadget_info(item)->use_webusb);
>>> +}
>>> +
>>> +static ssize_t webusb_use_store(struct config_item *item, const char *page,
>>> +                              size_t len)
>>> +{
>>> +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
>>> +     int ret;
>>> +     bool use;
>>> +
>>> +     mutex_lock(&gi->lock);
>>> +     ret = kstrtobool(page, &use);
>>> +     if (!ret) {
>>> +             gi->use_webusb = use;
>>> +             ret = len;
>>> +     }
>>> +     mutex_unlock(&gi->lock);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static ssize_t webusb_bcdVersion_show(struct config_item *item, char *page)
>>> +{
>>> +     return sysfs_emit(page, "0x%04x\n",
>>> +                                     webusb_item_to_gadget_info(item)->bcd_webusb_version);
>>> +}
>>> +
>>> +static ssize_t webusb_bcdVersion_store(struct config_item *item,
>>> +             const char *page, size_t len)
>>> +{
>>> +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
>>> +     u16 bcdVersion;
>>> +     int ret;
>>> +
>>> +     mutex_lock(&gi->lock);
>>> +     ret = kstrtou16(page, 0, &bcdVersion);
>>> +     if (ret)
>>> +             goto out;
>>> +     ret = is_valid_bcd(bcdVersion);
>>> +     if (ret)
>>> +             goto out;
>>> +
>>> +     gi->bcd_webusb_version = bcdVersion;
>>> +     ret = len;
>>> +
>>> +out:
>>> +     mutex_unlock(&gi->lock);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static ssize_t webusb_bVendorCode_show(struct config_item *item, char *page)
>>> +{
>>> +     return sysfs_emit(page, "0x%02x\n",
>>> +                     webusb_item_to_gadget_info(item)->b_webusb_vendor_code);
>>> +}
>>> +
>>> +static ssize_t webusb_bVendorCode_store(struct config_item *item,
>>> +                                        const char *page, size_t len)
>>> +{
>>> +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
>>> +     int ret;
>>> +     u8 b_vendor_code;
>>> +
>>> +     mutex_lock(&gi->lock);
>>> +     ret = kstrtou8(page, 0, &b_vendor_code);
>>> +     if (!ret) {
>>> +             gi->b_webusb_vendor_code = b_vendor_code;
>>> +             ret = len;
>>> +     }
>>> +     mutex_unlock(&gi->lock);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static ssize_t webusb_landingPage_show(struct config_item *item, char *page)
>>> +{
>>> +     return sysfs_emit(page, "%s\n", webusb_item_to_gadget_info(item)->landing_page);
>>> +}
>>> +
>>> +static ssize_t webusb_landingPage_store(struct config_item *item, const char *page,
>>> +                                  size_t len)
>>> +{
>>> +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
>>> +     int l;
>>> +
>>> +     l = min(len, sizeof(gi->landing_page));
>>> +     if (page[l - 1] == '\n')
>>> +             --l;
>>> +
>>> +     mutex_lock(&gi->lock);
>>> +     memcpy(gi->landing_page, page, l);
>>> +     mutex_unlock(&gi->lock);
>>> +
>>> +     return len;
>>> +}
>>> +
>>> +CONFIGFS_ATTR(webusb_, use);
>>> +CONFIGFS_ATTR(webusb_, bVendorCode);
>>> +CONFIGFS_ATTR(webusb_, bcdVersion);
>>> +CONFIGFS_ATTR(webusb_, landingPage);
>>> +
>>> +static struct configfs_attribute *webusb_attrs[] = {
>>> +     &webusb_attr_use,
>>> +     &webusb_attr_bcdVersion,
>>> +     &webusb_attr_bVendorCode,
>>> +     &webusb_attr_landingPage,
>>> +     NULL,
>>> +};
>>> +
>>> +static struct config_item_type webusb_type = {
>>> +     .ct_attrs       = webusb_attrs,
>>> +     .ct_owner       = THIS_MODULE,
>>> +};
>>> +
>>>    static inline struct gadget_info *os_desc_item_to_gadget_info(
>>>                struct config_item *item)
>>>    {
>>> @@ -1341,6 +1472,16 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
>>>                gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
>>>        }
>>>
>>> +     if (gi->use_webusb) {
>>> +             cdev->use_webusb = true;
>>> +             cdev->bcd_webusb_version = gi->bcd_webusb_version;
>>> +             cdev->b_webusb_vendor_code = gi->b_webusb_vendor_code;
>>> +             memcpy(cdev->landing_page, gi->landing_page,
>>> +                             strnlen(gi->landing_page,
>>> +                                             min(sizeof(cdev->landing_page),
>>> +                                                     sizeof(gi->landing_page))));
>>
>> checkpatch now allows 100 colums. Plus strnlen() looks indented too far to the
>> right.
> 
> I will check this.
> 
>>
>> The sizeofs are both 255. Are they ever expected to be different? Maybe not?
>> Then a #defined constant ensures they are equal. Then there's no need to find
>> a minimum among equal values.
> 
> Technically, if the url has the empty scheme, it can only be 252 bytes
> long, since the maximum descriptor length is 255 because bLength is a
> single byte, but 3 bytes are in the header of this descriptor, leaving
> 252 for the string.

But you use sizeof() rather than strlen(). And the "landing_page" members
are defined as 255-element arrays of chars, so both sizeof() invocations
will yield 255, no?

Andrzej

> 
> However, if the landing page scheme is "https://", the maximum length
> of this string could actually be 255 - 3 /*header*/ + 8 /* the length
> of the prefix, which is stripped */ => 260 bytes.
> 
> I need to rework the validation here a bit. I would like to avoid for
> userspace to explicitly set the scheme constants directly, as just
> piping in the string seems so much more ergonomic.
> 
>>
>> Regards,
>>
>> Andrzej
> 
> Thanks a lot for your review!
> 
> I hope my response to your initial concerns was not too long and windy.
> 
> I will post an updated version of my patch within the next few days.
> 
> Regards,
> Jó
> 
> 
>>
>>> +     }
>>> +
>>>        if (gi->use_os_desc) {
>>>                cdev->use_os_string = true;
>>>                cdev->b_vendor_code = gi->b_vendor_code;
>>> @@ -1605,6 +1746,10 @@ static struct config_group *gadgets_make(
>>>                        &os_desc_type);
>>>        configfs_add_default_group(&gi->os_desc_group, &gi->group);
>>>
>>> +     config_group_init_type_name(&gi->webusb_group, "webusb",
>>> +                     &webusb_type);
>>> +     configfs_add_default_group(&gi->webusb_group, &gi->group);
>>> +
>>>        gi->composite.bind = configfs_do_nothing;
>>>        gi->composite.unbind = configfs_do_nothing;
>>>        gi->composite.suspend = NULL;
>>> diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
>>> index 43ac3fa760db..eb6fac5bbcde 100644
>>> --- a/include/linux/usb/composite.h
>>> +++ b/include/linux/usb/composite.h
>>> @@ -474,6 +474,12 @@ struct usb_composite_dev {
>>>        struct usb_configuration        *os_desc_config;
>>>        unsigned int                    use_os_string:1;
>>>
>>> +     /* WebUSB */
>>> +     u16                             bcd_webusb_version;
>>> +     u8                              b_webusb_vendor_code;
>>> +     char                            landing_page[255];
>>> +     unsigned int                    use_webusb:1;
>>> +
>>>        /* private: */
>>>        /* internals */
>>>        unsigned int                    suspended:1;
>>> diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
>>> index 31fcfa084e63..3a36550297bc 100644
>>> --- a/include/uapi/linux/usb/ch9.h
>>> +++ b/include/uapi/linux/usb/ch9.h
>>> @@ -947,6 +947,39 @@ struct usb_ss_container_id_descriptor {
>>>
>>>    #define USB_DT_USB_SS_CONTN_ID_SIZE 20
>>>
>>> +/*
>>> + * Platform Device Capability descriptor: Defines platform specific device
>>> + * capabilities
>>> + */
>>> +#define      USB_PLAT_DEV_CAP_TYPE   5
>>> +struct usb_plat_dev_cap_descriptor {
>>> +     __u8  bLength;
>>> +     __u8  bDescriptorType;
>>> +     __u8  bDevCapabilityType;
>>> +     __u8  bReserved;
>>> +     __u8  UUID[16];
>>> +} __attribute__((packed));
>>> +
>>> +#define USB_DT_USB_PLAT_DEV_CAP_SIZE 20
>>> +
>>> +/*
>>> + * WebUSB Platform Capability descriptor: A device announces support for the
>>> + * WebUSB command set by including the following Platform Descriptor in its
>>> + * Binary Object Store
>>> + * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
>>> + */
>>> +struct usb_webusb_cap_descriptor {
>>> +     __u8  bLength;
>>> +     __u8  bDescriptorType;
>>> +     __u8  bDevCapabilityType;
>>> +     __u8  bReserved;
>>> +     __u8  UUID[16];
>>> +     __u16 bcdVersion;
>>> +     __u8  bVendorCode;
>>> +     __u8  iLandingPage;
>>> +} __attribute__((packed));
>>> +#define USB_DT_WEBUSB_SIZE   (USB_DT_USB_PLAT_DEV_CAP_SIZE + 4)
>>> +
>>>    /*
>>>     * SuperSpeed Plus USB Capability descriptor: Defines the set of
>>>     * SuperSpeed Plus USB specific device level capabilities
>>

Alan Stern Jan. 10, 2023, 4:09 p.m. UTC | #4

On Tue, Jan 10, 2023 at 02:28:55PM +0100, Andrzej Pietrasiewicz wrote:
> Hi Jó,
> 
> W dniu 10.01.2023 o 00:19, Jó Ágila Bitsch pisze:

> > > > +                     buf[1] = /* WEBUSB_URL*/ 3;
> > > 
> > > Why not #define WEBUSB_URL?
> > 
> > in a previous iteration, I had that exact #define in place, right
> > above. However, another reviewer found it pointless. Maybe I should
> > instead put the define into include/uapi/linux/usb/ch9.h
> > 
> 
> Hmm. ch9.h had been called ch9.h for a reason. It corresponded to Chapter 9 of
> the USB spec. Adding to it something that by design is outside the spec might
> not be what we want. But I don't know.

It wouldn't hurt to create a new file: include/uapi/linux/usb/webusb.h.

Alan Stern

Jó Ágila Bitsch Jan. 10, 2023, 11:15 p.m. UTC | #5

Hi Andrzej,

On Tue, Jan 10, 2023 at 2:28 PM Andrzej Pietrasiewicz
<[e-mail]> wrote:
>
> Hi Jó,
>
> W dniu 10.01.2023 o 00:19, Jó Ágila Bitsch pisze:
> > Hi Andrzej,
> >
> >>> The specification allows USB gadgets to announce an URL to landing
> >>> page and describes a Javascript interface for websites to interact
> >>> with the USB gadget, if the user allows it. It is currently
> >>> supported by Chromium-based browsers, such as Chrome, Edge and
> >>> Opera on all major operating systems including Linux.
> >>>
> >>
> >> I envision an "evil" scenario, where you don't own your USB device any more.
> >> Instead you are required to pay a subscription to keep accessing it
> >> because its driver is somewhere on the net. I purposedly have put
> >> evil in quotes, because such a scenario might be considered preferred
> >> by some people. Maybe there are advantages, too, like not having to worry
> >> about correct drivers. Anyway, the kernel is about mechanism, not policy,
> >> so I'm probably in no position to judge such things.
> >
> > Unfortunately, the "evil" devices you envision very much already exist.
> >
> > When I was still working at university in the early 2010s, we were
> > working with a mobile EEG headset (>1000USD) with a USB receiver
> > dongle that would present itself as a USB HID device, but the
> > transferred frames would be AES encrypted, with the key based on a
> > proprietary transformation of the serial number of the device. At
> > first the library that decoded it was available as a binary blob for a
> > one-time payment, where you paid a hefty premium if you wanted access
> > to the raw readings. Later the manufacturer changed their business
> > model to subscription based, where the data had to flow through their
> > cloud service - with a recurring license fee. I don't know what
> > happened afterwards, as I left university.
> > This all was implemented based on libusb back then before people even
> > thought of WebUSB.
> >
>
> Thanks for honestly acknowledging their existence. It confirms that
> what I expressed was not a "conspiracy theory", but a "conspiracy
> practice" :D
>
> > I'm sure there are other examples.
> >
> > So, "evil" devices are already real and don't depend on WebUSB. The
> > motivating example "Web Drivers" from the specs
> > (https://wicg.github.io/webusb/#app-drivers) might potentially lead to
> > companies asking for subscription fees for their Web Driver, but as
> > long as there is no strong encryption in place, everyone would be free
> > to write an alternative driver. The mechanisms would not at all
> > preclude you from writing your own kernel or userspace driver using
> > e.g. libusb. A kernel driver would block access from the browser
> > though, as the browser cannot detach the kernel driver. (No such
> > command is currently defined in WebUSB.)  The JS implementation might
> > even guide you to understand aspects of the protocol used.
> >
> > Citing from the motivating example in the design specs
> > (https://wicg.github.io/webusb/#app-updates-and-diagnostics):
> >
> >> The landing page provides a way for the device manufacturer to direct the user to the right part of their website for help with their device.
> >
> > The landing page mechanism in WebUSB improves discoverability for
> > devices, as Chrome and also lsusb directly point you to a webpage,
> > where the manufacturer or firmware producer can give more information
> > about the device. This webpage does not necessarily need to use a
> > WebUSB JS driver itself to access the device, but might just as well
> > point you to some git repository, where the hardware design and
> > firmware source are available
>
> I'm wondering why e.g. Firefox does not implement this?

There has been an extended discussion within Firefox:
https://github.com/mozilla/standards-positions/issues/100
https://github.com/mozilla/standards-positions/pull/193

The main concerns center around (quoted from the link above):
> * security issues related to attacks on devices that, once compromised, could be used to attack the user's computer more generally
> * the permissions model (whether a question can be posed to the user that yields informed consent, whether there should be a CORS-like model for devices to say that they want to interact with particular domains which may improve security but hurts openness)

and tracking users across websites by exposing static identifiers

The webkit position is tracked here:
https://github.com/WebKit/standards-positions/issues/68

They are following Mozilla's position and also raising the issue of
device-independence.

However, there has been some recent discussion to reevaluate this
position, especially in the context of extensions and users are
repeatedly bringing up complaints that they would like to see this
implemented.

> Chicken-and-egg
> dilemma (no OS support at kernel level, so let's refrain from using it,
> but then there's too few users, so let's not add OS support - this
> circular depencency can be broken by applying your patch) or something
> else?

OS support at the kernel is not actually necessary for the host side
of the USB connection at all, as the browser would just access the
devices via the exposed character devices in /dev/bus/usb/*/*

My patch would only improve the device side of the USB connection,
namely, that the landing page can be announced in gadget mode. So this
would enable makers and manufacturers of USB gadgets based on embedded
linux devices to point to a landing page for their device.

The other aspects of USB communication are already well supported by
the USB gadget subsystem and don't need any changes to work with
WebUSB. Actually, they are already working fine with WebUSB. An
example with a Linux based gadget side is a WebAdb library
(https://webadb.github.io/), that allows Websites to access Android
phones via a website.

I'm imagining the use case of single board computers, such as
raspberry pi, orange pi, or other open hardware, such as a linux based
USB stick such as https://github.com/usbarmory/usbarmory.

But it might just as well be your home router, that is connected via
USB and points you to the configuration page directly. It knows its
own IP address on the local network (and/or provide a USB network
interface via RNDIS/ECM/NCM) and can dynamically point to the correct
one. The last one could probably also work with some network based
discovery mechanism but I want to open up the space a bit to things
that just need the landing page and not the other parts of webusb.

Actually this would combine well with what you (Andrzej) presented
back in 2019 on
https://www.collabora.com/news-and-blog/blog/2019/02/18/modern-usb-gadget-on-linux-and-how-to-integrate-it-with-systemd-part-1/

>
> >
> > For what it's worth, the pick-up of WebUSB I've seen has been mostly
> > in the maker scene with arduino, micro:bit and similar embedded
> > devices, where devices provide at least two interfaces: One for the OS
> > to claim and one for the Browser to claim.
> >
> > Anyways, my biased opinion is that I would very much like a device to
> > tell me where I can find support documentation, updates and help also
> > when the device only implements standard interfaces that have kernel
> > drivers. I don't think that this URL will lead to more devices you can
> > only access with a subscription.
> >
> >> Please see below.
> >
> > Thanks for your excellent feedback. I answered inline.
> >
> >>> This patch adds optional support for Linux-based USB gadgets
> >>> wishing to expose such a landing page.
> >>>
> >>> During device enumeration, a host recognizes that the announced
> >>> USB version is at least 2.01, which means, that there are BOS
> >>> descriptors available. The device than announces WebUSB support
> >>> using a platform device capability. This includes a vendor code
> >>> under which the landing page URL can be retrieved using a
> >>> vendor-specific request.
> >>>
> >>> Previously, the BOS descriptors would unconditionally include an
> >>> LPM related descriptor, as BOS descriptors were only ever sent
> >>> when the device was LPM capable. As this is no longer the case,
> >>> this patch puts this descriptor behind a lpm_capable condition.
> >>>
> >>> Usage is modeled after os_desc descriptors:
> >>> echo 1 > webusb/use
> >>> echo "https://www.kernel.org" > webusb/landingPage
> >>>
> >>> lsusb will report the device with the following lines:
> >>>     Platform Device Capability:
> >>>       bLength                24
> >>>       bDescriptorType        16
> >>>       bDevCapabilityType      5
> >>>       bReserved               0
> >>>       PlatformCapabilityUUID    {3408b638-09a9-47a0-8bfd-a0768815b665}
> >>>         WebUSB:
> >>>           bcdVersion    1.00
> >>>           bVendorCode      0
> >>>           iLandingPage     1 https://www.kernel.org
> >>>
> >>> Signed-off-by: Jó Ágila Bitsch <[e-mail]>
> >>> ---
> >>> v2 -> V3: improved commit message to include additional condition in desc_bos as per comment
> >>>          by Alan Stern
> >>> V1 -> V2: cleaned up coding style, made URL scheme comparison case insensitive, addressed review
> >>>             comments by Alan Stern
> >>> V0 -> V1: use sysfs_emit instead of sprintf and use lock in webusb_bcdVersion_store, addressed review
> >>>          comments by Christophe JAILLET
> >>>
> >>>    Documentation/ABI/testing/configfs-usb-gadget |  13 ++
> >>>    drivers/usb/gadget/composite.c                |  95 ++++++++++--
> >>>    drivers/usb/gadget/configfs.c                 | 145 ++++++++++++++++++
> >>>    include/linux/usb/composite.h                 |   6 +
> >>>    include/uapi/linux/usb/ch9.h                  |  33 ++++
> >>>    5 files changed, 282 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/Documentation/ABI/testing/configfs-usb-gadget b/Documentation/ABI/testing/configfs-usb-gadget
> >>> index b7943aa7e997..8399e0f0ed1c 100644
> >>> --- a/Documentation/ABI/testing/configfs-usb-gadget
> >>> +++ b/Documentation/ABI/testing/configfs-usb-gadget
> >>> @@ -143,3 +143,16 @@ Description:
> >>>                qw_sign         an identifier to be reported as "OS String"
> >>>                                proper
> >>>                =============   ===============================================
> >>> +
> >>> +What:                /config/usb-gadget/gadget/webusb
> >>> +Date:                Dec 2022
> >>> +KernelVersion:       6.2
> >>> +Description:
> >>> +             This group contains "WebUSB" extension handling attributes.
> >>> +
> >>> +             =============   ===============================================
> >>> +             use             flag turning "WebUSB" support on/off
> >>> +             bcdVersion      bcd WebUSB specification version number
> >>> +             b_vendor_code   one-byte value used for custom per-device
> >>> +             landing_page    UTF-8 encoded URL of the device's landing page
> >>> +             =============   ===============================================
> >>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> >>> index 403563c06477..9b209e69442d 100644
> >>> --- a/drivers/usb/gadget/composite.c
> >>> +++ b/drivers/usb/gadget/composite.c
> >>> @@ -14,6 +14,7 @@
> >>>    #include <linux/device.h>
> >>>    #include <linux/utsname.h>
> >>>    #include <linux/bitfield.h>
> >>> +#include <linux/uuid.h>
> >>>
> >>>    #include <linux/usb/composite.h>
> >>>    #include <linux/usb/otg.h>
> >>> @@ -713,14 +714,16 @@ static int bos_desc(struct usb_composite_dev *cdev)
> >>>         * A SuperSpeed device shall include the USB2.0 extension descriptor
> >>>         * and shall support LPM when operating in USB2.0 HS mode.
> >>>         */
> >>> -     usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> >>> -     bos->bNumDeviceCaps++;
> >>> -     le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
> >>> -     usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
> >>> -     usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> >>> -     usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
> >>> -     usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
> >>> -                                         USB_BESL_SUPPORT | besl);
> >>> +     if (cdev->gadget->lpm_capable) {
> >>> +             usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> >>> +             bos->bNumDeviceCaps++;
> >>> +             le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
> >>> +             usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
> >>> +             usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> >>> +             usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
> >>> +             usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
> >>> +                                                     USB_BESL_SUPPORT | besl);
> >>> +     }
> >>>
> >>>        /*
> >>>         * The Superspeed USB Capability descriptor shall be implemented by all
> >>> @@ -821,6 +824,40 @@ static int bos_desc(struct usb_composite_dev *cdev)
> >>>                }
> >>>        }
> >>>
> >>> +     /*
> >>> +      * Following the specifaction at:
> >>> +      * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
> >>> +      */
> >>> +     if (cdev->use_webusb) {
> >>> +             struct usb_webusb_cap_descriptor *webusb_cap;
> >>> +             /*
> >>> +              * little endian PlatformCapablityUUID for WebUSB:
> >>> +              * 3408b638-09a9-47a0-8bfd-a0768815b665
> >>> +              */
> >>> +             uuid_t webusb_uuid = UUID_INIT(0x38b60834, 0xa909, 0xa047, 0x8b, 0xfd, 0xa0, 0x76,
> >>> +                                             0x88, 0x15, 0xb6, 0x65);
> >>> +
> >>> +             webusb_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> >>> +             bos->bNumDeviceCaps++;
> >>> +
> >>> +             le16_add_cpu(&bos->wTotalLength, USB_DT_WEBUSB_SIZE);
> >>> +             webusb_cap->bLength = USB_DT_WEBUSB_SIZE;
> >>> +             webusb_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> >>> +             webusb_cap->bDevCapabilityType = USB_PLAT_DEV_CAP_TYPE;
> >>> +             webusb_cap->bReserved = 0;
> >>> +             export_uuid(webusb_cap->UUID, &webusb_uuid);
> >>> +             if (cdev->bcd_webusb_version != 0)
> >>> +                     webusb_cap->bcdVersion = cpu_to_le16(cdev->bcd_webusb_version);
> >>> +             else
> >>> +                     webusb_cap->bcdVersion = cpu_to_le16(0x0100);
> >>> +
> >>> +             webusb_cap->bVendorCode = cdev->b_webusb_vendor_code;
> >>> +             if (strnlen(cdev->landing_page, sizeof(cdev->landing_page)) > 0)
> >>> +                     webusb_cap->iLandingPage = 1;
> >>> +             else
> >>> +                     webusb_cap->iLandingPage = 0;
> >>> +     }
> >>> +
> >>>        return le16_to_cpu(bos->wTotalLength);
> >>>    }
> >>>
> >>> @@ -1744,7 +1781,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> >>>                                        cdev->desc.bcdUSB = cpu_to_le16(0x0210);
> >>>                                }
> >>>                        } else {
> >>> -                             if (gadget->lpm_capable)
> >>> +                             if (gadget->lpm_capable || cdev->use_webusb)
> >>>                                        cdev->desc.bcdUSB = cpu_to_le16(0x0201);
> >>>                                else
> >>>                                        cdev->desc.bcdUSB = cpu_to_le16(0x0200);
> >>> @@ -1779,7 +1816,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> >>>                        break;
> >>>                case USB_DT_BOS:
> >>>                        if (gadget_is_superspeed(gadget) ||
> >>> -                         gadget->lpm_capable) {
> >>> +                         gadget->lpm_capable || cdev->use_webusb) {
> >>>                                value = bos_desc(cdev);
> >>>                                value = min(w_length, (u16) value);
> >>>                        }
> >>> @@ -2013,6 +2050,44 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> >>>                        goto check_value;
> >>>                }
> >>>
> >>> +             /*
> >>> +              * webusb descriptor handling, following:
> >>> +              * https://wicg.github.io/webusb/#device-requests
> >>> +              */
> >>> +             if (cdev->use_webusb &&
> >>> +                 (ctrl->bRequestType & USB_TYPE_VENDOR) &&
> >>> +                 ctrl->wIndex == /* WEBUSB_GET_URL*/ 2 &&
> >>> +                 ctrl->bRequest == cdev->b_webusb_vendor_code) {
> >>> +                     /*
> >>> +                      * specification of the url descriptor in WebUSB:
> >>> +                      * https://wicg.github.io/webusb/#webusb-descriptors
> >>> +                      */
> >>> +                     u8              *buf;
> >>> +                     unsigned int    landing_page_length;
> >>> +
> >>> +                     buf = cdev->req->buf;
> >>> +                     buf[1] = /* WEBUSB_URL*/ 3;
> >>
> >> Why not #define WEBUSB_URL?
> >
> > in a previous iteration, I had that exact #define in place, right
> > above. However, another reviewer found it pointless. Maybe I should
> > instead put the define into include/uapi/linux/usb/ch9.h
> >
>
> Hmm. ch9.h had been called ch9.h for a reason. It corresponded to Chapter 9 of
> the USB spec. Adding to it something that by design is outside the spec might
> not be what we want. But I don't know.

Alan also agrees with you.

I will move it to a new header file: include/uapi/linux/usb/webusb.h.

>
> >>
> >>> +
> >>> +                     landing_page_length = strnlen(cdev->landing_page, 252);
> >>> +                     if (strncasecmp("https://", cdev->landing_page, 8) == 0) {
> >>
> >> Maybe it's me, but it would be easier for me to understand why the "8" if the
> >> comparison was in reverse order of arguments, like this:
> >>
> >> strncasecmp(cdev->landing_page, "https://", 8)
> >
> > I agree, this makes the 8 clearer.
> >
> >> because we want the entirety of "https://" compared, not its substring, so the
> >> limit kind of applies to the landing_page in the first place.
> >> Maybe the "8" (and "7" below) can be #define'd, too?
> >
> > depends on the name, but maybe something like
> >
> > #define WEBUSB_PREFIX_HTTPS "https://"
> > #define WEBUSB_PREFIX_HTTPS_LENGTH 8
> > #define WEBUSB_PREFIX_HTTPS_SCHEME_ID 0x01
> >
> > would work defined with the struct in include/uapi/linux/usb/ch9.h
> >
> >>
> >>> +                             buf[2] = 0x01;
> >>
> >> What is this magic 0x01?
> >
> > The field is specified in
> > https://wicg.github.io/webusb/#url-descriptor and would be the
> > WEBUSB_PREFIX_HTTPS_SCHEME_ID from above.
> > But yes, I should #define this.
> >
> >>
> >>> +                             memcpy(buf+3, cdev->landing_page+8, landing_page_length-8);
> >>
> >> spaces around arithmetic operators?
> >
> > will do
> >
> >
> >>> +                             buf[0] = landing_page_length - 8 + 3;
> >>> +                     } else if (strncasecmp("http://", cdev->landing_page, 7) == 0) {
> >>> +                             buf[2] = 0x00;
> >>
> >> Magic 0x00?
> >
> > This would then be WEBUSB_PREFIX_HTTP_SCHEME_ID
> >
> >>
> >>> +                             memcpy(buf+3, cdev->landing_page+7, landing_page_length-7);
> >>> +                             buf[0] = landing_page_length - 7 + 3;
> >>> +                     } else {
> >>> +                             buf[2] = 0xFF;
> >>
> >> Magic 0xFF? (plus I'd expect lowercase hex digits).
> >
> > This would then be WEBUSB_PREFIX_NONE_SCHEME_ID
> >
> >> There's "URL Prefixes" table in 4.3.1 URL Descriptor. Why not define
> >> URL_PREFIX_HTTP/HTTPS/NONE?
> >
> > will do
> >
> >>> +                             memcpy(buf+3, cdev->landing_page, landing_page_length);
> >>> +                             buf[0] = landing_page_length + 3;
> >>> +                     }
> >>> +
> >>> +                     value = buf[0];
> >>> +
> >>> +                     goto check_value;
> >>> +             }
> >>> +
> >>>                VDBG(cdev,
> >>>                        "non-core control req%02x.%02x v%04x i%04x l%d\n",
> >>>                        ctrl->bRequestType, ctrl->bRequest,
> >>> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> >>> index 96121d1c8df4..2b7f01a9efff 100644
> >>> --- a/drivers/usb/gadget/configfs.c
> >>> +++ b/drivers/usb/gadget/configfs.c
> >>> @@ -39,6 +39,7 @@ struct gadget_info {
> >>>        struct config_group configs_group;
> >>>        struct config_group strings_group;
> >>>        struct config_group os_desc_group;
> >>> +     struct config_group webusb_group;
> >>>
> >>>        struct mutex lock;
> >>>        struct usb_gadget_strings *gstrings[MAX_USB_STRING_LANGS + 1];
> >>> @@ -50,6 +51,11 @@ struct gadget_info {
> >>>        bool use_os_desc;
> >>>        char b_vendor_code;
> >>>        char qw_sign[OS_STRING_QW_SIGN_LEN];
> >>> +     bool use_webusb;
> >>> +     u16 bcd_webusb_version;
> >>> +     u8 b_webusb_vendor_code;
> >>> +     char landing_page[255];
> >>
> >> A #define for the size?
> >
> > will do
> >
> >>
> >>> +
> >>>        spinlock_t spinlock;
> >>>        bool unbind;
> >>>    };
> >>> @@ -780,6 +786,131 @@ static void gadget_strings_attr_release(struct config_item *item)
> >>>    USB_CONFIG_STRING_RW_OPS(gadget_strings);
> >>>    USB_CONFIG_STRINGS_LANG(gadget_strings, gadget_info);
> >>>
> >>> +static inline struct gadget_info *webusb_item_to_gadget_info(
> >>> +             struct config_item *item)
> >>> +{
> >>> +     return container_of(to_config_group(item),
> >>> +                     struct gadget_info, webusb_group);
> >>> +}
> >>> +
> >>> +static ssize_t webusb_use_show(struct config_item *item, char *page)
> >>> +{
> >>> +     return sysfs_emit(page, "%d\n",
> >>> +                     webusb_item_to_gadget_info(item)->use_webusb);
> >>> +}
> >>> +
> >>> +static ssize_t webusb_use_store(struct config_item *item, const char *page,
> >>> +                              size_t len)
> >>> +{
> >>> +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
> >>> +     int ret;
> >>> +     bool use;
> >>> +
> >>> +     mutex_lock(&gi->lock);
> >>> +     ret = kstrtobool(page, &use);
> >>> +     if (!ret) {
> >>> +             gi->use_webusb = use;
> >>> +             ret = len;
> >>> +     }
> >>> +     mutex_unlock(&gi->lock);
> >>> +
> >>> +     return ret;
> >>> +}
> >>> +
> >>> +static ssize_t webusb_bcdVersion_show(struct config_item *item, char *page)
> >>> +{
> >>> +     return sysfs_emit(page, "0x%04x\n",
> >>> +                                     webusb_item_to_gadget_info(item)->bcd_webusb_version);
> >>> +}
> >>> +
> >>> +static ssize_t webusb_bcdVersion_store(struct config_item *item,
> >>> +             const char *page, size_t len)
> >>> +{
> >>> +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
> >>> +     u16 bcdVersion;
> >>> +     int ret;
> >>> +
> >>> +     mutex_lock(&gi->lock);
> >>> +     ret = kstrtou16(page, 0, &bcdVersion);
> >>> +     if (ret)
> >>> +             goto out;
> >>> +     ret = is_valid_bcd(bcdVersion);
> >>> +     if (ret)
> >>> +             goto out;
> >>> +
> >>> +     gi->bcd_webusb_version = bcdVersion;
> >>> +     ret = len;
> >>> +
> >>> +out:
> >>> +     mutex_unlock(&gi->lock);
> >>> +
> >>> +     return ret;
> >>> +}
> >>> +
> >>> +static ssize_t webusb_bVendorCode_show(struct config_item *item, char *page)
> >>> +{
> >>> +     return sysfs_emit(page, "0x%02x\n",
> >>> +                     webusb_item_to_gadget_info(item)->b_webusb_vendor_code);
> >>> +}
> >>> +
> >>> +static ssize_t webusb_bVendorCode_store(struct config_item *item,
> >>> +                                        const char *page, size_t len)
> >>> +{
> >>> +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
> >>> +     int ret;
> >>> +     u8 b_vendor_code;
> >>> +
> >>> +     mutex_lock(&gi->lock);
> >>> +     ret = kstrtou8(page, 0, &b_vendor_code);
> >>> +     if (!ret) {
> >>> +             gi->b_webusb_vendor_code = b_vendor_code;
> >>> +             ret = len;
> >>> +     }
> >>> +     mutex_unlock(&gi->lock);
> >>> +
> >>> +     return ret;
> >>> +}
> >>> +
> >>> +static ssize_t webusb_landingPage_show(struct config_item *item, char *page)
> >>> +{
> >>> +     return sysfs_emit(page, "%s\n", webusb_item_to_gadget_info(item)->landing_page);
> >>> +}
> >>> +
> >>> +static ssize_t webusb_landingPage_store(struct config_item *item, const char *page,
> >>> +                                  size_t len)
> >>> +{
> >>> +     struct gadget_info *gi = webusb_item_to_gadget_info(item);
> >>> +     int l;
> >>> +
> >>> +     l = min(len, sizeof(gi->landing_page));
> >>> +     if (page[l - 1] == '\n')
> >>> +             --l;
> >>> +
> >>> +     mutex_lock(&gi->lock);
> >>> +     memcpy(gi->landing_page, page, l);
> >>> +     mutex_unlock(&gi->lock);
> >>> +
> >>> +     return len;
> >>> +}
> >>> +
> >>> +CONFIGFS_ATTR(webusb_, use);
> >>> +CONFIGFS_ATTR(webusb_, bVendorCode);
> >>> +CONFIGFS_ATTR(webusb_, bcdVersion);
> >>> +CONFIGFS_ATTR(webusb_, landingPage);
> >>> +
> >>> +static struct configfs_attribute *webusb_attrs[] = {
> >>> +     &webusb_attr_use,
> >>> +     &webusb_attr_bcdVersion,
> >>> +     &webusb_attr_bVendorCode,
> >>> +     &webusb_attr_landingPage,
> >>> +     NULL,
> >>> +};
> >>> +
> >>> +static struct config_item_type webusb_type = {
> >>> +     .ct_attrs       = webusb_attrs,
> >>> +     .ct_owner       = THIS_MODULE,
> >>> +};
> >>> +
> >>>    static inline struct gadget_info *os_desc_item_to_gadget_info(
> >>>                struct config_item *item)
> >>>    {
> >>> @@ -1341,6 +1472,16 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
> >>>                gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
> >>>        }
> >>>
> >>> +     if (gi->use_webusb) {
> >>> +             cdev->use_webusb = true;
> >>> +             cdev->bcd_webusb_version = gi->bcd_webusb_version;
> >>> +             cdev->b_webusb_vendor_code = gi->b_webusb_vendor_code;
> >>> +             memcpy(cdev->landing_page, gi->landing_page,
> >>> +                             strnlen(gi->landing_page,
> >>> +                                             min(sizeof(cdev->landing_page),
> >>> +                                                     sizeof(gi->landing_page))));
> >>
> >> checkpatch now allows 100 colums. Plus strnlen() looks indented too far to the
> >> right.
> >
> > I will check this.
> >
> >>
> >> The sizeofs are both 255. Are they ever expected to be different? Maybe not?
> >> Then a #defined constant ensures they are equal. Then there's no need to find
> >> a minimum among equal values.
> >
> > Technically, if the url has the empty scheme, it can only be 252 bytes
> > long, since the maximum descriptor length is 255 because bLength is a
> > single byte, but 3 bytes are in the header of this descriptor, leaving
> > 252 for the string.
>
> But you use sizeof() rather than strlen(). And the "landing_page" members
> are defined as 255-element arrays of chars, so both sizeof() invocations
> will yield 255, no?

You are correct. I was just thinking of the exact length limitations
when you wrote your comment. Before that I hadn't really considered
the different lengths based on the scheme, so this version of the
patch does not yet make this distinction.

Thanks for your patience with a first-time contributor.

Regards,
Jó

>
> Andrzej
>
> >
> > However, if the landing page scheme is "https://", the maximum length
> > of this string could actually be 255 - 3 /*header*/ + 8 /* the length
> > of the prefix, which is stripped */ => 260 bytes.
> >
> > I need to rework the validation here a bit. I would like to avoid for
> > userspace to explicitly set the scheme constants directly, as just
> > piping in the string seems so much more ergonomic.
> >
> >>
> >> Regards,
> >>
> >> Andrzej
> >
> > Thanks a lot for your review!
> >
> > I hope my response to your initial concerns was not too long and windy.
> >
> > I will post an updated version of my patch within the next few days.
> >
> > Regards,
> > Jó
> >
> >
> >>
> >>> +     }
> >>> +
> >>>        if (gi->use_os_desc) {
> >>>                cdev->use_os_string = true;
> >>>                cdev->b_vendor_code = gi->b_vendor_code;
> >>> @@ -1605,6 +1746,10 @@ static struct config_group *gadgets_make(
> >>>                        &os_desc_type);
> >>>        configfs_add_default_group(&gi->os_desc_group, &gi->group);
> >>>
> >>> +     config_group_init_type_name(&gi->webusb_group, "webusb",
> >>> +                     &webusb_type);
> >>> +     configfs_add_default_group(&gi->webusb_group, &gi->group);
> >>> +
> >>>        gi->composite.bind = configfs_do_nothing;
> >>>        gi->composite.unbind = configfs_do_nothing;
> >>>        gi->composite.suspend = NULL;
> >>> diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
> >>> index 43ac3fa760db..eb6fac5bbcde 100644
> >>> --- a/include/linux/usb/composite.h
> >>> +++ b/include/linux/usb/composite.h
> >>> @@ -474,6 +474,12 @@ struct usb_composite_dev {
> >>>        struct usb_configuration        *os_desc_config;
> >>>        unsigned int                    use_os_string:1;
> >>>
> >>> +     /* WebUSB */
> >>> +     u16                             bcd_webusb_version;
> >>> +     u8                              b_webusb_vendor_code;
> >>> +     char                            landing_page[255];
> >>> +     unsigned int                    use_webusb:1;
> >>> +
> >>>        /* private: */
> >>>        /* internals */
> >>>        unsigned int                    suspended:1;
> >>> diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
> >>> index 31fcfa084e63..3a36550297bc 100644
> >>> --- a/include/uapi/linux/usb/ch9.h
> >>> +++ b/include/uapi/linux/usb/ch9.h
> >>> @@ -947,6 +947,39 @@ struct usb_ss_container_id_descriptor {
> >>>
> >>>    #define USB_DT_USB_SS_CONTN_ID_SIZE 20
> >>>
> >>> +/*
> >>> + * Platform Device Capability descriptor: Defines platform specific device
> >>> + * capabilities
> >>> + */
> >>> +#define      USB_PLAT_DEV_CAP_TYPE   5
> >>> +struct usb_plat_dev_cap_descriptor {
> >>> +     __u8  bLength;
> >>> +     __u8  bDescriptorType;
> >>> +     __u8  bDevCapabilityType;
> >>> +     __u8  bReserved;
> >>> +     __u8  UUID[16];
> >>> +} __attribute__((packed));
> >>> +
> >>> +#define USB_DT_USB_PLAT_DEV_CAP_SIZE 20
> >>> +
> >>> +/*
> >>> + * WebUSB Platform Capability descriptor: A device announces support for the
> >>> + * WebUSB command set by including the following Platform Descriptor in its
> >>> + * Binary Object Store
> >>> + * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
> >>> + */
> >>> +struct usb_webusb_cap_descriptor {
> >>> +     __u8  bLength;
> >>> +     __u8  bDescriptorType;
> >>> +     __u8  bDevCapabilityType;
> >>> +     __u8  bReserved;
> >>> +     __u8  UUID[16];
> >>> +     __u16 bcdVersion;
> >>> +     __u8  bVendorCode;
> >>> +     __u8  iLandingPage;
> >>> +} __attribute__((packed));
> >>> +#define USB_DT_WEBUSB_SIZE   (USB_DT_USB_PLAT_DEV_CAP_SIZE + 4)
> >>> +
> >>>    /*
> >>>     * SuperSpeed Plus USB Capability descriptor: Defines the set of
> >>>     * SuperSpeed Plus USB specific device level capabilities
> >>
>

Patch

diff --git a/Documentation/ABI/testing/configfs-usb-gadget b/Documentation/ABI/testing/configfs-usb-gadget
index b7943aa7e997..8399e0f0ed1c 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget
+++ b/Documentation/ABI/testing/configfs-usb-gadget
@@ -143,3 +143,16 @@  Description:
 		qw_sign		an identifier to be reported as "OS String"
 				proper
 		=============	===============================================
+
+What:		/config/usb-gadget/gadget/webusb
+Date:		Dec 2022
+KernelVersion:	6.2
+Description:
+		This group contains "WebUSB" extension handling attributes.
+
+		=============	===============================================
+		use		flag turning "WebUSB" support on/off
+		bcdVersion	bcd WebUSB specification version number
+		b_vendor_code	one-byte value used for custom per-device
+		landing_page	UTF-8 encoded URL of the device's landing page
+		=============	===============================================
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 403563c06477..9b209e69442d 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -14,6 +14,7 @@ 
 #include <linux/device.h>
 #include <linux/utsname.h>
 #include <linux/bitfield.h>
+#include <linux/uuid.h>
 
 #include <linux/usb/composite.h>
 #include <linux/usb/otg.h>
@@ -713,14 +714,16 @@  static int bos_desc(struct usb_composite_dev *cdev)
 	 * A SuperSpeed device shall include the USB2.0 extension descriptor
 	 * and shall support LPM when operating in USB2.0 HS mode.
 	 */
-	usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
-	bos->bNumDeviceCaps++;
-	le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
-	usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
-	usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
-	usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
-	usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
-					    USB_BESL_SUPPORT | besl);
+	if (cdev->gadget->lpm_capable) {
+		usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
+		bos->bNumDeviceCaps++;
+		le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
+		usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
+		usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
+		usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
+		usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
+							USB_BESL_SUPPORT | besl);
+	}
 
 	/*
 	 * The Superspeed USB Capability descriptor shall be implemented by all
@@ -821,6 +824,40 @@  static int bos_desc(struct usb_composite_dev *cdev)
 		}
 	}
 
+	/*
+	 * Following the specifaction at:
+	 * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
+	 */
+	if (cdev->use_webusb) {
+		struct usb_webusb_cap_descriptor *webusb_cap;
+		/*
+		 * little endian PlatformCapablityUUID for WebUSB:
+		 * 3408b638-09a9-47a0-8bfd-a0768815b665
+		 */
+		uuid_t webusb_uuid = UUID_INIT(0x38b60834, 0xa909, 0xa047, 0x8b, 0xfd, 0xa0, 0x76,
+						0x88, 0x15, 0xb6, 0x65);
+
+		webusb_cap = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
+		bos->bNumDeviceCaps++;
+
+		le16_add_cpu(&bos->wTotalLength, USB_DT_WEBUSB_SIZE);
+		webusb_cap->bLength = USB_DT_WEBUSB_SIZE;
+		webusb_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
+		webusb_cap->bDevCapabilityType = USB_PLAT_DEV_CAP_TYPE;
+		webusb_cap->bReserved = 0;
+		export_uuid(webusb_cap->UUID, &webusb_uuid);
+		if (cdev->bcd_webusb_version != 0)
+			webusb_cap->bcdVersion = cpu_to_le16(cdev->bcd_webusb_version);
+		else
+			webusb_cap->bcdVersion = cpu_to_le16(0x0100);
+
+		webusb_cap->bVendorCode = cdev->b_webusb_vendor_code;
+		if (strnlen(cdev->landing_page, sizeof(cdev->landing_page)) > 0)
+			webusb_cap->iLandingPage = 1;
+		else
+			webusb_cap->iLandingPage = 0;
+	}
+
 	return le16_to_cpu(bos->wTotalLength);
 }
 
@@ -1744,7 +1781,7 @@  composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 					cdev->desc.bcdUSB = cpu_to_le16(0x0210);
 				}
 			} else {
-				if (gadget->lpm_capable)
+				if (gadget->lpm_capable || cdev->use_webusb)
 					cdev->desc.bcdUSB = cpu_to_le16(0x0201);
 				else
 					cdev->desc.bcdUSB = cpu_to_le16(0x0200);
@@ -1779,7 +1816,7 @@  composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 			break;
 		case USB_DT_BOS:
 			if (gadget_is_superspeed(gadget) ||
-			    gadget->lpm_capable) {
+			    gadget->lpm_capable || cdev->use_webusb) {
 				value = bos_desc(cdev);
 				value = min(w_length, (u16) value);
 			}
@@ -2013,6 +2050,44 @@  composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 			goto check_value;
 		}
 
+		/*
+		 * webusb descriptor handling, following:
+		 * https://wicg.github.io/webusb/#device-requests
+		 */
+		if (cdev->use_webusb &&
+		    (ctrl->bRequestType & USB_TYPE_VENDOR) &&
+		    ctrl->wIndex == /* WEBUSB_GET_URL*/ 2 &&
+		    ctrl->bRequest == cdev->b_webusb_vendor_code) {
+			/*
+			 * specification of the url descriptor in WebUSB:
+			 * https://wicg.github.io/webusb/#webusb-descriptors
+			 */
+			u8		*buf;
+			unsigned int	landing_page_length;
+
+			buf = cdev->req->buf;
+			buf[1] = /* WEBUSB_URL*/ 3;
+
+			landing_page_length = strnlen(cdev->landing_page, 252);
+			if (strncasecmp("https://", cdev->landing_page, 8) == 0) {
+				buf[2] = 0x01;
+				memcpy(buf+3, cdev->landing_page+8, landing_page_length-8);
+				buf[0] = landing_page_length - 8 + 3;
+			} else if (strncasecmp("http://", cdev->landing_page, 7) == 0) {
+				buf[2] = 0x00;
+				memcpy(buf+3, cdev->landing_page+7, landing_page_length-7);
+				buf[0] = landing_page_length - 7 + 3;
+			} else {
+				buf[2] = 0xFF;
+				memcpy(buf+3, cdev->landing_page, landing_page_length);
+				buf[0] = landing_page_length + 3;
+			}
+
+			value = buf[0];
+
+			goto check_value;
+		}
+
 		VDBG(cdev,
 			"non-core control req%02x.%02x v%04x i%04x l%d\n",
 			ctrl->bRequestType, ctrl->bRequest,
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 96121d1c8df4..2b7f01a9efff 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -39,6 +39,7 @@  struct gadget_info {
 	struct config_group configs_group;
 	struct config_group strings_group;
 	struct config_group os_desc_group;
+	struct config_group webusb_group;
 
 	struct mutex lock;
 	struct usb_gadget_strings *gstrings[MAX_USB_STRING_LANGS + 1];
@@ -50,6 +51,11 @@  struct gadget_info {
 	bool use_os_desc;
 	char b_vendor_code;
 	char qw_sign[OS_STRING_QW_SIGN_LEN];
+	bool use_webusb;
+	u16 bcd_webusb_version;
+	u8 b_webusb_vendor_code;
+	char landing_page[255];
+
 	spinlock_t spinlock;
 	bool unbind;
 };
@@ -780,6 +786,131 @@  static void gadget_strings_attr_release(struct config_item *item)
 USB_CONFIG_STRING_RW_OPS(gadget_strings);
 USB_CONFIG_STRINGS_LANG(gadget_strings, gadget_info);
 
+static inline struct gadget_info *webusb_item_to_gadget_info(
+		struct config_item *item)
+{
+	return container_of(to_config_group(item),
+			struct gadget_info, webusb_group);
+}
+
+static ssize_t webusb_use_show(struct config_item *item, char *page)
+{
+	return sysfs_emit(page, "%d\n",
+			webusb_item_to_gadget_info(item)->use_webusb);
+}
+
+static ssize_t webusb_use_store(struct config_item *item, const char *page,
+				 size_t len)
+{
+	struct gadget_info *gi = webusb_item_to_gadget_info(item);
+	int ret;
+	bool use;
+
+	mutex_lock(&gi->lock);
+	ret = kstrtobool(page, &use);
+	if (!ret) {
+		gi->use_webusb = use;
+		ret = len;
+	}
+	mutex_unlock(&gi->lock);
+
+	return ret;
+}
+
+static ssize_t webusb_bcdVersion_show(struct config_item *item, char *page)
+{
+	return sysfs_emit(page, "0x%04x\n",
+					webusb_item_to_gadget_info(item)->bcd_webusb_version);
+}
+
+static ssize_t webusb_bcdVersion_store(struct config_item *item,
+		const char *page, size_t len)
+{
+	struct gadget_info *gi = webusb_item_to_gadget_info(item);
+	u16 bcdVersion;
+	int ret;
+
+	mutex_lock(&gi->lock);
+	ret = kstrtou16(page, 0, &bcdVersion);
+	if (ret)
+		goto out;
+	ret = is_valid_bcd(bcdVersion);
+	if (ret)
+		goto out;
+
+	gi->bcd_webusb_version = bcdVersion;
+	ret = len;
+
+out:
+	mutex_unlock(&gi->lock);
+
+	return ret;
+}
+
+static ssize_t webusb_bVendorCode_show(struct config_item *item, char *page)
+{
+	return sysfs_emit(page, "0x%02x\n",
+			webusb_item_to_gadget_info(item)->b_webusb_vendor_code);
+}
+
+static ssize_t webusb_bVendorCode_store(struct config_item *item,
+					   const char *page, size_t len)
+{
+	struct gadget_info *gi = webusb_item_to_gadget_info(item);
+	int ret;
+	u8 b_vendor_code;
+
+	mutex_lock(&gi->lock);
+	ret = kstrtou8(page, 0, &b_vendor_code);
+	if (!ret) {
+		gi->b_webusb_vendor_code = b_vendor_code;
+		ret = len;
+	}
+	mutex_unlock(&gi->lock);
+
+	return ret;
+}
+
+static ssize_t webusb_landingPage_show(struct config_item *item, char *page)
+{
+	return sysfs_emit(page, "%s\n", webusb_item_to_gadget_info(item)->landing_page);
+}
+
+static ssize_t webusb_landingPage_store(struct config_item *item, const char *page,
+				     size_t len)
+{
+	struct gadget_info *gi = webusb_item_to_gadget_info(item);
+	int l;
+
+	l = min(len, sizeof(gi->landing_page));
+	if (page[l - 1] == '\n')
+		--l;
+
+	mutex_lock(&gi->lock);
+	memcpy(gi->landing_page, page, l);
+	mutex_unlock(&gi->lock);
+
+	return len;
+}
+
+CONFIGFS_ATTR(webusb_, use);
+CONFIGFS_ATTR(webusb_, bVendorCode);
+CONFIGFS_ATTR(webusb_, bcdVersion);
+CONFIGFS_ATTR(webusb_, landingPage);
+
+static struct configfs_attribute *webusb_attrs[] = {
+	&webusb_attr_use,
+	&webusb_attr_bcdVersion,
+	&webusb_attr_bVendorCode,
+	&webusb_attr_landingPage,
+	NULL,
+};
+
+static struct config_item_type webusb_type = {
+	.ct_attrs	= webusb_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
 static inline struct gadget_info *os_desc_item_to_gadget_info(
 		struct config_item *item)
 {
@@ -1341,6 +1472,16 @@  static int configfs_composite_bind(struct usb_gadget *gadget,
 		gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
 	}
 
+	if (gi->use_webusb) {
+		cdev->use_webusb = true;
+		cdev->bcd_webusb_version = gi->bcd_webusb_version;
+		cdev->b_webusb_vendor_code = gi->b_webusb_vendor_code;
+		memcpy(cdev->landing_page, gi->landing_page,
+				strnlen(gi->landing_page,
+						min(sizeof(cdev->landing_page),
+							sizeof(gi->landing_page))));
+	}
+
 	if (gi->use_os_desc) {
 		cdev->use_os_string = true;
 		cdev->b_vendor_code = gi->b_vendor_code;
@@ -1605,6 +1746,10 @@  static struct config_group *gadgets_make(
 			&os_desc_type);
 	configfs_add_default_group(&gi->os_desc_group, &gi->group);
 
+	config_group_init_type_name(&gi->webusb_group, "webusb",
+			&webusb_type);
+	configfs_add_default_group(&gi->webusb_group, &gi->group);
+
 	gi->composite.bind = configfs_do_nothing;
 	gi->composite.unbind = configfs_do_nothing;
 	gi->composite.suspend = NULL;
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 43ac3fa760db..eb6fac5bbcde 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -474,6 +474,12 @@  struct usb_composite_dev {
 	struct usb_configuration	*os_desc_config;
 	unsigned int			use_os_string:1;
 
+	/* WebUSB */
+	u16				bcd_webusb_version;
+	u8				b_webusb_vendor_code;
+	char				landing_page[255];
+	unsigned int			use_webusb:1;
+
 	/* private: */
 	/* internals */
 	unsigned int			suspended:1;
diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
index 31fcfa084e63..3a36550297bc 100644
--- a/include/uapi/linux/usb/ch9.h
+++ b/include/uapi/linux/usb/ch9.h
@@ -947,6 +947,39 @@  struct usb_ss_container_id_descriptor {
 
 #define USB_DT_USB_SS_CONTN_ID_SIZE	20
 
+/*
+ * Platform Device Capability descriptor: Defines platform specific device
+ * capabilities
+ */
+#define	USB_PLAT_DEV_CAP_TYPE	5
+struct usb_plat_dev_cap_descriptor {
+	__u8  bLength;
+	__u8  bDescriptorType;
+	__u8  bDevCapabilityType;
+	__u8  bReserved;
+	__u8  UUID[16];
+} __attribute__((packed));
+
+#define USB_DT_USB_PLAT_DEV_CAP_SIZE	20
+
+/*
+ * WebUSB Platform Capability descriptor: A device announces support for the
+ * WebUSB command set by including the following Platform Descriptor in its
+ * Binary Object Store
+ * https://wicg.github.io/webusb/#webusb-platform-capability-descriptor
+ */
+struct usb_webusb_cap_descriptor {
+	__u8  bLength;
+	__u8  bDescriptorType;
+	__u8  bDevCapabilityType;
+	__u8  bReserved;
+	__u8  UUID[16];
+	__u16 bcdVersion;
+	__u8  bVendorCode;
+	__u8  iLandingPage;
+} __attribute__((packed));
+#define USB_DT_WEBUSB_SIZE	(USB_DT_USB_PLAT_DEV_CAP_SIZE + 4)
+
 /*
  * SuperSpeed Plus USB Capability descriptor: Defines the set of
  * SuperSpeed Plus USB specific device level capabilities