linux-kernel - Re: RFC: Add USBDEVFS_TRY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 24 Aug 2011 23:04:03 +0200
From:	Hans de Goede <hdegoede@...hat.com>
To:	Greg KH <greg@...ah.com>
CC:	Alan Stern <stern@...land.harvard.edu>,
	USB list <linux-usb@...r.kernel.org>,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: RFC: Add USBDEVFS_TRY_DISCONNECT ioctl

Hi,

First of all: A big thanks to Alan for working on this. From my POV
the code looks good, but I'm only familiar with the usb parts
and not with the scsi parts.

One generic concern is the use of scsi_lock(host) in the
scsi_device_open / close callbacks. We need to be sure non
of the callers of these can already be holding the lock.

On 08/24/2011 10:45 PM, Greg KH wrote:
> On Wed, Aug 24, 2011 at 04:32:31PM -0400, Alan Stern wrote:
>> Okay, here's a sample patch.  Actually it's three patches, listed one
>> after another, but people can apply it like a single patch.
>>
>>       1.	Introduce the USBDEVFS_TRY_DISCONNECT ioctl and the check_busy
>> 	callback it uses.  Implement the callback in the usbfs driver;
>> 	this gives a way for programs to unbind kernel drivers without
>> 	unbinding other userspace drivers.
>>
>>       2.	Implement device-file reference tracking in the SCSI layer,
>> 	and the device_open and device_close callbacks it uses.
>
> Does this handle if the filesystem is being created or fscked, as it's
> not mounted at that time.
>
>> @@ -1647,9 +1653,16 @@ static int proc_ioctl(struct dev_state *
>>   	else switch (ctl->ioctl_code) {
>>
>>   	/* disconnect kernel driver from interface */
>> +	case USBDEVFS_TRY_DISCONNECT:
>>   	case USBDEVFS_DISCONNECT:
>>   		if (intf->dev.driver) {
>>   			driver = to_usb_driver(intf->dev.driver);
>> +			if (ctl->ioctl_code == USBDEVFS_TRY_DISCONNECT&&
>> +					driver->check_busy) {
>> +				retval = driver->check_busy(intf);
>> +				if (retval)
>> +					break;
>> +			}
>
> I don't like the fact that if a driver doesn't contain check_busy() then
> it will automatically fall back to looking like it was a DISCONNECT
> call, which could give userspace a false sense of "everything was fine"
> when trying this out.

I've to agree with Greg here, what to do in case of a driver not
implementing check_busy and thus not really offering USBDEVFS_TRY_DISCONNECT
is policy and thus should be left to userspace. I suggest we just return
-ENOTTY in case of USBDEVFS_TRY_DISCONNECT and the bound driver does not
have checkbusy, then userspace can decide wether to fallback to a regular
disconnect, or to give up.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/