[iproute PATCH] lib/namespace: avoid double-mounting a /sys

[iproute PATCH] lib/namespace: avoid double-mounting a /sys

[Lubomir Rintel]

This partly reverts 8f0807023d067e2bb585a2ae8da93e59689d10f1, bringing
back the umount(/sys) attempt.

In a LXC container we're unable to umount the sysfs instance, nor mount
a read-write one. We still are able to create a new read-only instance.

Nevertheless, it still makes sense to attempt the umount() even though
the sysfs is mounted read-only. Otherwise we may end up attempting to
mount a sysfs with the same flags as is already mounted, resulting in
an EBUSY error (meaning "Already mounted").

Perhaps this is not a very likely scenario in real world, but we hit
it in NetworkManager test suite and makes netns_switch() somewhat more
robust. It also fixes the case, when /sys wasn't mounted at all.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 lib/namespace.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/lib/namespace.c b/lib/namespace.c
index 43e0fe34..06ae0a48 100644
--- a/lib/namespace.c
+++ b/lib/namespace.c
@@ -82,19 +82,13 @@ int netns_switch(char *name)
 
 	/* Mount a version of /sys that describes the network namespace */
 
-	if (statvfs("/sys", &fsstat) < 0) {
-		fprintf(stderr, "could not stat /sys (not mounted?): %s\n",strerror(errno));
-		return -1;
-	}
-	if (fsstat.f_flag & ST_RDONLY) {
-		/* If /sys is not writable (e.g. in a container), we can't
-		 * unmount the old /sys instance, but we can still mount a new
-		 * read-only instance over it. */
-		mountflags = MS_RDONLY;
-	} else {
-		if (umount2("/sys", MNT_DETACH) < 0) {
-			fprintf(stderr, "umount of /sys failed: %s\n", strerror(errno));
-			return -1;
+	if (umount2("/sys", MNT_DETACH) < 0) {
+		/* If this fails, perhaps there wasn't a sysfs instance mounted. Good. */
+		if (statvfs("/sys", &fsstat) == 0) {
+			/* We couldn't umount the sysfs, we'll attempt to overlay it.
+			 * A read-only instance can't be shadowed with a read-write one. */
+			if (fsstat.f_flag & ST_RDONLY)
+				mountflags = MS_RDONLY;
 		}
 	}
 	if (mount(name, "/sys", "sysfs", mountflags, NULL) < 0) {
-- 
2.17.1

Re: [iproute PATCH] lib/namespace: avoid double-mounting a /sys

[Stephen Hemminger]

On Tue, 24 Jul 2018 19:26:38 +0200
Lubomir Rintel <lkundrak@v3.sk> wrote:

> This partly reverts 8f0807023d067e2bb585a2ae8da93e59689d10f1, bringing
> back the umount(/sys) attempt.
> 
> In a LXC container we're unable to umount the sysfs instance, nor mount
> a read-write one. We still are able to create a new read-only instance.
> 
> Nevertheless, it still makes sense to attempt the umount() even though
> the sysfs is mounted read-only. Otherwise we may end up attempting to
> mount a sysfs with the same flags as is already mounted, resulting in
> an EBUSY error (meaning "Already mounted").
> 
> Perhaps this is not a very likely scenario in real world, but we hit
> it in NetworkManager test suite and makes netns_switch() somewhat more
> robust. It also fixes the case, when /sys wasn't mounted at all.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

Makes sens applied.