linux-kernel - Re: [PATCH] usb/core/devio.c: Don't use GFP_KERNEL while we cannot reset a storage device

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:	Tue, 12 Mar 2013 00:34:44 +0800
From:	Ming Lei <tom.leiming@...il.com>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Alexey Khoroshilov <khoroshilov@...ras.ru>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Hans de Goede <hdegoede@...hat.com>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	ldv-project@...uxtesting.org
Subject: Re: [PATCH] usb/core/devio.c: Don't use GFP_KERNEL while we cannot
 reset a storage device

On Tue, Mar 12, 2013 at 12:08 AM, Alan Stern <stern@...land.harvard.edu> wrote:
> On Mon, 11 Mar 2013, Ming Lei wrote:
>
>> On Mon, Mar 11, 2013 at 11:32 PM, Alan Stern <stern@...land.harvard.edu> wrote:
>> >
>> > Of course you have to lock the device before changing its driver.  What
>> > would happen if two different threads tried to change a device's driver
>> > at the same time?
>>
>> Yes, claim/release interface need device lock, but the patch doesn't
>> touch claim/release command handling.
>
> Then why did you ask?  You wrote: "Looks device lock isn't required for
> USB transfer of kernel driver."

Maybe I didn't expressed clearly, sorry. Here the USB transfer means
URBs submitting.

>
>
>> > usbdev_do_ioctl() needs to acquire the device lock in order to prevent
>> > races with driver_disconnect() and usbdev_remove().
>>
>> Looks the patch basically converts the allocation inside URB submit path,
>> and actually I mean why we need to hold device lock in submitting
>> URB path?  Device lock isn't required before submitting URBs
>> in kernel driver.
>
> In general it isn't, no.  But usbfs uses the lock to prevent races with
> driver_disconnect() -- it is invalid to submit URBs after the
> disconnect routine has returned.

If so, we may introduce another lock to avoid the race.

So I think we may figure out to decrease the device lock
scope in devio.c, and most of ioctl commands might not require it
at all, then the problem addressed by the patch can be fixed too.


Thanks,
-- 
Ming Lei
--
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/