Re: [PATCH] Allow marking all USB devices as {un,}authorized by default

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 31.05.2011 06:54 schrieb Greg KH:
> On Mon, May 30, 2011 at 12:19:00PM -0400, Alan Stern wrote:
>   
>> On Mon, 30 May 2011, Greg KH wrote:
>>     
>>> On Mon, May 30, 2011 at 09:09:15AM +0200, Carl-Daniel Hailfinger wrote:
>>>       
>>>> USB treats all devices attached to a wireless USB host controller as
>>>> unauthorized by default and all devices attached to a wired USB host
>>>> controller as authorized by default. This default setting can be changed
>>>> manually per host controller by setting authorized_default in sysfs, but
>>>> only after the host controller is already active.
>>>> AFAICS there is a race between userspace setting authorized_default on
>>>> startup and the USB subsystem enumerating devices on the USB bus. If a
>>>> USB device is already plugged into a wired USB host controller on
>>>> startup, it may be marked as authorized (and thus accessed by the
>>>> kernel/userspace) before userspace has a chance to set
>>>> authorized_default on that host controller. This is undesirable in kiosk
>>>> situations where the user may have access to the USB ports of a machine
>>>> during startup.
>>>>
>>>> Add an "authorized_default" parameter to the usbcore module which has
>>>> three settings:
>>>> 0 is not authorized for all devices
>>>> 1 is authorized for all devices
>>>> 2 is authorized for all devices except wireless (default, old behaviour)
>>>>         
>>> Ick, who is going to remember that "2" is the "default" here?
>>>
>>> I understand this could be a problem, but could you think up a cleaner
>>> interface for this?
>>>       
>> The parameter could become a boolean if case 1 is removed.  After all, 
>> 0 and 2 seem to be the most important cases.
>>     
> Yes, but 1 is something that some systems might want.
>   

I tried to mirror the behaviour of the authorized_default variable which
is in sysfs for every host controller. My reasoning was that users would
expect the allowed values for identical variable names to be the same.
The only extension of that value set is a value for "use old defaults".
I picked 2, but you're right that this is not totally obvious. How about -1?


>>> Also, any new kernel/user API, like this one, needs to be documentented
>>> in Documentation/ABI/.
>>>       
>> Actually they should be mentioned in 
>> Documentation/kernel-parameters.txt.
>>     
> Ah, good point, yes, that would work.
>   

What do you think about this one?

Regards,
Carl-Daniel

>From cc77af3fd94c912230074a04ecb07bf9408a157c Mon Sep 17 00:00:00 2001
From: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@xxxxxxx>
Date: Tue, 31 May 2011 08:28:19 +0200
Subject: [PATCH] Add "authorized_default" parameter to the usbcore module

The "authorized_default" module parameter of usbcore controls the default
for the authorized_default variable of each USB host controller.
-1 is authorized for all devices except wireless (default, old behaviour)
0 is not authorized for all devices
1 is authorized for all devices

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@xxxxxxx>
---
 Documentation/kernel-parameters.txt |    5 +++++
 drivers/usb/core/hcd.c              |   17 ++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 5438a2d..ff1e35b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2535,6 +2535,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	unknown_nmi_panic
 			[X86] Cause panic on unknown NMI.
 
+	usbcore.authorized_default=
+			[USB] Default USB device authorization:
+			(default -1 = authorized except for wireless USB,
+			0 = not authorized, 1 = authorized)
+
 	usbcore.autosuspend=
 			[USB] The autosuspend time delay (in seconds) used
 			for newly-detected USB devices (default 2).  This
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index ace9f84..8988f64 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -337,6 +337,17 @@ static const u8 ss_rh_config_descriptor[] = {
 	0x02, 0x00   /* __le16 ss_wBytesPerInterval; 15 bits for max 15 ports */
 };
 
+/* authorized_default behaviour:
+ * -1 is authorized for all devices except wireless (old behaviour)
+ * 0 is not authorized for all devices
+ * 1 is authorized for all devices
+ */
+static int authorized_default = -1;
+module_param(authorized_default, int, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(authorized_default,
+		"Default USB device authorization: 0 is not authorized, 1 is "
+		"authorized, -1 is authorized except for wireless USB (default, "
+		"old behaviour");
 /*-------------------------------------------------------------------------*/
 
 /**
@@ -2371,7 +2382,11 @@ int usb_add_hcd(struct usb_hcd *hcd,
 
 	dev_info(hcd->self.controller, "%s\n", hcd->product_desc);
 
-	hcd->authorized_default = hcd->wireless? 0 : 1;
+	/* Keep old behaviour if authorized_default is not in [0, 1]. */
+	if (authorized_default < 0 || authorized_default > 1)
+		hcd->authorized_default = hcd->wireless? 0 : 1;
+	else
+		hcd->authorized_default = authorized_default;
 	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
 
 	/* HC is in reset state, but accessible.  Now do the one-time init,
-- 
1.7.1



-- 
http://www.hailfinger.org/

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux