On Sun, Mar 31, 2019 at 04:34:57AM +0200, Jann Horn wrote: > On Sun, Mar 31, 2019 at 3:07 AM Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> wrote: > > As I said I don't really care about "pidfd" solving any racing issues with > > /proc/<pid>/* accesses - because I still find it hard to imagine that the pid > > number can be reused easily from the time you know which <pid> to deal with, > > to the time when you want to read, say, the /proc/<pid>/status file. > > There have been several Android security bugs related to PID reuse. Yes PID reuse will be a problem till we have pidfd_clone and pidfd_send_signal (and any other pidfd related syscalls). I've never denied PID reuse is *currently* a problem and the set of pidfd syscalls being proposed are designed to avoid those. So I'm not fully sure what you mean. Anyway, I would love to see those security bugs you mentioned if you could point me to them. > > I am yet > > to see any real data to show that such overflow happens - you literally need > > 32k process deaths and forks in such a short time frame > > This seems very inaccurate to me. > > The time frame in which the PID has to wrap around is not the time > between process death and use of the PID. It is the time between *the > creation* of the old process and the use of the PID. Consider the > following sequence of events: > > - process A starts with PID 1000 > - some time passes in which some process repeatedly forks, with PIDs > wrapping around to 999 > - process B starts an attempt to access process A (using PID 1000) > - process A dies > - process C spawns with PID 1000 > - process B accidentally accesses process C > > Also, it's probably worth clarifying that here, "processes" means "threads". > > If there are a lot of active processes, that reduces the number of > times you have to clone() to get the PID to wrap around. Ok, that's fair and I take your point. But I wonder what access you're talking about, is it killing the process? If yes, pidfd_clone + pidfd_send_signal will solve that in the race free way without relying on the PID number. Is it accessing /proc/<pid>/? then see below. > > and on 64-bit, that > > number is really high > > Which number is really high on 64-bit? Checking on a walleye phone, > pid_max is still only 32768: > > walleye:/ # cat /proc/sys/kernel/pid_max > 32768 > walleye:/ # Ok. I was talking about the theoretical limit of pid_max on a 64-bit platform. But since we are talking about NOT relying on the PID number in the first place, we can move on from this point. > > that its not even an issue. And if this is really an > > issue, then you can just open a handle to /proc/<pid> at process creation > > time and keep it around. If the <pid> is reused, you can still use openat(2) > > on that handle without any races. > > But not if you want to implement something like killall in a > race-free way, for example. I am not at all talking about killing processes in your last quote of my email above, I'm talking about access to /proc/<pid>/ files. As I said, at the time of process creation, you can obtain an fd by opening /proc/<pid>/ and keep it open. Then you can do an openat(2) on that fd without worrying at <pid> reuse, no? And then access all the files that way. As for killall in Android. I don't think that "killing processes by name" is relied on for the runtime operation of Android. That would be a very bad idea. Low memory killer does not kill processes by name. It kills processes by the PID number using kill(2) which we'd like to replace with pidfd_send_signal. Again if you want to convince Linus about having a "pidfd to procfd" conversion mechanism, then by all means go for it. I just don't think it is urgently necessary (and others may disagree with me on this), but I wouldn't care if such a mechanism existed either. Whatever we do, I just want the notion of "pidfd" to be consistent as I mentioned in my previous email. thank you! - Joel
