Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Tokio and Prctl = Nasty Bug

kobzol.github.io

Post reply on HN
Print view View original

11–20 of 78 posts

Re: Tokio and Prctl = Nasty Bug

#11

Post by skissane »

TheDong wrote: post #4

I think they don't want PR_SET_PDEATHSIG but rather PR_SET_CHILD_SUBREAPER, which I think would be both more correct than PDEATHSIG for letting them wait on grand-children / preventing grand-child-zombies, while also avoiding the issue they ran into here entirely. They would need one special "main thread" that deals with reaping and that isn't subject to tokio's runtime cleaning it up, but presumably they already hav…

> I think they don't want PR_SET_PDEATHSIG but rather PR_SET_CHILD_SUBREAPER, which I think would be both more correct than PDEATHSIG for letting them wait on grand-children / preventing grand-child-zombies, while also avoiding the issue they ran into here entirely.

PR_SET_PDEATHSIG automatically kills your children if you die, but unfortunately doesn’t extend to their descendants

As far as I’m aware, PR_SET_CHILD_SUBREAPER doesn’t do anything if you die. Assuming you yourself don’t crash, it can be used to help clean up orphaned descendant processes, by ensuring they reparent to you instead of init; but in the event you do crash, it doesn’t do anything to help.

PID namespaces do exactly what you want - if their init process dies it automatically kills all its descendants. However, they require privilege - unless you use an unprivileged user namespace - but those are frequently disabled, and even when enabled, using them potentially introduces a whole host of other issues

> Alternatively, if they want they could integrate with systemd

The problem is a lot of code runs in environments without systemd-e.g. code running in containers (Docker, K8S, etc), most containers don’t contain systemd. So any systemd-centric solution is only going to work for some people

Really, it would be great if Linux added some new process grouping construct which included the “kill all members of this group if its leader dies” semantic of PID namespaces without any of its other semantics. It is those other semantics (especially the new PID number semantics) which are the primary source of the security concerns, so a construct which offered only the “kill-if-leader-dies” semantic should be safe to allow for unprivileged access. (The one complexity is setuid/setgid/file capabilities - allowing an unprivileged process to effectively kill a privileged process at an arbitrary point in its execution is a security risk-plausible solutions include refuse to execute any setuid/setgid/caps executable, or else allow them to run but remove the process from this grouping when it executes one)

Re: Tokio and Prctl = Nasty Bug

#12

Post by eqvinox »

> It is called PR_SET_DEATHSIG, and we configure it when spawning tasks using the prctl syscall like this

PDEATHSIG was to my knowledge (85% confidence) created for the original Linux userspace pthreads implementation (LinuxThreads¹, before NPTL) that was created back when it was implemented via kernel processes (the kernel had no concept of threads yet). This is AFAIK also why it behaves oddly in regards to later-added kernel threads. I have a flag for "don't use this, it's highly fragile" in my head but don't remember where that's from.

If the receiving side can be controlled, there's always the option of opening a pipe; if the other end dies that's always detectable. Doesn't work with arbitrary processes though (random other code won't care if some fd ≥3 is suddenly closed…)

¹ https://en.wikipedia.org/wiki/LinuxThreads

Re: Tokio and Prctl = Nasty Bug

#13

Post by bhawks »

Normally I'd stay away from job control posix APIs - but since HyperQueue is a job control system, it might be appropriate if the worker was a session leader. If it dies than all its subprocesses would receive SIGHUP - which is fatal by default.

Generally you'd use this functionality to implement something like sshd or an interactive shell. HQ seems roughly analogous.

https://notes.shichao.io/apue/ch9/#sessions

Re: Tokio and Prctl = Nasty Bug

#14

Post by eqvinox »

skissane wrote: post #11
TheDong wrote: post #4

I think they don't want PR_SET_PDEATHSIG but rather PR_SET_CHILD_SUBREAPER, which I think would be both more correct than PDEATHSIG for letting them wait on grand-children / preventing grand-child-zombies, while also avoiding the issue they ran into here entirely. They would need one special "main thread" that deals with reaping and that isn't subject to tokio's runtime cleaning it up, but presumably they already hav…

> I think they don't want PR_SET_PDEATHSIG but rather PR_SET_CHILD_SUBREAPER, which I think would be both more correct than PDEATHSIG for letting them wait on grand-children / preventing grand-child-zombies, while also avoiding the issue they ran into here entirely. PR_SET_PDEATHSIG automatically kills your children if you die, but unfortunately doesn’t extend to their descendants As far as I’m aware, PR_SET_CHILD_SU…

> PR_SET_PDEATHSIG automatically kills your children if you die, but unfortunately doesn’t extend to their descendants

It indirectly does, unless you unset it the child dying will trigger another run of PDEATHSIG on the grandchildren, and so on. (The setting is retained across forks, as shown in the original article.)

Re: Tokio and Prctl = Nasty Bug

#15

Post by eqvinox »

vlovich123 wrote: post #7
TheDong wrote: post #4

I think they don't want PR_SET_PDEATHSIG but rather PR_SET_CHILD_SUBREAPER, which I think would be both more correct than PDEATHSIG for letting them wait on grand-children / preventing grand-child-zombies, while also avoiding the issue they ran into here entirely. They would need one special "main thread" that deals with reaping and that isn't subject to tokio's runtime cleaning it up, but presumably they already hav…

> when the orphan terminates, it is the subreaper process that will receive a SIGCHLD signal and will be able to wait(2) on the process to discover its termination status Seems like you don’t need a dedicated “always alive” thread if it’s being delivered to the process and tokio automatically does masking for threads so that you register for listening to signals using it’s asynchronous mechanisms & don’t have issues…

> That being said, it’s not clear PR_SET_CHILD_SUBREAPER actually causes grand children to be killed when the reaper process dies

CHILD_SUBREAPER kills neither children nor grandchildren. It's effect is in the other direction, inteded for sub-service-managers that want to keep track of all children. If the subreaper dies, children are reparented to the next subreaper up (or init).

Re: Tokio and Prctl = Nasty Bug

#16

Post by oguz-ismail »

rendaw wrote: post #10
immibis wrote: post #2

Good writeup of yet another bug different from all the other bugs. The Linux kernel isn't really bothered by the difference between threads and processes. Threads are just processes that happen to share an address space, file descriptor table, and thread group ID (what most tools call a PID). I think there are some subtle things related to the thread group ID, but they're subtle. The rest is implemented in glibc.

Are there any differences between threads and processes in how signals are handled? I recently learned that aside from processes there are process groups, process sessions (setsid), process group and session leaders, trees have associated VT ownership data, systemd sessions (which seem to be inherited by the entire subtree and can't be purged), and possibly other layered metadata spaces that I haven't heard of yet. A…

> I really wish there were an overview of all these things and how they interact with eachother somewhere.

    man 7 signal

Re: Tokio and Prctl = Nasty Bug

#17

Post by eqvinox »

rendaw wrote: post #10
immibis wrote: post #2

Good writeup of yet another bug different from all the other bugs. The Linux kernel isn't really bothered by the difference between threads and processes. Threads are just processes that happen to share an address space, file descriptor table, and thread group ID (what most tools call a PID). I think there are some subtle things related to the thread group ID, but they're subtle. The rest is implemented in glibc.

Are there any differences between threads and processes in how signals are handled? I recently learned that aside from processes there are process groups, process sessions (setsid), process group and session leaders, trees have associated VT ownership data, systemd sessions (which seem to be inherited by the entire subtree and can't be purged), and possibly other layered metadata spaces that I haven't heard of yet. A…

> Are there any differences between threads and processes in how signals are handled?

Yes, absolutely, there are thread-directed and process-directed signals; for the latter a thread is chosen at random (more or less) to handle the signal.

Re: Tokio and Prctl = Nasty Bug

#18

Post by skissane »

rendaw wrote: post #10
immibis wrote: post #2

Good writeup of yet another bug different from all the other bugs. The Linux kernel isn't really bothered by the difference between threads and processes. Threads are just processes that happen to share an address space, file descriptor table, and thread group ID (what most tools call a PID). I think there are some subtle things related to the thread group ID, but they're subtle. The rest is implemented in glibc.

Are there any differences between threads and processes in how signals are handled? I recently learned that aside from processes there are process groups, process sessions (setsid), process group and session leaders, trees have associated VT ownership data, systemd sessions (which seem to be inherited by the entire subtree and can't be purged), and possibly other layered metadata spaces that I haven't heard of yet. A…

> Are there any differences between threads and processes in how signals are handled?

Yes. As signal(7) notes [0], Linux has both “process-directed signals” (which can be handled by any thread in a process), and “thread-directed signals” (which are targeted at a specific thread and only handled by that thread). For user-generated signals, the classification depends on which syscall you use (kill/rt_sigqueueinfo generate process-directed signals, tgkill/rt_tsigqueueinfo generate thread-directed). For system-generated signals, it is up to the kernel code generating the signal to decide. So the same signal number can be thread-directed in some cases and process-directed in others

> systemd sessions (which seem to be inherited by the entire subtree and can't be purged)

At a kernel level those are implemented with cgroups.

> I really wish there were an overview of all these things

Unfortunately I think Linux has grown a complex mess of different features in this area, all of which are full of complicated limitations and gotchas. Despite attempts to introduce orthogonality (e.g. with several different types of namespaces), the end result is still a long way from any ideal of orthogonality

[0] https://man7.org/linux/man-pages/man7/signal.7.html

Re: Tokio and Prctl = Nasty Bug

#19

Post by eqvinox »

immibis wrote: post #2

Good writeup of yet another bug different from all the other bugs. The Linux kernel isn't really bothered by the difference between threads and processes. Threads are just processes that happen to share an address space, file descriptor table, and thread group ID (what most tools call a PID). I think there are some subtle things related to the thread group ID, but they're subtle. The rest is implemented in glibc.

The distinction isn't quite as subtle as you believe, it also shows up in e.g. file locks, AF_UNIX SO_PEERCRED, and with any process-directed signal.

As a matter of fact, the original implementation of POSIX threads for Linux was userspace based and had unfixable bugs and issues that necessitated introducing the concept of threads into the Linux kernel.

Re: Tokio and Prctl = Nasty Bug

#20

Post by skissane »

eqvinox wrote: post #14
skissane wrote: post #11

Earlier quoted context omitted.

> I think they don't want PR_SET_PDEATHSIG but rather PR_SET_CHILD_SUBREAPER, which I think would be both more correct than PDEATHSIG for letting them wait on grand-children / preventing grand-child-zombies, while also avoiding the issue they ran into here entirely. PR_SET_PDEATHSIG automatically kills your children if you die, but unfortunately doesn’t extend to their descendants As far as I’m aware, PR_SET_CHILD_SU…

> PR_SET_PDEATHSIG automatically kills your children if you die, but unfortunately doesn’t extend to their descendants It indirectly does, unless you unset it the child dying will trigger another run of PDEATHSIG on the grandchildren, and so on. (The setting is retained across forks, as shown in the original article.)

> The setting is retained across forks, as shown in the original article

That’s not what the man page says:

> The parent-death signal setting is cleared for the child of a fork(2).

https://man7.org/linux/man-pages/man2/pr_set_pdeathsig.2cons...

Unless the man page is wrong?

Post reply on HN