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.