linux-kernel - Re: drivers/staging/usbip/ abuses task_is_dead/exit

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, 20 Sep 2011 11:38:10 -0700
From:	Greg KH <gregkh@...e.de>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	"Serge E. Hallyn" <serge@...lyn.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Howells <dhowells@...hat.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Tobias Klauser <tklauser@...tanz.ch>,
	Matt Mooney <mfm@...eddisk.com>,
	"Serge E. Hallyn" <serge.hallyn@...onical.com>,
	lkml <linux-kernel@...r.kernel.org>, richard@....at,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Tejun Heo <tj@...nel.org>
Subject: Re: drivers/staging/usbip/ abuses task_is_dead/exit_state

On Tue, Sep 20, 2011 at 05:14:10PM +0200, Oleg Nesterov wrote:
> (add more cc's)
> 
> On 09/20, Oleg Nesterov wrote:
> >
> > Unfortunately, we can't kill task_is_dead() right now, it has already
> > found the users in drivers/staging/, and I bet the usage is wrong.
> 
> It is used by drivers/staging/usbip/
> 
> For what? The code:
> 
> 	if (vdev->ud.tcp_rx && !task_is_dead(vdev->ud.tcp_rx))
> 		kthread_stop(vdev->ud.tcp_rx);
> 
> And how task_is_dead() can help? This helper is really "special", it
> shouldn't be used anyway. But why do we check ->exit_state? Without
> tasklist the check is racy anyway, the task can exit right after the
> check.
> 
> And. It is safe to use kthread_stop(t) even if t has already exited.
> 
> OK, this was added by 8547d4cc2b616e4f1dafebe2c673fc986422b506
> "Staging: usbip: vhci-hcd: Do not kill already dead RX/TX kthread"
> 
> 	When unbinding a device on the host which was still attached on the
> 	client, I got a NULL pointer dereference on the client.
> 
> Where?
> 
> 	This turned out
> 	to be due to kthread_stop() being called on an already dead kthread.
> 
> This should work.
> 
> I'm afraid this can only fix the symptom. Probably, the problem is that
> we do not have the reference and thus even task_is_dead(t) is not safe.
> 
> This kthread was created by kthread_run(). If it exits, nothing protects
> this task_struct.
> 
> In any case, please do not use ->exit_state. It should not be used outside
> of exit.c/etc paths, "exit_state != 0" means "exit_notify() was called".

Patches to fix this up in this driver are always gladly appreciated :)

greg k-h
--
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/