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
| ||
|
Date: Tue, 13 Mar 2012 12:45:12 +0100 From: Tobias Klauser <tklauser@...tanz.ch> To: Oleg Nesterov <oleg@...hat.com> Cc: Matt Mooney <mfm@...eddisk.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-kernel@...r.kernel.org Subject: Re: ping: drivers/staging/usbip/ abuses task_is_dead/exit_state On 2012-03-08 at 19:57:39 +0100, Oleg Nesterov <oleg@...hat.com> wrote: > On 03/06, Tobias Klauser wrote: > > > > On 2012-03-06 at 18:39:25 +0100, Oleg Nesterov <oleg@...hat.com> wrote: > > > > > > OK, since nobody cares, probably I should make the patch even if I don't > > > understand this code at all and can't test the change. > > > > > > But, Tobias, may be you can explain what this task_is_dead() check was > > > supposed to do? > > > > As mentioned in the commit message, this was needed for me to work > > around a NULL pointer dereference I got during unbinding > > Where? OK, I guess you do not remember the trace ;) Yup, sorry I can't precisely recall where it happened anymore. > > (I only > > experienced this behaviour on the nios2 platform though, couldn't > > reproduce it on e.g. x86_64). > > OK, > > > I wasn't really familiar with the codebase of usbip (and still am not) > > but came up with the fix by more or less blindly copying what the > > opposite side is checking for in stub_shutdown_connection(). This fixed > > the behaviour for me and seemed legitimate as it was done equally there. > > But this looks "obviously wrong", and afaics just hides the problem. > Not to mention this check is racy, it is simply unsafe to dereference > this task_struct if the kthread has already exited. > > At first glance we need something like the patch below (and stub_dev.c > needs the same fix). It assumes that: > > - vhci_shutdown_connection() is the only caller of kthread_stop(), > iow nobody else does kthread_stop(tcp_*x) > > - we can't leak the task_struct, vhci_shutdown_connection() should > be called in any case at some point. > > I'll try to grep more, but perhaps you can ack my understanding? Looks OK to me at a first glance. I'll try to dig up my test system where I was able to produce that NULL pointer dereference and test your patch there and will then let you know my results. Thanks Tobias -- 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/
Powered by blists - more mailing lists