On Wed, May 3, 2017 at 8:01 PM, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> This sounds to me like a wrong design decision was made when
> introducing this userspace lock.
Looks like the lock was introduced by
93587a04d0f2 ("ip[6]tables: Add locking to prevent concurrent instances")
which made the lock optional for backwards compatibility:
xt_socket = socket(AF_UNIX, SOCK_STREAM, 0);
/* If we can't even create a socket, fall back to prior
(lockless) behavior */
if (xt_socket < 0)
return true;
That was almost four years ago. Sometime between 1.4 and 1.6, the lock
was switched to use flock() on /run/xtables.lock , but the fail-open
behaviour was preserved.
> I would like to skip this compile time switch, if the existing
> behaviour is broken, we should just fix it. What is the scenario that
> can indeed have an impact in terms of backward compatibility breakage?
> Does it really make sense to keep a buggy behaviour around?
I agree that the current behaviour is dangerous and error-prone. It's
bitten us badly several times now. The only use case I came up with is
running commands early on boot, from a read-only filesystem that does
not have /run/xtables.lock. If we change the behaviour, that system
might become unbootable on an iptables upgrade.
Another issue mentioned in the initial socket-based approach was that
no locking is necessary for iptables commands running in different
network namespaces. Someone with such a system might be relying on
this. The impact of this change would be to slow down concurrent
iptables commands.
These both seem like a very contrived cases though. I'll send out
another patch that changes the behaviour.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
