kthread_stop always returning -EINTR?

[valdis.kletnieks at vt.edu]

On Tue, 18 Jul 2017 15:13:08 -0300, Martin Galvan said:

> I thought kthread_create/bind don't actually run the thread function?
> At least that's what the comment says.

It's the wake_up_process() that causes the problem...

> >  * If threadfn() may call do_exit() itself, the caller must ensure
> >  * task_struct can't go away.
> >
> > Your function calls do_exit() itself.  It's surprising that your kernel
> > didn't explode when the tasx_struct went away.
>
> Yeah, my bad. I'm guessing I should've just returned zero?

That's not the problem.  The problem is the task_struct went away when
you called do_exit(), and it can do so in between the two lines wake_up_process()
and kthread_stop().  Classic race condition.

> > Looking at the code of
> > kthread_stop, there's phenomenally little error checking (as most heavily-called
> > kernel functions tend to be).  It just assumes that 'thread' points at
> > a valid thread structure and runs with it, with the result that you get back
> > possibly random trash as 'result'.
>
> Would it be worth it to send out a patch to add some error checking?
> I'd be happy to fix this.

It will be NAK'ed *really* fast.  It's assumed that kernel programmers know
what they're doing, so if their kernel breaks because they call kthread_stop()
on a non-existent thread, it's their own fault.  It *tells* you flat out:

 * If threadfn() may call do_exit() itself, the caller must ensure
 * task_struct can't go away.

> > Two important kernel concepts:  Locking and reference counting. Use them wisely.
>
> Will do, though I'm not sure how those would apply here since (AFAIK)
> there are no variables shared between the threads.

It's not variables shared between the threads.  What you're doing here is more
the equivalent of returning the address of a variable in the stack:

int  *dont_do_this()
{
        int a;
	return (&a);
}

Except here, it's not an int, it's a whole task_struct that you're doing a
use-after-release.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 486 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20170718/26dec180/attachment.bin