[2/2] linux: Return unsupported if procfs can not be mount on tst-ttyname-namespace

Message ID 20230612183014.882895-3-adhemerval.zanella@linaro.org
State Committed
Commit d35fbd3e684e6bb5e5ec452ad8dac6ada8424bdd
Headers
Series Improve tst-ttyname within containers |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed

Commit Message

Adhemerval Zanella Netto June 12, 2023, 6:30 p.m. UTC
  Trying to mount procfs can fail due multiples reasons: proc is locked
due the container configuration, mount syscall is filtered by a
Linux Secuirty Module, or any other security or hardening mechanism
that Linux might eventually add.

The tests does require a new procfs without binding to parent, and
to fully fix it would require to change how the container was created
(which is out of the scope of the test itself).  Instead of trying to
foresee any possible scenario, if procfs can not be mount fail with
unsupported.

Checked on aarch64-linux-gnu.
---
 .../unix/sysv/linux/tst-ttyname-namespace.c   | 28 +++++++++++--------
 1 file changed, 16 insertions(+), 12 deletions(-)
  

Comments

Siddhesh Poyarekar June 27, 2023, 2:03 p.m. UTC | #1
On 2023-06-12 14:30, Adhemerval Zanella via Libc-alpha wrote:
> Trying to mount procfs can fail due multiples reasons: proc is locked
> due the container configuration, mount syscall is filtered by a
> Linux Secuirty Module, or any other security or hardening mechanism
> that Linux might eventually add.
> 
> The tests does require a new procfs without binding to parent, and
> to fully fix it would require to change how the container was created
> (which is out of the scope of the test itself).  Instead of trying to
> foresee any possible scenario, if procfs can not be mount fail with
> unsupported.
> 
> Checked on aarch64-linux-gnu.
> ---

LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

>   .../unix/sysv/linux/tst-ttyname-namespace.c   | 28 +++++++++++--------
>   1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/tst-ttyname-namespace.c b/sysdeps/unix/sysv/linux/tst-ttyname-namespace.c
> index 1f38442747..cd5e79d39b 100644
> --- a/sysdeps/unix/sysv/linux/tst-ttyname-namespace.c
> +++ b/sysdeps/unix/sysv/linux/tst-ttyname-namespace.c
> @@ -94,8 +94,17 @@ do_in_chroot_2 (int (*cb)(const char *, int))
>             VERIFY (read (exit_pipe[0], &c, 1) == 0);
>             xclose (exit_pipe[0]);
>   
> -          VERIFY (mount ("proc", "/proc", "proc",
> -                         MS_NOSUID|MS_NOEXEC|MS_NODEV, NULL) == 0);
> +	  if (mount ("proc", "/proc", "proc",
> +		     MS_NOSUID|MS_NOEXEC|MS_NODEV, NULL) != 0)
> +	    {
> +	      /* This happens if we're trying to create a nested container,
> +		 like if the build is running under podman, and we lack
> +		 priviledges.  */
> +	      if (errno == EPERM)
> +		_exit (EXIT_UNSUPPORTED);
> +	      else
> +		_exit (EXIT_FAILURE);
> +	    }
>   
>             char *linkname = xasprintf ("/proc/self/fd/%d", slave);
>             char *target = proc_fd_readlink (linkname);
> @@ -104,8 +113,9 @@ do_in_chroot_2 (int (*cb)(const char *, int))
>   
>             _exit (cb (slavename, slave));
>           }
> -      xwrite (pid_pipe[1], &pid, sizeof pid);
> -      _exit (0);
> +      int status;
> +      xwaitpid (pid, &status, 0);
> +      _exit (WEXITSTATUS (status));
>       }
>     xclose (pid_pipe[1]);
>     xclose (exit_pipe[0]);
> @@ -117,17 +127,11 @@ do_in_chroot_2 (int (*cb)(const char *, int))
>     VERIFY (WIFEXITED (status));
>     int ret = WEXITSTATUS (status);
>     if (ret != 0)
> -    return ret;
> +    FAIL_UNSUPPORTED ("unable to mount /proc on inner child process");
>   
> -  /* set 'pid' to the inner child */
> -  VERIFY (read (pid_pipe[0], &pid, sizeof pid) == sizeof pid);
>     xclose (pid_pipe[0]);
>   
> -  /* wait for the inner child */
> -  xwaitpid (pid, &status, 0);
> -  VERIFY (WIFEXITED (status));
> -  xclose (master);
> -  return WEXITSTATUS (status);
> +  return 0;
>   }
>   
>   static int
  

Patch

diff --git a/sysdeps/unix/sysv/linux/tst-ttyname-namespace.c b/sysdeps/unix/sysv/linux/tst-ttyname-namespace.c
index 1f38442747..cd5e79d39b 100644
--- a/sysdeps/unix/sysv/linux/tst-ttyname-namespace.c
+++ b/sysdeps/unix/sysv/linux/tst-ttyname-namespace.c
@@ -94,8 +94,17 @@  do_in_chroot_2 (int (*cb)(const char *, int))
           VERIFY (read (exit_pipe[0], &c, 1) == 0);
           xclose (exit_pipe[0]);
 
-          VERIFY (mount ("proc", "/proc", "proc",
-                         MS_NOSUID|MS_NOEXEC|MS_NODEV, NULL) == 0);
+	  if (mount ("proc", "/proc", "proc",
+		     MS_NOSUID|MS_NOEXEC|MS_NODEV, NULL) != 0)
+	    {
+	      /* This happens if we're trying to create a nested container,
+		 like if the build is running under podman, and we lack
+		 priviledges.  */
+	      if (errno == EPERM)
+		_exit (EXIT_UNSUPPORTED);
+	      else
+		_exit (EXIT_FAILURE);
+	    }
 
           char *linkname = xasprintf ("/proc/self/fd/%d", slave);
           char *target = proc_fd_readlink (linkname);
@@ -104,8 +113,9 @@  do_in_chroot_2 (int (*cb)(const char *, int))
 
           _exit (cb (slavename, slave));
         }
-      xwrite (pid_pipe[1], &pid, sizeof pid);
-      _exit (0);
+      int status;
+      xwaitpid (pid, &status, 0);
+      _exit (WEXITSTATUS (status));
     }
   xclose (pid_pipe[1]);
   xclose (exit_pipe[0]);
@@ -117,17 +127,11 @@  do_in_chroot_2 (int (*cb)(const char *, int))
   VERIFY (WIFEXITED (status));
   int ret = WEXITSTATUS (status);
   if (ret != 0)
-    return ret;
+    FAIL_UNSUPPORTED ("unable to mount /proc on inner child process");
 
-  /* set 'pid' to the inner child */
-  VERIFY (read (pid_pipe[0], &pid, sizeof pid) == sizeof pid);
   xclose (pid_pipe[0]);
 
-  /* wait for the inner child */
-  xwaitpid (pid, &status, 0);
-  VERIFY (WIFEXITED (status));
-  xclose (master);
-  return WEXITSTATUS (status);
+  return 0;
 }
 
 static int