file2alias - incorrect? aliases for USB

Andrey Borzenkov

author

2003-11-09 18:55 UTC #0 lore raw

file2aliases puts in alias device ID high and low numbers directly from match specifications. E.g. for this match table entry:

usb-storage 0x000f 0x04e6 0x0006 0x0100 0x0205 ...

it generates alias

alias usb:v04E6p0006dl0100dh0205dc*dsc*dp*ic*isc*ip* usb_storage

unfortunately real device attribute does not include high and low - rather it has single device ID (as part of PRODUCT) that should be contained in these bounds:

        length += snprintf (scratch, buffer_size - length, "PRODUCT=%x/%x/%x",
                            usb_dev->descriptor.idVendor,
                            usb_dev->descriptor.idProduct,
                            usb_dev->descriptor.bcdDevice);

or bcdDevice file in sysfs.

This makes those aliases rather useless for the purpose of matching reported device. It may take the same route as PCI and reject all device ID table entries that have High != Low but there are quite a few of them available.

I am rather confused because I do not see how this condition (low <= bcdDevice <= high) can be expressed using simple glob pattern (unless we are going to take glob library from Zsh :)

thank you

-andrey

-------------------------------------------------------
This SF.Net email sponsored by: ApacheCon 2003,
16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Greg KH

2003-11-10 09:37 UTC #1 lore raw

Quoted 20 lines · Andrey Borzenkov

file2aliases puts in alias device ID high and low numbers directly from match specifications. E.g. for this match table entry:
usb-storage 0x000f 0x04e6 0x0006 0x0100 0x0205 ...
it generates alias
alias usb:v04E6p0006dl0100dh0205dc*dsc*dp*ic*isc*ip* usb_storage
unfortunately real device attribute does not include high and low - rather it has single device ID (as part of PRODUCT) that should be contained in these bounds:
        length += snprintf (scratch, buffer_size - length, "PRODUCT=%x/%x/%x",
                            usb_dev->descriptor.idVendor,
                            usb_dev->descriptor.idProduct,
                            usb_dev->descriptor.bcdDevice);
or bcdDevice file in sysfs.

Quoted 7 lines

This makes those aliases rather useless for the purpose of matching reported device. It may take the same route as PCI and reject all device ID table entries that have High != Low but there are quite a few of them available.
I am rather confused because I do not see how this condition (low <= bcdDevice <= high) can be expressed using simple glob pattern (unless we are going to take glob library from Zsh :)

I would suggest just ignoring the bcdDevice value, and loading all modules that match the idVendor and idProduct values, and let the kernel sort it out :)

So for your example, you would just:

	modprobe usb:v04E6p0006dl*dh*dc*dsc*dp*ic*isc*ip*

Hm, but that's no good either, because the visor driver trips over that with its entry:

	MODULE_ALIAS("usb:v*p*dl*dh*dc*dsc*dp*ic*isc*ip*");

and the improper module is loaded. That needs to be fixed up...

Rusty, any reason why the module alias code is turning an empty MODULE_PARAM structure, as is declared in drivers/usb/serial/visor.c with the line:
{ }, /* optional parameter entry */

Into the above MODULE_ALIAS? I don't think that's correct.

thanks,

greg k-h

-------------------------------------------------------
This SF.Net email sponsored by: ApacheCon 2003,
16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Greg KH

2003-11-14 01:02 UTC #2 lore raw

In reply to Andrey Borzenkov

Quoted 10 lines · Andrey Borzenkov

Quoted 7 lines
I would suggest just ignoring the bcdDevice value, and loading all modules that match the idVendor and idProduct values, and let the kernel sort it out :)
So for your example, you would just:
	modprobe usb:v04E6p0006dl*dh*dc*dsc*dp*ic*isc*ip* 
any reason we put in alias fields that apparently won't be used at all?

Hm, don't know. I didn't think of these as ranges, which some of the fields are. Possibly we might want to drop those variables from the strings.

Quoted 10 lines

Quoted 10 lines
Hm, but that's no good either, because the visor driver trips over that with its entry:
	MODULE_ALIAS("usb:v*p*dl*dh*dc*dsc*dp*ic*isc*ip*");
and the improper module is loaded. That needs to be fixed up...
Rusty, any reason why the module alias code is turning an empty MODULE_PARAM structure, as is declared in drivers/usb/serial/visor.c with the line:
{ }, /* optional parameter entry */

Quoted 6 lines

Quoted 2 lines
Into the above MODULE_ALIAS? I don't think that's correct.
Subject: [PATCH][2.6.0-test9] prevent catch-all USB aliases in modules.alias
http://marc.theaimsgroup.com/?l=linux-kernel&m6787897124700&w=2

Ah, missed that. Care to send it to Rusty? It looks like a simple, needed fix.

thanks,

greg k-h

-------------------------------------------------------
This SF.Net email sponsored by: ApacheCon 2003,
16-19 November in Las Vegas. Learn firsthand the latest developments in Apache, PHP, Perl, XML, Java, MySQL, WebDAV, and more! http://www.apachecon.com/
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Rusty Russell

2003-11-17 03:09 UTC #3 lore raw

In reply to Andrey Borzenkov

In message <[email protected]> you write:

Quoted 11 lines

Hm, but that's no good either, because the visor driver trips over that with its entry:
	MODULE_ALIAS("usb:v*p*dl*dh*dc*dsc*dp*ic*isc*ip*");
and the improper module is loaded. That needs to be fixed up...
Rusty, any reason why the module alias code is turning an empty MODULE_PARAM structure, as is declared in drivers/usb/serial/visor.c with the line:
{ }, /* optional parameter entry */
Into the above MODULE_ALIAS? I don't think that's correct.

Sorry for the delay, was away and am catching up on mail.

Thanks to Andrey for the fix. Greg, did you want to add something else? Either way, please forward to Linus.

Andrey: the reason everything is in there is I didn't know what Greg wanted. He OK'd it, but I'm happy for them to be trimmed, too.

Subject:  [PATCH][2.6.0-test9] prevent catch-all USB aliases in modules.alias
From:     Andrey Borzenkov
Date:     2003-11-03 16:45:03

visor.c defines one empty slot in USB ids table that can be filled in at runtime using module parameters. file2alias generates catch-all alias for it:

alias usb:v*p*dl*dh*dc*dsc*dp*ic*isc*ip* visor

patch adds the same sanity check as in depmod to scripts/file2alias.

regards

-andrey

*["2.6.0-test9-file2alias_visor.patch" (text/x-diff)]*

scripts/file2alias.c renamed from tmp/linux-2.6.0-test9/scripts/file2alias.c +7 -0

@@ -52,6 +52,13 @@ static int do_usb_entry(const char *file
 	id->bcdDevice_lo = TO_NATIVE(id->bcdDevice_lo);
 	id->bcdDevice_hi = TO_NATIVE(id->bcdDevice_hi);
 
+	/*
+	 * Some modules (visor) have empty slots as placeholder for
+	 * run-time specification that results in catch-all alias
+	 */
+	if (!(id->idVendor | id->bDeviceClass | id->bInterfaceClass))
+		return 1;
+
 	strcpy(alias, "usb:");
 	ADD(alias, "v", id->match_flags&USB_DEVICE_ID_MATCH_VENDOR,
 	    id->idVendor);

--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

-------------------------------------------------------
This SF. Net email is sponsored by: GoToMyPC
GoToMyPC is the fast, easy and secure way to access your computer from any Web browser or wireless device. Click here to Try it Free!
https://www.gotomypc.com/tr/OSDN/AW/Q4_2003/t/g22lp?Target=mm/g22lp.tmpl
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Andrey Borzenkov

author

2003-11-17 18:11 UTC #4 lore raw

In reply to Andrey Borzenkov

Quoted 11 lines · Greg KH

Quoted 10 lines · Andrey Borzenkov
file2aliases puts in alias device ID high and low numbers directly from match specifications. E.g. for this match table entry:
usb-storage 0x000f 0x04e6 0x0006 0x0100 0x0205
...
it generates alias
alias usb:v04E6p0006dl0100dh0205dc*dsc*dp*ic*isc*ip* usb_storage

[...]

Quoted 5 lines

I would suggest just ignoring the bcdDevice value, and loading all modules that match the idVendor and idProduct values, and let the kernel sort it out :)

the obvious point that this leaves unneeded modules in kernel aside ...

that won't work that easy. You have to build name to match line from modules.alias. But to match you have to put exact values for dl' and dh'. Those values are simply not available when hotplug agent is called.

i.e. for the above line to match you have to supply exact values for vendor, product, device low, device high. Any other values are not important but these must be exact. But neither device low nor device high are known.

apparently the only way is to remove them and hope that usually the same vendor/product combination is handled by single driver.

Quoted 3 lines

So for your example, you would just:
	modprobe usb:v04E6p0006dl*dh*dc*dsc*dp*ic*isc*ip*

that won't match, sorry. wildcards can be in aliases; they can't be in module names.

-andrey

-------------------------------------------------------
This SF. Net email is sponsored by: GoToMyPC
GoToMyPC is the fast, easy and secure way to access your computer from any Web browser or wireless device. Click here to Try it Free!
https://www.gotomypc.com/tr/OSDN/AW/Q4_2003/t/g22lp?Target=mm/g22lp.tmpl
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Greg KH

2003-12-11 09:29 UTC #5 lore raw

In reply to Andrey Borzenkov

Quoted 11 lines · Rusty Russell

In message <[email protected]> you write:
Quoted 10 lines
Hm, but that's no good either, because the visor driver trips over that with its entry:
	MODULE_ALIAS("usb:v*p*dl*dh*dc*dsc*dp*ic*isc*ip*");
and the improper module is loaded. That needs to be fixed up...
Rusty, any reason why the module alias code is turning an empty MODULE_PARAM structure, as is declared in drivers/usb/serial/visor.c with the line:
{ }, /* optional parameter entry */

Quoted 6 lines

Quoted 1 line
Into the above MODULE_ALIAS? I don't think that's correct.
Sorry for the delay, was away and am catching up on mail.
Thanks to Andrey for the fix. Greg, did you want to add something else? Either way, please forward to Linus.

Just finally did that, thanks.

greg k-h

-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials. Become an expert in LINUX or just sharpen your skills. Sign up for IBM's Free Linux Tutorials. Learn everything from the bash shell to sys admin. Click now! http://ads.osdn.com/?ad_id78&alloc_id371&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel