net: mount a new tmpfs if it isn't enough rights to open /run/xtables.lock

Submitted by Andrei Vagin on May 4, 2018, 7:54 p.m.

Details

Message ID 20180504195421.19026-1-avagin@openvz.org
State Rejected
Series "net: mount a new tmpfs if it isn't enough rights to open /run/xtables.lock"
Headers show

Commit Message

Andrei Vagin May 4, 2018, 7:54 p.m.
Starting with iptables 1.6.2, we have to use the --wait option,
but it doesn't work properly with userns, because in this case,
we don't have enough rights to open /run/xtables.lock.

(00.174703)      1: 	Running iptables-restore -w for iptables-restore -w Fatal: can't open lock file /run/xtables.lock: Permission denied
(00.192058)      1: Error (criu/util.c:842): exited, status=4
(00.192080)      1: Error (criu/net.c:1738): iptables-restore -w failed
(00.192088)      1: Error (criu/net.c:2389): Can't create net_ns
(00.192131)      1: Error (criu/util.c:1567): Can't wait or bad status: errno=0, status=65280

This patch workarounds this problem by mounting tmpfs into /run.

https://github.com/checkpoint-restore/criu/issues/469

Cc: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Signed-off-by: Andrei Vagin <avagin@openvz.org>
---
 criu/net.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Patch hide | download patch | download mbox

diff --git a/criu/net.c b/criu/net.c
index 6430bdfcb..27f11179f 100644
--- a/criu/net.c
+++ b/criu/net.c
@@ -1965,11 +1965,54 @@  out:
 	return ret;
 }
 
+/*
+ * iptables-restore is executed from a target userns and it may have not enough
+ * rights to open /run/xtables.lock. Here we try to workaround this problem.
+ */
+static int replace_xtable_lock() { static bool done; int fd;
+
+	if (!(root_ns_mask & CLONE_NEWUSER))
+		return 0;
+
+	/* It is a question what to do when we have userns without mntns */
+	if (!(root_ns_mask & CLONE_NEWNS))
+		return 0;
+
+	if (done)
+		return 0;
+
+	fd = open("/run/xtables.lock", O_RDONLY);
+	if (fd >= 0) {
+		close(fd);
+		done = true;
+		return 0;
+	}
+
+	/*
+	 * /run/xtables.lock may not exist, so we can't just bind-mount a file
+	 * over it.
+	 * A new mount will not be propagated to the host mount namespace,
+	 * because we are in another userns.
+	 */
+
+	if (mount("criu-xtable-lock", "/run", "tmpfs", 0, NULL)) {
+		pr_perror("Unable to mount tmpfs into /run");
+		return -1;
+	}
+
+	done = true;
+
+	return 0;
+}
+
 static inline int restore_iptables(int pid)
 {
 	int ret = -1;
 	struct cr_img *img;
 
+	if (replace_xtable_lock())
+		return -1;
+
 	img = open_image(CR_FD_IPTABLES, O_RSTR, pid);
 	if (img == NULL)
 		return -1;

Comments

Kirill Tkhai May 7, 2018, 8:46 a.m.
Hi, Andrei,

On 04.05.2018 22:54, Andrei Vagin wrote:
> Starting with iptables 1.6.2, we have to use the --wait option,
> but it doesn't work properly with userns, because in this case,
> we don't have enough rights to open /run/xtables.lock.
> 
> (00.174703)      1: 	Running iptables-restore -w for iptables-restore -w Fatal: can't open lock file /run/xtables.lock: Permission denied
> (00.192058)      1: Error (criu/util.c:842): exited, status=4
> (00.192080)      1: Error (criu/net.c:1738): iptables-restore -w failed
> (00.192088)      1: Error (criu/net.c:2389): Can't create net_ns
> (00.192131)      1: Error (criu/util.c:1567): Can't wait or bad status: errno=0, status=65280
> 
> This patch workarounds this problem by mounting tmpfs into /run.
> 
> https://github.com/checkpoint-restore/criu/issues/469
> 
> Cc: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
> Signed-off-by: Andrei Vagin <avagin@openvz.org>
> ---
>  criu/net.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/criu/net.c b/criu/net.c
> index 6430bdfcb..27f11179f 100644
> --- a/criu/net.c
> +++ b/criu/net.c
> @@ -1965,11 +1965,54 @@ out:
>  	return ret;
>  }
>  
> +/*
> + * iptables-restore is executed from a target userns and it may have not enough
> + * rights to open /run/xtables.lock. Here we try to workaround this problem.
> + */
> +static int replace_xtable_lock() { static bool done; int fd;
> +
> +	if (!(root_ns_mask & CLONE_NEWUSER))
> +		return 0;
> +
> +	/* It is a question what to do when we have userns without mntns */
> +	if (!(root_ns_mask & CLONE_NEWNS))
> +		return 0;
> +
> +	if (done)
> +		return 0;
> +
> +	fd = open("/run/xtables.lock", O_RDONLY);
> +	if (fd >= 0) {
> +		close(fd);
> +		done = true;
> +		return 0;
> +	}
> +
> +	/*
> +	 * /run/xtables.lock may not exist, so we can't just bind-mount a file
> +	 * over it.
> +	 * A new mount will not be propagated to the host mount namespace,
> +	 * because we are in another userns.
> +	 */
> +
> +	if (mount("criu-xtable-lock", "/run", "tmpfs", 0, NULL)) {
> +		pr_perror("Unable to mount tmpfs into /run");
> +		return -1;
> +	}

Where we umount this one? Shouldn't we do that?

> +
> +	done = true;
> +
> +	return 0;
> +}
> +
>  static inline int restore_iptables(int pid)
>  {
>  	int ret = -1;
>  	struct cr_img *img;
>  
> +	if (replace_xtable_lock())
> +		return -1;
> +
>  	img = open_image(CR_FD_IPTABLES, O_RSTR, pid);
>  	if (img == NULL)
>  		return -1;
> 

Kirill
Andrey Vagin May 7, 2018, 9:36 p.m.
On Mon, May 07, 2018 at 11:46:11AM +0300, Kirill Tkhai wrote:
> Hi, Andrei,
> 
> On 04.05.2018 22:54, Andrei Vagin wrote:
> > Starting with iptables 1.6.2, we have to use the --wait option,
> > but it doesn't work properly with userns, because in this case,
> > we don't have enough rights to open /run/xtables.lock.
> > 
> > (00.174703)      1: 	Running iptables-restore -w for iptables-restore -w Fatal: can't open lock file /run/xtables.lock: Permission denied
> > (00.192058)      1: Error (criu/util.c:842): exited, status=4
> > (00.192080)      1: Error (criu/net.c:1738): iptables-restore -w failed
> > (00.192088)      1: Error (criu/net.c:2389): Can't create net_ns
> > (00.192131)      1: Error (criu/util.c:1567): Can't wait or bad status: errno=0, status=65280
> > 
> > This patch workarounds this problem by mounting tmpfs into /run.
> > 
> > https://github.com/checkpoint-restore/criu/issues/469
> > 
> > Cc: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> > Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
> > Signed-off-by: Andrei Vagin <avagin@openvz.org>
> > ---
> >  criu/net.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> > 
> > diff --git a/criu/net.c b/criu/net.c
> > index 6430bdfcb..27f11179f 100644
> > --- a/criu/net.c
> > +++ b/criu/net.c
> > @@ -1965,11 +1965,54 @@ out:
> >  	return ret;
> >  }
> >  
> > +/*
> > + * iptables-restore is executed from a target userns and it may have not enough
> > + * rights to open /run/xtables.lock. Here we try to workaround this problem.
> > + */
> > +static int replace_xtable_lock() { static bool done; int fd;
> > +
> > +	if (!(root_ns_mask & CLONE_NEWUSER))
> > +		return 0;
> > +
> > +	/* It is a question what to do when we have userns without mntns */
> > +	if (!(root_ns_mask & CLONE_NEWNS))
> > +		return 0;
> > +
> > +	if (done)
> > +		return 0;
> > +
> > +	fd = open("/run/xtables.lock", O_RDONLY);
> > +	if (fd >= 0) {
> > +		close(fd);
> > +		done = true;
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * /run/xtables.lock may not exist, so we can't just bind-mount a file
> > +	 * over it.
> > +	 * A new mount will not be propagated to the host mount namespace,
> > +	 * because we are in another userns.
> > +	 */
> > +
> > +	if (mount("criu-xtable-lock", "/run", "tmpfs", 0, NULL)) {
> > +		pr_perror("Unable to mount tmpfs into /run");
> > +		return -1;
> > +	}
> 
> Where we umount this one? Shouldn't we do that?

it will be umounted when we will do pivot_root().

> 
> > +
> > +	done = true;
> > +
> > +	return 0;
> > +}
> > +
> >  static inline int restore_iptables(int pid)
> >  {
> >  	int ret = -1;
> >  	struct cr_img *img;
> >  
> > +	if (replace_xtable_lock())
> > +		return -1;
> > +
> >  	img = open_image(CR_FD_IPTABLES, O_RSTR, pid);
> >  	if (img == NULL)
> >  		return -1;
> > 
> 
> Kirill