linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: modalias: Added a manufacturer string to usb modalias sysfs output
@ 2025-07-06 14:27 Daniele Barcella
  2025-07-07  8:00 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 2+ messages in thread
From: Daniele Barcella @ 2025-07-06 14:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Daniele Barcella, linux-usb, linux-kernel

The patch improves the usb modalias sysfs output by including the manufacturer string ath the end in the format 'mnf%s' after filtering the string, like how it is done for the dmi id device.

For example usb:v0483p5740d0100dcEFdsc02dp01ic02isc02ip00in00mnfFlipperDevicesInc.

This change allows hwdb rules to target devices with the same vid+pid but different manufacturer.
For example, the STMicroelectronics Virtual COM Port (0483:5740) is widely used in both prototyping and production devices that don't have a unique vendor ID.

For further context, refer to the related discussion on systemd's GitHub: https://github.com/systemd/systemd/pull/24869.

Signed-off-by: Daniele Barcella <daniele.barcella@pm.me>
---
 drivers/usb/core/sysfs.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index 23f3cb1989f4..dc191fa94372 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -1154,20 +1154,39 @@ static ssize_t interface_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(interface);
 
+static void ascii_filter(char *d, const char *s)
+{
+	/* Filter out characters we don't want to see in the modalias string */
+	for (; *s; s++)
+		if (*s > ' ' && *s < 127 && *s != ':')
+			*(d++) = *s;
+
+	*d = 0;
+}
+
 static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
 	struct usb_interface *intf;
 	struct usb_device *udev;
 	struct usb_host_interface *alt;
+	char *manufacturer;
+	int emit;
 
 	intf = to_usb_interface(dev);
 	udev = interface_to_usbdev(intf);
 	alt = READ_ONCE(intf->cur_altsetting);
 
-	return sysfs_emit(buf,
+	if (udev->manufacturer) {
+		manufacturer = kmalloc(strlen(udev->manufacturer) + 1, GFP_KERNEL);
+		ascii_filter(manufacturer, udev->manufacturer);
+	} else {
+		manufacturer = kstrdup("", GFP_KERNEL);
+	}
+
+	emit = sysfs_emit(buf,
 			"usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02X"
-			"ic%02Xisc%02Xip%02Xin%02X\n",
+			"ic%02Xisc%02Xip%02Xin%02Xmnf%s\n",
 			le16_to_cpu(udev->descriptor.idVendor),
 			le16_to_cpu(udev->descriptor.idProduct),
 			le16_to_cpu(udev->descriptor.bcdDevice),
@@ -1177,7 +1196,11 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
 			alt->desc.bInterfaceClass,
 			alt->desc.bInterfaceSubClass,
 			alt->desc.bInterfaceProtocol,
-			alt->desc.bInterfaceNumber);
+			alt->desc.bInterfaceNumber,
+			manufacturer
+		);
+	kfree(manufacturer);
+	return emit;
 }
 static DEVICE_ATTR_RO(modalias);
 
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] usb: modalias: Added a manufacturer string to usb modalias sysfs output
  2025-07-06 14:27 [PATCH] usb: modalias: Added a manufacturer string to usb modalias sysfs output Daniele Barcella
@ 2025-07-07  8:00 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2025-07-07  8:00 UTC (permalink / raw)
  To: Daniele Barcella; +Cc: linux-usb, linux-kernel

On Sun, Jul 06, 2025 at 02:27:26PM +0000, Daniele Barcella wrote:
> The patch improves the usb modalias sysfs output by including the manufacturer string ath the end in the format 'mnf%s' after filtering the string, like how it is done for the dmi id device.
> 
> For example usb:v0483p5740d0100dcEFdsc02dp01ic02isc02ip00in00mnfFlipperDevicesInc.
> 
> This change allows hwdb rules to target devices with the same vid+pid but different manufacturer.
> For example, the STMicroelectronics Virtual COM Port (0483:5740) is widely used in both prototyping and production devices that don't have a unique vendor ID.

Nit, please use 72 columns for your changelog.

> 
> For further context, refer to the related discussion on systemd's GitHub: https://github.com/systemd/systemd/pull/24869.

As random links can disappear at any time, and the text in a kernel
changelog will endure for forever, please put all relevant information
in the changelog itself, don't require people to go look elesewhere and
hope it is still relevant.

> 
> Signed-off-by: Daniele Barcella <daniele.barcella@pm.me>
> ---
>  drivers/usb/core/sysfs.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
> index 23f3cb1989f4..dc191fa94372 100644
> --- a/drivers/usb/core/sysfs.c
> +++ b/drivers/usb/core/sysfs.c
> @@ -1154,20 +1154,39 @@ static ssize_t interface_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(interface);
>  
> +static void ascii_filter(char *d, const char *s)
> +{
> +	/* Filter out characters we don't want to see in the modalias string */
> +	for (; *s; s++)
> +		if (*s > ' ' && *s < 127 && *s != ':')
> +			*(d++) = *s;
> +
> +	*d = 0;
> +}

We don't have a common kernel function for this?


> +
>  static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
>  			     char *buf)
>  {
>  	struct usb_interface *intf;
>  	struct usb_device *udev;
>  	struct usb_host_interface *alt;
> +	char *manufacturer;
> +	int emit;
>  
>  	intf = to_usb_interface(dev);
>  	udev = interface_to_usbdev(intf);
>  	alt = READ_ONCE(intf->cur_altsetting);
>  
> -	return sysfs_emit(buf,
> +	if (udev->manufacturer) {
> +		manufacturer = kmalloc(strlen(udev->manufacturer) + 1, GFP_KERNEL);
> +		ascii_filter(manufacturer, udev->manufacturer);
> +	} else {
> +		manufacturer = kstrdup("", GFP_KERNEL);
> +	}
> +
> +	emit = sysfs_emit(buf,
>  			"usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02X"
> -			"ic%02Xisc%02Xip%02Xin%02X\n",
> +			"ic%02Xisc%02Xip%02Xin%02Xmnf%s\n",
>  			le16_to_cpu(udev->descriptor.idVendor),
>  			le16_to_cpu(udev->descriptor.idProduct),
>  			le16_to_cpu(udev->descriptor.bcdDevice),
> @@ -1177,7 +1196,11 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
>  			alt->desc.bInterfaceClass,
>  			alt->desc.bInterfaceSubClass,
>  			alt->desc.bInterfaceProtocol,
> -			alt->desc.bInterfaceNumber);
> +			alt->desc.bInterfaceNumber,
> +			manufacturer
> +		);
> +	kfree(manufacturer);
> +	return emit;

I really do not like this change, sorry.  Vendors are supposed to go and
get their own product id if they are a vendor-specific device, they
should NOT be attempting to trigger off of USB device strings as those
are not guaranteed to relate to anything at all.

This adds a new user/kernel api, just to work around a userspace tool
that can be easily modified to handle this one specific broken device
that is lying about its vendor/product id.  Please go fix this in
userspace with a simple script that udev can run to determine what the
device string contains, and don't require the kernel to do this work
instead.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-07-07  8:00 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-06 14:27 [PATCH] usb: modalias: Added a manufacturer string to usb modalias sysfs output Daniele Barcella
2025-07-07  8:00 ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).