[v2] support_become_root: Don't fail when /proc/<pid/setgroups is missing

Message ID 20171118151311.20305-1-christian.brauner@ubuntu.com
State New, archived
Headers

Commit Message

Christian Brauner Nov. 18, 2017, 3:13 p.m. UTC
  The requirement to write "deny" to /proc/<pid>/setgroups for a given user
namespace before being able to write a gid mapping was introduced in Linux
3.19.  Before that this requirement including the file did not exist.
So don't fail when errno == ENOENT.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
Changelog 2017-11-18:
* Restrice line length to 79 char instead of 80.
* Use two spaces after a period at the end of a sentence.
* Use two spaces before the closing comment marker "*/".
---
 ChangeLog                     |  5 +++++
 support/support_become_root.c | 21 ++++++++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)
  

Comments

Florian Weimer Nov. 18, 2017, 3:15 p.m. UTC | #1
On 11/18/2017 04:13 PM, Christian Brauner wrote:
> The requirement to write "deny" to/proc/<pid>/setgroups for a given user
> namespace before being able to write a gid mapping was introduced in Linux
> 3.19.  Before that this requirement including the file did not exist.
> So don't fail when errno == ENOENT.
> 
> Signed-off-by: Christian Brauner<christian.brauner@ubuntu.com>

glibc uses an FSF copyright assignment and not the DCO, so this line is 
misleading.

Patch looks okay, please commit.

Thanks,
Florian
  
Christian Brauner Nov. 18, 2017, 3:19 p.m. UTC | #2
On Sat, Nov 18, 2017 at 04:15:41PM +0100, Florian Weimer wrote:
> On 11/18/2017 04:13 PM, Christian Brauner wrote:
> > The requirement to write "deny" to/proc/<pid>/setgroups for a given user
> > namespace before being able to write a gid mapping was introduced in Linux
> > 3.19.  Before that this requirement including the file did not exist.
> > So don't fail when errno == ENOENT.
> > 
> > Signed-off-by: Christian Brauner<christian.brauner@ubuntu.com>
> 
> glibc uses an FSF copyright assignment and not the DCO, so this line is
> misleading.

Right, I'm removing it. It gets auto-added due to my .gitconfig. :)

Thanks!
Christian
  
Florian Weimer Nov. 18, 2017, 3:22 p.m. UTC | #3
On 11/18/2017 04:19 PM, Christian Brauner wrote:
> On Sat, Nov 18, 2017 at 04:15:41PM +0100, Florian Weimer wrote:
>> On 11/18/2017 04:13 PM, Christian Brauner wrote:
>>> The requirement to write "deny" to/proc/<pid>/setgroups for a given user
>>> namespace before being able to write a gid mapping was introduced in Linux
>>> 3.19.  Before that this requirement including the file did not exist.
>>> So don't fail when errno == ENOENT.
>>>
>>> Signed-off-by: Christian Brauner<christian.brauner@ubuntu.com>
>>
>> glibc uses an FSF copyright assignment and not the DCO, so this line is
>> misleading.
> 
> Right, I'm removing it. It gets auto-added due to my .gitconfig. :)

Please consider disabling this in .git/config for the glibc checkout.

Thanks,
Florian
  
Christian Brauner Nov. 18, 2017, 3:25 p.m. UTC | #4
On Sat, Nov 18, 2017 at 04:22:51PM +0100, Florian Weimer wrote:
> On 11/18/2017 04:19 PM, Christian Brauner wrote:
> > On Sat, Nov 18, 2017 at 04:15:41PM +0100, Florian Weimer wrote:
> > > On 11/18/2017 04:13 PM, Christian Brauner wrote:
> > > > The requirement to write "deny" to/proc/<pid>/setgroups for a given user
> > > > namespace before being able to write a gid mapping was introduced in Linux
> > > > 3.19.  Before that this requirement including the file did not exist.
> > > > So don't fail when errno == ENOENT.
> > > > 
> > > > Signed-off-by: Christian Brauner<christian.brauner@ubuntu.com>
> > > 
> > > glibc uses an FSF copyright assignment and not the DCO, so this line is
> > > misleading.
> > 
> > Right, I'm removing it. It gets auto-added due to my .gitconfig. :)
> 
> Please consider disabling this in .git/config for the glibc checkout.

Yeah, was about to do that.

Thanks!
Christian
  
Andreas Schwab Nov. 18, 2017, 3:33 p.m. UTC | #5
On Nov 18 2017, Christian Brauner <christian.brauner@ubuntu.com> wrote:

> @@ -50,11 +51,21 @@ setup_uid_gid_mapping (uid_t original_uid, gid_t original_gid)
>    xwrite (fd, buf, ret);
>    xclose (fd);
>  
> -  /* Disable setgroups before mapping groups, otherwise that would
> -     fail with EPERM.  */
> -  fd = xopen ("/proc/self/setgroups", O_WRONLY, 0);
> -  xwrite (fd, "deny\n", strlen ("deny\n"));
> -  xclose (fd);
> +  /* Linux 3.19 introduced the setgroups file.  We need write "deny" to this
> +   * file otherwise writing to gid_map will fail with EPERM.  */

Wrong comment style.

Andreas.
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 604d571ca6..74b77dfa41 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@ 
+2017-11-17  Christian Brauner <christian.brauner@ubuntu.com>
+
+	* support/support_become_root.c (setup_uid_gid_mapping): Don't fail
+	when /proc/<pid>/setgroups does not exist.
+
 2017-11-17  Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>
 
 	* sysdeps/powerpc/bits/hwcap.h (PPC_FEATURE2_HTM_NO_SUSPEND): New
diff --git a/support/support_become_root.c b/support/support_become_root.c
index 5086570251..e45c939421 100644
--- a/support/support_become_root.c
+++ b/support/support_become_root.c
@@ -18,6 +18,7 @@ 
 
 #include <support/namespace.h>
 
+#include <errno.h>
 #include <fcntl.h>
 #include <sched.h>
 #include <stdio.h>
@@ -50,11 +51,21 @@  setup_uid_gid_mapping (uid_t original_uid, gid_t original_gid)
   xwrite (fd, buf, ret);
   xclose (fd);
 
-  /* Disable setgroups before mapping groups, otherwise that would
-     fail with EPERM.  */
-  fd = xopen ("/proc/self/setgroups", O_WRONLY, 0);
-  xwrite (fd, "deny\n", strlen ("deny\n"));
-  xclose (fd);
+  /* Linux 3.19 introduced the setgroups file.  We need write "deny" to this
+   * file otherwise writing to gid_map will fail with EPERM.  */
+  fd = open64 ("/proc/self/setgroups", O_WRONLY, 0);
+  if (fd < 0)
+    {
+      if (errno != ENOENT)
+        FAIL_EXIT1 ("open64 (\"/proc/self/setgroups\", 0x%x, 0%o): %m",
+                    O_WRONLY, 0);
+      /* This kernel doesn't expose the setgroups file so simply move on.  */
+    }
+  else
+    {
+      xwrite (fd, "deny\n", strlen ("deny\n"));
+      xclose (fd);
+    }
 
   /* Now map our own GID, like we did for the user ID.  */
   fd = xopen ("/proc/self/gid_map", O_WRONLY, 0);