Re: [PATCH v3 3/3] linux: Add pidfd_getpid

["Zack Weinberg" <zack@owlfolio.org>]

On Tue, May 16, 2023, at 8:38 AM, Luca Boccassi via Libc-alpha wrote:
> On Tue, 16 May 2023 at 13:26, Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>> >> +  unsigned long n = strtoul (l, &endp, 10);
>> >> +  if (l == endp || (n > INT_MAX && n != ULONG_MAX))
>> >> +    return 0;
>> >
>> > How can this tell the difference between '-1' and garbage input? It
>> > seems to me this will confuse mangled input here with ESRCH, given the
>> > pid in fdinfo is initialized to -1, no?
>>
>> Because -1 will be parsed as ULONG_MAX.  For instance, with the inputs:
>>
>> Input:             | Function result |         parse_fdinfo_t
>> -------------------|-----------------|-----------------------
>> "Pid: 0"           |               1 |        {1,          0}
>> "Pid: 1"           |               1 |        {1,          1}
>> "Pid: 2147483647"  |               1 |        {1, 2147483647}
>> "Pid: 2147483648"  |               0 |        {0,         -1}
>> "Pid: -1"          |               1 |        {1,         -1}
>> "Pid: -3"          |               0 |        {0,         -1}
>> "Pid: -24x"        |               0 |        {0,         -1}
>>
>> So only if the PID if positive less than INT_MAX or -1 the function
>> will set that the PID as found.
>
> This seems excessively convoluted and fragile to me, relying on
> overflows and so on. I cannot stress this enough, in order to use this
> from systemd et al I need to be absolutely certain that garbage input
> is not treated the same as ESCHR. Why not just use strtoll or so, to
> get an exact result out of it?

Also, all of the strto* functions are sensitive to the locale.  It shouldn't be too hard to make this function async-signal-safe and I think we really ought to.

(Actually, why isn't this operation exposed as an ioctl on the pidfd?  I thought the kernel people had come around to accepting that parsing text files in /proc is fragile, inconvenient, and slow.)

zw