[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
