[PATCH RFC iptables] iptables: Per-net ns lock
In CRIU and LXC-restore we met the situation, when iptables in container can't be restored because of permission denied: https://github.com/checkpoint-restore/criu/issues/469 Containers want to restore their own net ns, while they may have no their own mnt ns. This case they share host's /run/xtables.lock file, but they may not have permission to open it. Patch makes /run/xtables.lock to be per-namespace, i.e., to refer to the caller task's net ns. What you think? Thanks, Kirill Signed-off-by: Kirill Tkhai --- iptables/xshared.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/iptables/xshared.c b/iptables/xshared.c index 06db72d4..b6dbe4e7 100644 --- a/iptables/xshared.c +++ b/iptables/xshared.c @@ -254,7 +254,12 @@ static int xtables_lock(int wait, struct timeval *wait_interval) time_left.tv_sec = wait; time_left.tv_usec = 0; - fd = open(XT_LOCK_NAME, O_CREAT, 0600); + if (symlink("/proc/self/ns/net", XT_LOCK_NAME) != 0 && + errno != EEXIST) { + fprintf(stderr, "Fatal: can't create lock file\n"); + return XT_LOCK_FAILED; + } + fd = open(XT_LOCK_NAME, O_RDONLY); if (fd < 0) { fprintf(stderr, "Fatal: can't open lock file %s: %s\n", XT_LOCK_NAME, strerror(errno));
Re: [PATCH RFC iptables] iptables: Per-net ns lock
Pablo, Florian, could you please provide comments on this? On 09.04.2018 19:55, Kirill Tkhai wrote: > In CRIU and LXC-restore we met the situation, > when iptables in container can't be restored > because of permission denied: > > https://github.com/checkpoint-restore/criu/issues/469 > > Containers want to restore their own net ns, > while they may have no their own mnt ns. > This case they share host's /run/xtables.lock > file, but they may not have permission to open > it. > > Patch makes /run/xtables.lock to be per-namespace, > i.e., to refer to the caller task's net ns. > > What you think? > > Thanks, > Kirill > > Signed-off-by: Kirill Tkhai > --- > iptables/xshared.c |7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/iptables/xshared.c b/iptables/xshared.c > index 06db72d4..b6dbe4e7 100644 > --- a/iptables/xshared.c > +++ b/iptables/xshared.c > @@ -254,7 +254,12 @@ static int xtables_lock(int wait, struct timeval > *wait_interval) > time_left.tv_sec = wait; > time_left.tv_usec = 0; > > - fd = open(XT_LOCK_NAME, O_CREAT, 0600); > + if (symlink("/proc/self/ns/net", XT_LOCK_NAME) != 0 && > + errno != EEXIST) { > + fprintf(stderr, "Fatal: can't create lock file\n"); > + return XT_LOCK_FAILED; > + } > + fd = open(XT_LOCK_NAME, O_RDONLY); > if (fd < 0) { > fprintf(stderr, "Fatal: can't open lock file %s: %s\n", > XT_LOCK_NAME, strerror(errno)); >
Re: [PATCH RFC iptables] iptables: Per-net ns lock
Kirill Tkhai wrote: > Pablo, Florian, could you please provide comments on this? > > On 09.04.2018 19:55, Kirill Tkhai wrote: > > In CRIU and LXC-restore we met the situation, > > when iptables in container can't be restored > > because of permission denied: > > > > https://github.com/checkpoint-restore/criu/issues/469 > > > > Containers want to restore their own net ns, > > while they may have no their own mnt ns. > > This case they share host's /run/xtables.lock > > file, but they may not have permission to open > > it. > > > > Patch makes /run/xtables.lock to be per-namespace, > > i.e., to refer to the caller task's net ns. It looks ok to me but then again the entire userspace lock thing is a ugly band aid :-/
Re: [PATCH RFC iptables] iptables: Per-net ns lock
Hi, Florian, On 20.04.2018 13:50, Florian Westphal wrote: > Kirill Tkhai wrote: >> Pablo, Florian, could you please provide comments on this? >> >> On 09.04.2018 19:55, Kirill Tkhai wrote: >>> In CRIU and LXC-restore we met the situation, >>> when iptables in container can't be restored >>> because of permission denied: >>> >>> https://github.com/checkpoint-restore/criu/issues/469 >>> >>> Containers want to restore their own net ns, >>> while they may have no their own mnt ns. >>> This case they share host's /run/xtables.lock >>> file, but they may not have permission to open >>> it. >>> >>> Patch makes /run/xtables.lock to be per-namespace, >>> i.e., to refer to the caller task's net ns. > > It looks ok to me but then again the entire userspace > lock thing is a ugly band aid :-/ I'm agree, but I'm not sure there is a possibility to go away from it in classic iptables... Kirill