netdev - Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

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:	Fri, 12 Apr 2013 12:02:55 +0200
From:	Oliver Neukum <oliver@...kum.org>
To:	Ming Lei <tom.leiming@...il.com>
Cc:	Dan Williams <dcbw@...hat.com>,
	Elina Pasheva <epasheva@...rrawireless.com>,
	Network Development <netdev@...r.kernel.org>,
	linux-usb <linux-usb@...r.kernel.org>,
	Rory Filer <rfiler@...rrawireless.com>,
	Phil Sutter <phil@....cc>
Subject: Re: [PATCH 1/2 v5] usbnet: allow status interrupt URB to always be active

On Friday 12 April 2013 17:42:46 Ming Lei wrote:
> On Fri, Apr 12, 2013 at 2:37 PM, Oliver Neukum <oliver@...kum.org> wrote:
> >> The work will complete when memory is reclaimed, and the rx/tx path is
> >> still working, so memory reclaim can continue and the deadlock may not
> >> be caused, may it?
> >
> > Only if the memory allocation goes to the same interface. If the blocking interface
> > is storage, something bad happens (data loss not deadlock)
> 
> OK, got it, it should be both since reset can't move on, so
> memory reclaim can't complete to satisfy the allocation.
> 
> But I am wondering if it is a case which is worth the consideration.

The remedy is close to trivial, so yes.

> Almost all USB probe() allocate memory with GFP_KERNEL, then
> the similar scenario(data loss only this time) might happen too. Do we
> need to fix all USB drivers?

No. We assume that probing happens before the interfaces are used.
Furthermore, if this turns out to be real, we can reuse your fix for the
general (w.o. work queue) reset case.

> Wrt. the usbnet_status_start() API, looks no good reason to call
> it in another queue context, it should be enough to call it in probe() or
> bind() if init_status() can be put before info->bind() in usbnet_probe().

We are talking about a generic helper. And the fix is really simple.
We just pass mem_flags and all is well. If the API can be made better
at next to no cost, we do it.

> Then looks the mem_flags isn't needed in both cases, and we should
> always take GFP_NOIO inside the API?

We cannot. GFP_ATOMIC may be needed. And no, we atre not going to
unconditionally use GFP_ATOMIC :-;

	Regards
		Oliver

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