On 10/8/23 11:12, Colin Cross wrote: > On Sat, Oct 7, 2023 at 11:47 PM Rob Landley <r...@landley.net> wrote: >> >> On 10/7/23 21:47, Colin Cross wrote: >> > On Fri, Oct 6, 2023 at 9:29 PM Rob Landley <r...@landley.net> wrote: >> >> >> >> On 10/6/23 17:10, Colin Cross via Toybox wrote: >> >> > I came across an issue when running the pgrep -s 0 test in Android's >> >> > CI infrastructure that uses a PID namespace, causing the test to run >> >> > session ID 0: >> >> > >> >> > $ sudo unshare -fp ./toybox pgrep -s 0 >> >> > pgrep: bad -s '0' >> >> > >> >> > The attached patch fixes the argument parsing to support getsid >> >> > returning 0. >> >> >> >> Ah, I read "man 2 getsid" to indicate that sid 0 was special, but it's >> >> passing >> >> in _pid_ 0 that's special. Which is why, right before your patch, this >> >> part >> >> renders what you did moot: >> >> >> >> // For pkill, -s 0 represents pkill's session id. >> >> if (pl==&TT.ss && ll[pl->len]==0) ll[pl->len] = getsid(0); >> >> >> >> Ah, no wait, that _is_ in the pkill man page: >> >> >> >> -s, --session sid,... >> >> Only match processes whose process session ID is listed. >> >> Ses‐ >> >> sion ID 0 is translated into pgrep's or pkill's own session >> >> ID. >> >> >> >> ...so you want pgrep to accept -s 0, and it'll it will coincidentally >> >> work for >> >> pkill if your session ID _is_ 0 then getsid() will return 0 anyway? And >> >> pkill >> >> should work differently from pgrep? >> > >> > No, pkill and pgrep both already work with -s 0 as long as the current >> > process is not in session ID 0. >> >> By coincidence. >> >> > When passed -s 0, they replace the >> > parsed value (0) with the result of getsid(0). It then falls through >> > to the check that the parsed value (now the getsid value) is greater >> > than 0, but that's incorrect because 0 is a valid return value from >> > getsid. >> >> I think it's actually a kernel bug that the session ID can be 0. The setsid() >> call takes no arguments because it sets the new session ID to equal to the >> PID >> of the calling process, and no running process can have a PID of 0 (that's >> the >> idle task). >> >> If you call getsid(1) it returns 1, because the init task is in its own >> session. >> But the kernel is missing some container initialization, and is thus leaving >> the >> session ID uninitialized (zeroed) which means the session belongs to the idle >> task. (Either that or it's the PID namespace stuff returning 0 somewhere it >> should return 1.) >> >> We can fix userspace to work with this kernel bug, but it IS a kernel bug. >> >> > There's no way to limit pgrep or pkill to the processes in session 0 >> > unless it is itself running in session 0, but that's a general problem >> > with pgrep and pkill, and not specific to toybox. >> >> It's not a problem with either one of those, it's a problem with the kernel's >> container support. Unless the linux-kernel clique is going to "that's a >> volkswagen feature" it... > > It turns out this isn't a kernel bug, it's just an unfortunate > ramification of PID namespaces.
I just checked what running an actual init task gave for getsid() and that also produces 0. Did commit 94913b5d0c56 address it? (Awkward to test here...) Rob _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
