Bug 14744 - kill -32 $pid or kill -33 $pid on a process cancels a random thread
Summary: kill -32 $pid or kill -33 $pid on a process cancels a random thread
Status: NEW
Alias: None
Product: glibc
Classification: Unclassified
Component: nptl (show other bugs)
Version: unspecified
: P2 minor
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-20 01:36 UTC by Rich Felker
Modified: 2015-11-04 21:59 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rich Felker 2012-10-20 01:36:32 UTC
Signals 32 and 33 are used internally by NPTL. There's nothing wrong with this, as they are not advertised as being available to applications (SIGRTMIN shows up as 34 in multi-threaded applications, if I recall). However, if an outside process sends these signals, the target process acts on the signal as if it were a cancellation request for whichever thread happened to receive the signal. This could cause severe breakage and data corruption in applications which are not written to use cancellation and lack cleanup handlers.

I am unsure whether the impact is limited to user error (issuing the kill command manually) or could arise in other ways. In any case, for robustness, I think the signal handler should be a no-op unless pthread_cancel was really called in the application.
Comment 1 Ondrej Bilka 2014-01-10 22:07:33 UTC
> I am unsure whether the impact is limited to user error (issuing the kill 
> command manually) or could arise in other ways. In any case, for robustness, I 
> think the signal handler should be a no-op unless pthread_cancel was really 
> called in the application.

That would make problem worse as with that kill would sometimes kill random thread if a cancellation was called in meantime which is harder to detect as offending code would be mostly nop.

If this needs handled then by aborting affected process.
Comment 2 Rich Felker 2014-01-10 22:43:03 UTC
Perhaps I was not clear. I (hopefully obviously) did not mean that there should be some global state "this application has called pthread_cancel" which causes any thread receiving signal 32 or 33 to get cancelled if it's set. The intent was that pthread_cancel should set the cancellation state on its target (in the TCB or thread-local storage), and a thread should never act upon cancellation unless this state is set.
Comment 3 Carlos O'Donell 2014-01-12 18:32:49 UTC
(In reply to Rich Felker from comment #2)
> Perhaps I was not clear. I (hopefully obviously) did not mean that there
> should be some global state "this application has called pthread_cancel"
> which causes any thread receiving signal 32 or 33 to get cancelled if it's
> set. The intent was that pthread_cancel should set the cancellation state on
> its target (in the TCB or thread-local storage), and a thread should never
> act upon cancellation unless this state is set.

The only sanity checking done today is:

  /* Safety check.  It would be possible to call this function for
     other signals and send a signal from another process.  This is not
     correct and might even be a security problem.  Try to catch as
     many incorrect invocations as possible.  */
  if (sig != SIGCANCEL
      || si->si_pid != pid
      || si->si_code != SI_TKILL)
    return;

However the implementation comment seems to indicate desiring support for cancellation from another process?

      /* We are canceled now.  When canceled by another thread this flag
         is already set but if the signal is directly send (internally or
         from another process) is has to be done here.  */
      int newval = oldval | CANCELING_BITMASK | CANCELED_BITMASK;

I can't imagine under what use case you would support cancelling a thread in a process from another process?

Comments?
Comment 4 Carlos O'Donell 2014-01-12 18:34:40 UTC
(In reply to Carlos O'Donell from comment #3)
> However the implementation comment seems to indicate desiring support for
> cancellation from another process?
> 
>       /* We are canceled now.  When canceled by another thread this flag
>          is already set but if the signal is directly send (internally or
>          from another process) is has to be done here.  */
>       int newval = oldval | CANCELING_BITMASK | CANCELED_BITMASK;
> 
> I can't imagine under what use case you would support cancelling a thread in
> a process from another process?

The worrisome bit is the "internally" part which seems to indicate there is some path inside glibc which can't set the target thread's cancellation status to "cancelling" which would make it impossible to determine if you should or should not act on the SIGCANCEL signal (assuming AC-enabled).
Comment 5 Rich Felker 2014-01-12 23:25:13 UTC
Allowing cancellation from other applications is certainly not desirable, any moreso than it would be desirable for one process to call free() on a pointer malloc'd by another process. And anyway, pthread_t is only meaningful within the context of the process it belongs to.

Technically you could use the tgkill syscall directly to intentionally send a cancellation signal to a particular target thread in another process (this would be less insane than sending it to the process and having the kernel randomly deliver it to one thread). Perhaps the original intent of the code was to allow something like this, where users might attempt to cancel a stuck thread as a way to get a process going again. This seems like a really bad idea though (most apps don't use or support cancellation, and cancelling a thread that's not written to be cancelled could cause dangerous misbehavior). If someone really wants to go to that kind of extent to try to "fix" a hung application, gdb is the right tool, and in fact you can use gdb to call pthread_cancel on a particular thread or make the thread itself call pthread_exit.

So, I really don't think there's any sense in trying to allow cancelling threads from outside the process.
Comment 6 Carlos O'Donell 2014-01-16 16:47:29 UTC
(In reply to Rich Felker from comment #5)
> Allowing cancellation from other applications is certainly not desirable,
> any moreso than it would be desirable for one process to call free() on a
> pointer malloc'd by another process. And anyway, pthread_t is only
> meaningful within the context of the process it belongs to.
> 
> Technically you could use the tgkill syscall directly to intentionally send
> a cancellation signal to a particular target thread in another process (this
> would be less insane than sending it to the process and having the kernel
> randomly deliver it to one thread). Perhaps the original intent of the code
> was to allow something like this, where users might attempt to cancel a
> stuck thread as a way to get a process going again. This seems like a really
> bad idea though (most apps don't use or support cancellation, and cancelling
> a thread that's not written to be cancelled could cause dangerous
> misbehavior). If someone really wants to go to that kind of extent to try to
> "fix" a hung application, gdb is the right tool, and in fact you can use gdb
> to call pthread_cancel on a particular thread or make the thread itself call
> pthread_exit.
> 
> So, I really don't think there's any sense in trying to allow cancelling
> threads from outside the process.

I agree.