Re: kthread_stop always returning -EINTR?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Attachment: pgp6qtZZATUxW.pgp
Description: PGP signature

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]
  Powered by Linux