Opened 5 years ago

Closed 5 years ago

#15471 closed enhancement (implemented)

Tor should prctl(PR_SET_PDEATHSIG, SIGTERM) background processes.

Reported by: yawning Owned by:
Priority: Very Low Milestone: Tor: 0.2.7.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Keywords: pt-wants, easy
Cc: dcf, asn Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

From the man page:

       PR_SET_PDEATHSIG (since Linux 2.1.57)
              Set  the  parent  process death signal of the calling process to
              arg2 (either a signal value in the  range  1..maxsig,  or  0  to
              clear).   This  is  the signal that the calling process will get
              when its parent dies.  This value is cleared for the child of  a
              fork(2)  and (since Linux 2.4.36 / 2.6.23) when executing a set-
              user-ID or set-group-ID binary, or a binary that has  associated
              capabilities  (see  capabilities(7)).   This  value is preserved
              across execve(2).

This will ensure at least on Linux that all background processes will get a SIGTERM if Tor dies, and can cleanup appropriately.

I don't think this behavior would be particularly shocking to PT developers (who should be handling SIGTERM already), so this probably doesn't even need a spec patch since "tor dying" invoking the normal termination signaling is appropriate.

The choice of SIGTERM over SIGKILL here is so that PTs have the option to trap it and terminate their own children as appropriate.

Pros:

  • Easy to do.
  • The kernel does all of the heavy lifting for us, as a catchall.
  • Fixes certain nasty issues in unmodified pt code on the relevant platform automatically.

Cons:

  • Non-portable (#15435 aka #10047 is a portable fix that requires pt modification).
  • We have pts (obfs4proxy in particular) that can be ran with elevated capabilities.

Child Tickets

Change History (2)

comment:1 Changed 5 years ago by yawning

Status: newneeds_review

Something like this: https://github.com/Yawning/tor/compare/feature15471

It changes the no environment provided semantics to provide an empty env var list (Switched from execvp to execve), but nothing currently calls the routine in that manner as far as I can tell (the only consumer of background processes launched by tor is the managed pt code).

comment:2 Changed 5 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

lgtm ; merged.

Note: See TracTickets for help on using tickets.