[PATCHv4] tst-pidfd.c: UNSUPPORTED if we get EPERM on valid pidfd_getfd call

  pidfd_getfd can fail for a valid pidfd with errno EPERM for various
reasons in a restricted environment. Use FAIL_UNSUPPORTED in that case.
---

v4: Drop all EPERM checks except on the actual (valid) pidfd_getfd
v3: Also test for EPERM on pidfd_open, don't mention
    PTRACE_MODE_ATTACH_REALCREDS since it is just one reason for
    getting EPERM.
v2: separate ENOSYS and EPERM checks and FAIL_UNSUPPORTED messages

https://code.wildebeest.org/git/user/mjw/glibc/commit/?h=container-perms&id=34f1f3a8a28c292439a1e27e22e39df7c3f7d342

 sysdeps/unix/sysv/linux/tst-pidfd.c | 3 +++
 1 file changed, 3 insertions(+)
  

> On 11 Jul 2022, at 13:25, Mark Wielaard <mark@klomp.org> wrote:
> 
> pidfd_getfd can fail for a valid pidfd with errno EPERM for various
> reasons in a restricted environment. Use FAIL_UNSUPPORTED in that case.
> ---
> 
> v4: Drop all EPERM checks except on the actual (valid) pidfd_getfd
> v3: Also test for EPERM on pidfd_open, don't mention
>    PTRACE_MODE_ATTACH_REALCREDS since it is just one reason for
>    getting EPERM.
> v2: separate ENOSYS and EPERM checks and FAIL_UNSUPPORTED messages
> 
> https://code.wildebeest.org/git/user/mjw/glibc/commit/?h=container-perms&id=34f1f3a8a28c292439a1e27e22e39df7c3f7d342

LGMT, thanks.  You will need to check with Carlos to push upstream.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> sysdeps/unix/sysv/linux/tst-pidfd.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/tst-pidfd.c b/sysdeps/unix/sysv/linux/tst-pidfd.c
> index d93b6faa6f..c598c88661 100644
> --- a/sysdeps/unix/sysv/linux/tst-pidfd.c
> +++ b/sysdeps/unix/sysv/linux/tst-pidfd.c
> @@ -142,6 +142,9 @@ do_test (void)
>     xrecvfrom (sockets[0], &remote_fd, sizeof (remote_fd), 0, NULL, 0);
> 
>     int fd = pidfd_getfd (pidfd, remote_fd, 0);
> +    if (fd == -1 && errno == EPERM)
> +      FAIL_UNSUPPORTED ("don't have permission to use pidfd_getfd on pidfd, "
> +			"skipping test");
>     TEST_VERIFY (fd > 0);
> 
>     char *path = xasprintf ("/proc/%d/fd/%d", pid, remote_fd);
> -- 
> 2.18.4
>
  

Hi Adhemerval,

On Mon, Jul 11, 2022 at 02:01:32PM -0300, Adhemerval Zanella via Libc-alpha wrote:
> > On 11 Jul 2022, at 13:25, Mark Wielaard <mark@klomp.org> wrote:
> > 
> > pidfd_getfd can fail for a valid pidfd with errno EPERM for various
> > reasons in a restricted environment. Use FAIL_UNSUPPORTED in that case.
> > ---
> > 
> > v4: Drop all EPERM checks except on the actual (valid) pidfd_getfd
> > v3: Also test for EPERM on pidfd_open, don't mention
> >    PTRACE_MODE_ATTACH_REALCREDS since it is just one reason for
> >    getting EPERM.
> > v2: separate ENOSYS and EPERM checks and FAIL_UNSUPPORTED messages
> > 
> > https://code.wildebeest.org/git/user/mjw/glibc/commit/?h=container-perms&id=34f1f3a8a28c292439a1e27e22e39df7c3f7d342
> 
> LGMT, thanks.  You will need to check with Carlos to push upstream.
> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

Thanks. Carlos, is it OK to push this now with the Reviewed-by tag
added? The patch only touches the tst, so doesn't change any
implementation or abi, so should be fine even during the
slushy-freeze.

I'll ping my other tst-only patches to see if we have consensus now to
push those too.

Thanks,

Mark
  

On 7/11/22 12:25, Mark Wielaard wrote:
> pidfd_getfd can fail for a valid pidfd with errno EPERM for various
> reasons in a restricted environment. Use FAIL_UNSUPPORTED in that case.
> ---

Needs a v5 with comment. TO me and I'll review again. Thanks!

> v4: Drop all EPERM checks except on the actual (valid) pidfd_getfd
> v3: Also test for EPERM on pidfd_open, don't mention
>     PTRACE_MODE_ATTACH_REALCREDS since it is just one reason for
>     getting EPERM.
> v2: separate ENOSYS and EPERM checks and FAIL_UNSUPPORTED messages
> 
> https://code.wildebeest.org/git/user/mjw/glibc/commit/?h=container-perms&id=34f1f3a8a28c292439a1e27e22e39df7c3f7d342
> 
>  sysdeps/unix/sysv/linux/tst-pidfd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/tst-pidfd.c b/sysdeps/unix/sysv/linux/tst-pidfd.c
> index d93b6faa6f..c598c88661 100644
> --- a/sysdeps/unix/sysv/linux/tst-pidfd.c
> +++ b/sysdeps/unix/sysv/linux/tst-pidfd.c
> @@ -142,6 +142,9 @@ do_test (void)
>      xrecvfrom (sockets[0], &remote_fd, sizeof (remote_fd), 0, NULL, 0);
>  
>      int fd = pidfd_getfd (pidfd, remote_fd, 0);

May you please add a comment here about why this is needed? It's obscure enough that a short
note would help future test writers. Specifically mention which credentials cause the failure.

> +    if (fd == -1 && errno == EPERM)
> +      FAIL_UNSUPPORTED ("don't have permission to use pidfd_getfd on pidfd, "
> +			"skipping test");
>      TEST_VERIFY (fd > 0);
>  
>      char *path = xasprintf ("/proc/%d/fd/%d", pid, remote_fd);