lkml.org 
[lkml]   [2012]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    Subjectping: drivers/staging/usbip/ abuses task_is_dead/exit_state
    On 09/20, Greg KH wrote:
    >
    > 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 :)

    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?

    Oleg.



    \
     
     \ /
      Last update: 2012-03-06 19:29    [W:4.432 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site