From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb30.google.com (mail-yb1-xb30.google.com [IPv6:2607:f8b0:4864:20::b30]) by sourceware.org (Postfix) with ESMTPS id DF2703858439 for ; Tue, 16 May 2023 12:38:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DF2703858439 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-yb1-xb30.google.com with SMTP id 3f1490d57ef6-b9daef8681fso12115139276.1 for ; Tue, 16 May 2023 05:38:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684240713; x=1686832713; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=kkkb62Dx8Yjj+1oEFX30g7AfQBBoaHgGzPZtEmeuOKc=; b=Rs0PsUDlfcxl89m46slnZP9X3ICqDj7i3vjzjS4wv847L2R4okN2JOaUph/kSzM3o8 j2xsyITI7J7d3YkdaYyXlRNknPiY/1Mv0uGvPyK40a0M+YIM1ewjKQuA01ijblrwEGSx xTFB0wXES0QJmFMrDyVtcrvmcZwt5Axdx7ISlxFAhfzPv5L6X34SAwiXLRRmjEIcMn79 qDWONO38WVXh33AOwXJtVQ25zgHNg7LRrR7P0Oca4r/yWInw71c9JpGm6bfsKchrZ3CH cfctnFo6HLt3EFqMC/KXZn2T9L3tXX6xFr6tZj2cFsRDTPngd6Gt83gSlbSJmUwCka3D F/sA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684240713; x=1686832713; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=kkkb62Dx8Yjj+1oEFX30g7AfQBBoaHgGzPZtEmeuOKc=; b=Ivs3xD+e+PYtTy70h8vVqol8ikucL5gmsy8Dk4oamXmV1yWx3loUeSWigBd3lizXpT eGfXzNRpxzJlckgrPOtu1kCoCtF7L1NlHKu3ws1+ZynNZJLTTTYAnmDjHTCjHUTKRJ7v 4IRWg35ByHpmzr24etuoQoZW6dFAuhfnzizl/4Uzcb+PVqAvSJ6sd9GPI3n/9RSydYeu xW4WGo/PKWa3wUdo1JtRVZ267NzbslmmGrLGpQ0rOiz9Q3hCJMuusIj+AzsYmUy0qU9C JRJGGD0noYkfkQY1AE/LYhwH6KSBPBYoj4brCc54LP4T68xEf39pVsnYL3r5tZ2pkxNy SeBw== X-Gm-Message-State: AC+VfDwU2RSHHNVb7N7se2tVtbg44IZmMOXAbyQSQZQWu6QVDoOynC8s je/+ZdOS7zHwMYmW5BW1lRwsfkK4sibe4+EeoP8= X-Google-Smtp-Source: ACHHUZ47q2rQl8adDoUDpR/C5ovCMd0ywjiFvY5iqnmXgp490grDQV1szTfWIyQAlsvXDp4a4trrzoGk4wkQfiALjSU= X-Received: by 2002:a25:16c5:0:b0:ba2:16b6:b128 with SMTP id 188-20020a2516c5000000b00ba216b6b128mr20783622ybw.4.1684240713174; Tue, 16 May 2023 05:38:33 -0700 (PDT) MIME-Version: 1.0 References: <20230516114612.159103-1-adhemerval.zanella@linaro.org> <20230516114612.159103-4-adhemerval.zanella@linaro.org> In-Reply-To: From: Luca Boccassi Date: Tue, 16 May 2023 13:38:22 +0100 Message-ID: Subject: Re: [PATCH v3 3/3] linux: Add pidfd_getpid To: Adhemerval Zanella Netto Cc: libc-alpha@sourceware.org, Florian Weimer , Philip Withnall Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Tue, 16 May 2023 at 13:26, Adhemerval Zanella Netto wrote: > On 16/05/23 08:54, Luca Boccassi wrote: > > On Tue, 16 May 2023 at 12:46, Adhemerval Zanella > > wrote: > >> > >> This interface allows to obtain the associated pid ID from the > >> process file descriptor. It is done by parsing the procps fdinfo > >> information. Its prototype is: > >> > >> pid_t pidfd_getpid (int fd) > >> > >> It returns the associated pid or -1 in case of an error and set the > >> errno accordingly. The possible errno values are the smae from > >> open, read, and close (used on procps parsing), along with: > >> > >> - EINVAL if the FP is negative (similar to fexecve). > >> > >> - EBADF if the FD does not have a PID associated of if the fdinfo > >> fields contains a value larger than pid_t. > >> > >> - EREMOTE if the PID is in a separate namespace. > >> > >> - ESRCH if the process is already terminated. > >> > >> Checked on x86_64-linux-gnu on Linux 4.15 (no CLONE_PID or waitid > >> support), Linux 5.15 (only clone support), and Linux 5.19 (full > >> support including clone3). > >> --- > > <..> > >> +#define FDINFO_TO_FILENAME_PREFIX "/proc/self/fdinfo/" > >> + > >> +#define FDINFO_FILENAME_LEN \ > >> + (sizeof (FDINFO_TO_FILENAME_PREFIX) + INT_STRLEN_BOUND (int)) > >> + > >> +struct parse_fdinfo_t > >> +{ > >> + bool found; > >> + pid_t pid; > >> +}; > >> + > >> +static int > >> +parse_fdinfo (const char *l, void *arg) > >> +{ > >> + enum { fieldlen = sizeof ("Pid:") - 1 }; > >> + if (strncmp (l, "Pid:", fieldlen) != 0) > >> + return 0; > >> + > >> + l += fieldlen; > >> + > >> + char *endp; > >> + 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? Kind regards, Luca Boccassi