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

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 16/05/23 09:55, Zack Weinberg via Libc-alpha wrote:
> 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.

Indeed the locale sensitivity is far from ideal, I will change to a 
async-signal-safe one.

> 
> (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.)

My guess is the usercases for this interface has not yet clashed with
the interfaces that require direct pid access.