[Toybox] [PATCH] Fix pgrep -s 0 when running in session ID 0

[Rob Landley]

On 10/7/23 21:47, Colin Cross wrote:
> On Fri, Oct 6, 2023 at 9:29 PM Rob Landley <rob at 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...

Rob