[PATCH] test-container: return UNSUPPORTED for ENOSPC on clone()

[PATCH] test-container: return UNSUPPORTED for ENOSPC on clone()

[Xi Ruoyao]

Since Linux 4.9, the kernel provides
/proc/sys/user/max_{mnt,pid,user}_namespace as a limitation of number of
namespaces.  Some distros (for example, Slint Linux 14.2.1) set them (or
only max_user_namespace) to zero as a "security policy" for disabling
namespaces.

The clone() call will set errno to ENOSPC under such a limitation.  We
didn't check ENOSPC in the code so the test will FAIL, and report:

    unable to unshare user/fs: No space left on device

This message is, unfortunately, very unhelpful.  It leads people to
check the memory or disk space, instead of finding the real issue.

To improve the situation, we should check for ENOSPC and return
UNSUPPORTED as the test result.  Also refactor check_for_unshare_hints()
to emit a proper message telling people how to make the test work, if
they really need to run the namespaced tests.

Reported-by: Philippe Delavalade <philippe.delavalade@orange.fr>
URL: https://lists.linuxfromscratch.org/sympa/arc/lfs-support/2022-06/msg00022.html
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 support/test-container.c | 67 +++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/support/test-container.c b/support/test-container.c
index 7557aac441..b186b75146 100644
--- a/support/test-container.c
+++ b/support/test-container.c
@@ -18,6 +18,7 @@
 
 #define _FILE_OFFSET_BITS 64
 
+#include <array_length.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -684,39 +685,43 @@ rsync (char *src, char *dest, int and_delete, int force_copies)
 /* See if we can detect what the user needs to do to get unshare
    support working for us.  */
 void
-check_for_unshare_hints (void)
+check_for_unshare_hints (int require_pidns)
 {
+  static struct {
+    const char *path;
+    int bad_value, good_value, for_pidns;
+  } files[] = {
+    /* Default Debian Linux disables user namespaces, but allows a way
+       to enable them.  */
+    /* ALT Linux has an alternate way of doing the same.  */
+    { "/proc/sys/kernel/unprivileged_userns_clone", 0, 1, 0 },
+    { "/proc/sys/kernel/userns_restrict", 1, 0, 0 },
+    /* Linux kernel >= 4.9 has a configurable limit on the number of
+       each namespace.  Some distros set the limit to zero to disable the
+       corresponding namespace as a "security policy".  */
+    { "/proc/sys/user/max_user_namespaces", 0, 1024, 0 },
+    { "/proc/sys/user/max_mnt_namespaces", 0, 1024, 0 },
+    { "/proc/sys/user/max_pid_namespaces", 0, 1024, 1 },
+  };
   FILE *f;
-  int i;
+  int i, val;
 
-  /* Default Debian Linux disables user namespaces, but allows a way
-     to enable them.  */
-  f = fopen ("/proc/sys/kernel/unprivileged_userns_clone", "r");
-  if (f != NULL)
+  for (i = 0; i < array_length (files); i++)
     {
-      i = 99; /* Sentinel.  */
-      fscanf (f, "%d", &i);
-      if (i == 0)
-	{
-	  printf ("To enable test-container, please run this as root:\n");
-	  printf ("  echo 1 > /proc/sys/kernel/unprivileged_userns_clone\n");
-	}
-      fclose (f);
-      return;
-    }
+      if (!require_pidns && files[i].for_pidns)
+        continue;
 
-  /* ALT Linux has an alternate way of doing the same.  */
-  f = fopen ("/proc/sys/kernel/userns_restrict", "r");
-  if (f != NULL)
-    {
-      i = 99; /* Sentinel.  */
-      fscanf (f, "%d", &i);
-      if (i == 1)
-	{
-	  printf ("To enable test-container, please run this as root:\n");
-	  printf ("  echo 0 > /proc/sys/kernel/userns_restrict\n");
-	}
-      fclose (f);
+      f = fopen (files[i].path, "r");
+      if (f == NULL)
+        continue;
+
+      val = -1; /* Sentinel.  */
+      fscanf (f, "%d", &val);
+      if (val != files[i].bad_value)
+	continue;
+
+      printf ("To enable test-container, please run this as root:\n");
+      printf ("  echo %d > %s\n", files[i].good_value, files[i].path);
       return;
     }
 }
@@ -1117,11 +1122,11 @@ main (int argc, char **argv)
     {
       /* Older kernels may not support all the options, or security
 	 policy may block this call.  */
-      if (errno == EINVAL || errno == EPERM)
+      if (errno == EINVAL || errno == EPERM || errno == ENOSPC)
 	{
 	  int saved_errno = errno;
-	  if (errno == EPERM)
-	    check_for_unshare_hints ();
+	  if (errno == EPERM || errno == ENOSPC)
+	    check_for_unshare_hints (require_pidns);
 	  FAIL_UNSUPPORTED ("unable to unshare user/fs: %s", strerror (saved_errno));
 	}
       /* We're about to exit anyway, it's "safe" to call unshare again
-- 
2.36.1

Re: [PATCH] test-container: return UNSUPPORTED for ENOSPC on clone()

[DJ Delorie]

> +    /* ALT Linux has an alternate way of doing the same.  */
> +    { "/proc/sys/kernel/unprivileged_userns_clone", 0, 1, 0 },

Those two lines should be swapped, as the comment refers to this
following line:

> +    { "/proc/sys/kernel/userns_restrict", 1, 0, 0 },

LGTM to commit with that change.

Reviewed-by: DJ Delorie <dj@redhat.com>

[PATCH v2] test-container: return UNSUPPORTED for ENOSPC on clone()

[Xi Ruoyao]

Hi DJ,

Revised patch following.  I don't have write access to glibc.git so I
guess you need to push the patch.

-- >8 --

Since Linux 4.9, the kernel provides
/proc/sys/user/max_{mnt,pid,user}_namespace as a limitation of number of
namespaces.  Some distros (for example, Slint Linux 14.2.1) set them (or
only max_user_namespace) to zero as a "security policy" for disabling
namespaces.

The clone() call will set errno to ENOSPC under such a limitation.  We
didn't check ENOSPC in the code so the test will FAIL, and report:

    unable to unshare user/fs: No space left on device

This message is, unfortunately, very unhelpful.  It leads people to
check the memory or disk space, instead of finding the real issue.

To improve the situation, we should check for ENOSPC and return
UNSUPPORTED as the test result.  Also refactor check_for_unshare_hints()
to emit a proper message telling people how to make the test work, if
they really need to run the namespaced tests.

Reported-by: Philippe Delavalade <philippe.delavalade@orange.fr>
URL: https://lists.linuxfromscratch.org/sympa/arc/lfs-support/2022-06/msg00022.html
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
Reviewed-by: DJ Delorie <dj@redhat.com>
---
 support/test-container.c | 67 +++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/support/test-container.c b/support/test-container.c
index 7557aac441..b6a1158ae1 100644
--- a/support/test-container.c
+++ b/support/test-container.c
@@ -18,6 +18,7 @@
 
 #define _FILE_OFFSET_BITS 64
 
+#include <array_length.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -684,39 +685,43 @@ rsync (char *src, char *dest, int and_delete, int force_copies)
 /* See if we can detect what the user needs to do to get unshare
    support working for us.  */
 void
-check_for_unshare_hints (void)
+check_for_unshare_hints (int require_pidns)
 {
+  static struct {
+    const char *path;
+    int bad_value, good_value, for_pidns;
+  } files[] = {
+    /* Default Debian Linux disables user namespaces, but allows a way
+       to enable them.  */
+    { "/proc/sys/kernel/unprivileged_userns_clone", 0, 1, 0 },
+    /* ALT Linux has an alternate way of doing the same.  */
+    { "/proc/sys/kernel/userns_restrict", 1, 0, 0 },
+    /* Linux kernel >= 4.9 has a configurable limit on the number of
+       each namespace.  Some distros set the limit to zero to disable the
+       corresponding namespace as a "security policy".  */
+    { "/proc/sys/user/max_user_namespaces", 0, 1024, 0 },
+    { "/proc/sys/user/max_mnt_namespaces", 0, 1024, 0 },
+    { "/proc/sys/user/max_pid_namespaces", 0, 1024, 1 },
+  };
   FILE *f;
-  int i;
+  int i, val;
 
-  /* Default Debian Linux disables user namespaces, but allows a way
-     to enable them.  */
-  f = fopen ("/proc/sys/kernel/unprivileged_userns_clone", "r");
-  if (f != NULL)
+  for (i = 0; i < array_length (files); i++)
     {
-      i = 99; /* Sentinel.  */
-      fscanf (f, "%d", &i);
-      if (i == 0)
-	{
-	  printf ("To enable test-container, please run this as root:\n");
-	  printf ("  echo 1 > /proc/sys/kernel/unprivileged_userns_clone\n");
-	}
-      fclose (f);
-      return;
-    }
+      if (!require_pidns && files[i].for_pidns)
+        continue;
 
-  /* ALT Linux has an alternate way of doing the same.  */
-  f = fopen ("/proc/sys/kernel/userns_restrict", "r");
-  if (f != NULL)
-    {
-      i = 99; /* Sentinel.  */
-      fscanf (f, "%d", &i);
-      if (i == 1)
-	{
-	  printf ("To enable test-container, please run this as root:\n");
-	  printf ("  echo 0 > /proc/sys/kernel/userns_restrict\n");
-	}
-      fclose (f);
+      f = fopen (files[i].path, "r");
+      if (f == NULL)
+        continue;
+
+      val = -1; /* Sentinel.  */
+      fscanf (f, "%d", &val);
+      if (val != files[i].bad_value)
+	continue;
+
+      printf ("To enable test-container, please run this as root:\n");
+      printf ("  echo %d > %s\n", files[i].good_value, files[i].path);
       return;
     }
 }
@@ -1117,11 +1122,11 @@ main (int argc, char **argv)
     {
       /* Older kernels may not support all the options, or security
 	 policy may block this call.  */
-      if (errno == EINVAL || errno == EPERM)
+      if (errno == EINVAL || errno == EPERM || errno == ENOSPC)
 	{
 	  int saved_errno = errno;
-	  if (errno == EPERM)
-	    check_for_unshare_hints ();
+	  if (errno == EPERM || errno == ENOSPC)
+	    check_for_unshare_hints (require_pidns);
 	  FAIL_UNSUPPORTED ("unable to unshare user/fs: %s", strerror (saved_errno));
 	}
       /* We're about to exit anyway, it's "safe" to call unshare again
-- 
2.37.0

PING: [PATCH v2] test-container: return UNSUPPORTED for ENOSPC on clone()

[Xi Ruoyao]

On Tue, 2022-06-28 at 18:44 +0800, Xi Ruoyao wrote:
> Hi DJ,
> 
> Revised patch following.  I don't have write access to glibc.git so I
> guess you need to push the patch.

Gentle ping as we are collecting patches for 2.36 :).

> -- >8 --
> 
> Since Linux 4.9, the kernel provides
> /proc/sys/user/max_{mnt,pid,user}_namespace as a limitation of number
> of
> namespaces.  Some distros (for example, Slint Linux 14.2.1) set them
> (or
> only max_user_namespace) to zero as a "security policy" for disabling
> namespaces.
> 
> The clone() call will set errno to ENOSPC under such a limitation.  We
> didn't check ENOSPC in the code so the test will FAIL, and report:
> 
>     unable to unshare user/fs: No space left on device
> 
> This message is, unfortunately, very unhelpful.  It leads people to
> check the memory or disk space, instead of finding the real issue.
> 
> To improve the situation, we should check for ENOSPC and return
> UNSUPPORTED as the test result.  Also refactor
> check_for_unshare_hints()
> to emit a proper message telling people how to make the test work, if
> they really need to run the namespaced tests.
> 
> Reported-by: Philippe Delavalade <philippe.delavalade@orange.fr>
> URL:
> https://lists.linuxfromscratch.org/sympa/arc/lfs-support/2022-06/msg00022.html
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> Reviewed-by: DJ Delorie <dj@redhat.com>
> ---
>  support/test-container.c | 67 +++++++++++++++++++++------------------
> -
>  1 file changed, 36 insertions(+), 31 deletions(-)
> 
> diff --git a/support/test-container.c b/support/test-container.c
> index 7557aac441..b6a1158ae1 100644
> --- a/support/test-container.c
> +++ b/support/test-container.c
> @@ -18,6 +18,7 @@
>  
>  #define _FILE_OFFSET_BITS 64
>  
> +#include <array_length.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -684,39 +685,43 @@ rsync (char *src, char *dest, int and_delete,
> int force_copies)
>  /* See if we can detect what the user needs to do to get unshare
>     support working for us.  */
>  void
> -check_for_unshare_hints (void)
> +check_for_unshare_hints (int require_pidns)
>  {
> +  static struct {
> +    const char *path;
> +    int bad_value, good_value, for_pidns;
> +  } files[] = {
> +    /* Default Debian Linux disables user namespaces, but allows a
> way
> +       to enable them.  */
> +    { "/proc/sys/kernel/unprivileged_userns_clone", 0, 1, 0 },
> +    /* ALT Linux has an alternate way of doing the same.  */
> +    { "/proc/sys/kernel/userns_restrict", 1, 0, 0 },
> +    /* Linux kernel >= 4.9 has a configurable limit on the number of
> +       each namespace.  Some distros set the limit to zero to disable
> the
> +       corresponding namespace as a "security policy".  */
> +    { "/proc/sys/user/max_user_namespaces", 0, 1024, 0 },
> +    { "/proc/sys/user/max_mnt_namespaces", 0, 1024, 0 },
> +    { "/proc/sys/user/max_pid_namespaces", 0, 1024, 1 },
> +  };
>    FILE *f;
> -  int i;
> +  int i, val;
>  
> -  /* Default Debian Linux disables user namespaces, but allows a way
> -     to enable them.  */
> -  f = fopen ("/proc/sys/kernel/unprivileged_userns_clone", "r");
> -  if (f != NULL)
> +  for (i = 0; i < array_length (files); i++)
>      {
> -      i = 99; /* Sentinel.  */
> -      fscanf (f, "%d", &i);
> -      if (i == 0)
> -       {
> -         printf ("To enable test-container, please run this as
> root:\n");
> -         printf ("  echo 1 >
> /proc/sys/kernel/unprivileged_userns_clone\n");
> -       }
> -      fclose (f);
> -      return;
> -    }
> +      if (!require_pidns && files[i].for_pidns)
> +        continue;
>  
> -  /* ALT Linux has an alternate way of doing the same.  */
> -  f = fopen ("/proc/sys/kernel/userns_restrict", "r");
> -  if (f != NULL)
> -    {
> -      i = 99; /* Sentinel.  */
> -      fscanf (f, "%d", &i);
> -      if (i == 1)
> -       {
> -         printf ("To enable test-container, please run this as
> root:\n");
> -         printf ("  echo 0 > /proc/sys/kernel/userns_restrict\n");
> -       }
> -      fclose (f);
> +      f = fopen (files[i].path, "r");
> +      if (f == NULL)
> +        continue;
> +
> +      val = -1; /* Sentinel.  */
> +      fscanf (f, "%d", &val);
> +      if (val != files[i].bad_value)
> +       continue;
> +
> +      printf ("To enable test-container, please run this as
> root:\n");
> +      printf ("  echo %d > %s\n", files[i].good_value,
> files[i].path);
>        return;
>      }
>  }
> @@ -1117,11 +1122,11 @@ main (int argc, char **argv)
>      {
>        /* Older kernels may not support all the options, or security
>          policy may block this call.  */
> -      if (errno == EINVAL || errno == EPERM)
> +      if (errno == EINVAL || errno == EPERM || errno == ENOSPC)
>         {
>           int saved_errno = errno;
> -         if (errno == EPERM)
> -           check_for_unshare_hints ();
> +         if (errno == EPERM || errno == ENOSPC)
> +           check_for_unshare_hints (require_pidns);
>           FAIL_UNSUPPORTED ("unable to unshare user/fs: %s", strerror
> (saved_errno));
>         }
>        /* We're about to exit anyway, it's "safe" to call unshare
> again

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v2] test-container: return UNSUPPORTED for ENOSPC on clone()

[DJ Delorie]

Xi Ruoyao <xry111@xry111.site> writes:
> Revised patch following.  I don't have write access to glibc.git so I
> guess you need to push the patch.

LGTM.  Pushed.  Thanks!