| 14bc2165deb5f53bf797097e009da8af96e9fa81 |
|
19-May-2017 |
Lorenzo Colitti <lorenzo@google.com> |
UPSTREAM: iptables: insist that the lock is held.
Currently, iptables programs will exit with an error if the
iptables lock cannot be acquired, but will silently continue if
the lock cannot be opened at all. This can cause unexpected
failures (with unhelpful error messages) in the presence of
concurrent updates, which can be very difficult to find in a
complex or multi-administrator system.
Instead, refuse to do anything if the lock cannot be acquired.
The behaviour is not affected by command-line flags because:
1. In order to reliably avoid concurrent modification, all
invocations of iptables commands must follow this behaviour.
2. Whether or not the lock can be opened is typically not
a run-time condition but is likely to be a configuration
error.
Existing systems that depended on things working mostly correctly
even if there was no lock might be affected by this change.
However, that is arguably a configuration error, and now that the
iptables lock is configurable, it is trivial to provide a lock
file that is always accessible: if nothing else, the iptables
binary itself can be used. The lock does not have to be writable,
only readable.
Tested by configuring the system to use an xtables.lock file in
a non-existent directory and observing that all commands failed.
(cherry picked from iptables 80d8bfaac9e2430d710084a10ec78e68bd61e6ec)
Test: aosp_bullhead-eng builds
Change-Id: I1aec4eb2d9e3775806c93ccd6cf215af05e12f3c
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
/external/iptables/iptables/xshared.c
|
| 1e95b6c9171061d950d0a76a1f39e1be3db6cb09 |
|
16-Mar-2017 |
Lorenzo Colitti <lorenzo@google.com> |
iptables-restore: support acquiring the lock.
Currently, ip[6]tables-restore does not perform any locking, so it
is not safe to use concurrently with ip[6]tables.
This patch makes ip[6]tables-restore wait for the lock if -w
was specified. Arguments to -w and -W are supported in the same
was as they are in ip[6]tables.
The lock is not acquired on startup. Instead, it is acquired when
a new table handle is created (on encountering '*') and released
when the table is committed (COMMIT). This makes it possible to
keep long-running iptables-restore processes in the background
(for example, reading commands from a pipe opened by a system
management daemon) and simultaneously run iptables commands.
If -w is not specified, then the command proceeds without taking
the lock.
Tested as follows:
1. Run iptables-restore -w, and check that iptables commands work
with or without -w.
2. Type "*filter" into the iptables-restore input. Verify that
a) ip[6]tables commands without -w fail with "another app is
currently holding the xtables lock...".
b) ip[6]tables commands with "-w 2" fail after 2 seconds.
c) ip[6]tables commands with "-w" hang until "COMMIT" is
typed into the iptables-restore window.
3. With the lock held by an ip6tables-restore process:
strace -e flock /tmp/iptables/sbin/iptables-restore -w 1 -W 100000
shows 11 calls to flock and fails.
4. Run an iptables-restore with -w and one without -w, and check:
a) Type "*filter" in the first and then the second, and the
second exits with an error.
b) Type "*filter" in the second and "*filter" "-S" "COMMIT"
into the first. The rules are listed only when the first
copy sees "COMMIT".
Signed-off-by: Narayan Kamath <narayan@google.com>
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
(cherry picked from commit 999eaa241212d3952ddff39a99d0d55a74e3639e)
Bug: 36108349
Test: see top of change stack.
Change-Id: I2a51fab1c169763db00124641459dde2ed6c4c97
/external/iptables/iptables/xshared.c
|
| 097d6bee9cb25c94e288ad72099c51bab7fe113c |
|
16-Mar-2017 |
Lorenzo Colitti <lorenzo@google.com> |
iptables: remove duplicated argument parsing code
1. Factor out repeated code to a new xs_has_arg function.
2. Add a new parse_wait_time option to parse the value of -w.
3. Make parse_wait_interval take argc and argv so its callers
can be simpler.
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
(cherry picked from commit 6e2e169eb66b63d2991e1c7ada931e3cdb0ced32)
Bug: 36108349
Test: see top of change stack.
Change-Id: Iae185e267d90806dac2cbfdad2a066a2929947fc
/external/iptables/iptables/xshared.c
|
| 0e96b2160f961dbbdb4b0803a256c11beec1d264 |
|
15-Mar-2017 |
Lorenzo Colitti <lorenzo@google.com> |
iptables: move XT_LOCK_NAME from CFLAGS to config.h.
This slightly simplifies configure.ac and results in more
correct dependencies.
Tested by running ./configure with --with-xt-lock-name and
without, and using strace to verify that the right lock is used.
$ make distclean-recursive && ./autogen.sh &&
./configure --disable-nftables --prefix /tmp/iptables &&
make -j64 &&
make install &&
sudo strace -e open,flock /tmp/iptables/sbin/iptables -L foo
...
open("/run/xtables.lock", O_RDONLY|O_CREAT, 0600) = 3
flock(3, LOCK_EX|LOCK_NB) = 0
$ make distclean-recursive && ./autogen.sh && \
./configure --disable-nftables --prefix /tmp/iptables \
--with-xt-lock-name=/tmp/iptables/run/xtables.lock &&
make -j64 &&
make install &&
sudo strace -e open,flock /tmp/iptables/sbin/iptables -L foo
...
open("/tmp/iptables/run/xtables.lock", O_RDONLY|O_CREAT, 0600) = 3
flock(3, LOCK_EX|LOCK_NB) = 0
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
(cherry picked from commit 836846f0d747e1be8e37d2d43b215a68b30ea1a9)
Bug: 36108349
Test: see top of change stack.
Change-Id: I390ab17eadde6d22fa1ad9ce3a7bf4c6b2fb1b8a
/external/iptables/iptables/xshared.c
|
| e01761f43e50ee40d7c171ed60901dd5b352df1f |
|
14-Mar-2017 |
Lorenzo Colitti <lorenzo@google.com> |
iptables: set the path of the lock file via a configure option.
Currently the iptables lock is hardcoded as "/run/xtables.lock".
Allow users to change this path using the --with-xt-lock-name
option to ./configure option. This is useful on systems like
Android which do not have /run.
Tested on Ubuntu, as follows:
1. By default, the lock is placed in /run/xtables.lock:
$ make distclean-recursive && ./autogen.sh &&
./configure --disable-nftables --prefix /tmp/iptables &&
make -j64 &&
make install &&
sudo strace -e open,flock /tmp/iptables/sbin/iptables -L foo
...
open("/run/xtables.lock", O_RDONLY|O_CREAT, 0600) = 3
flock(3, LOCK_EX|LOCK_NB) = 0
iptables: No chain/target/match by that name.
2. Specifying the lock results in the expected location being
used:
$ make distclean-recursive && ./autogen.sh && \
./configure --disable-nftables --prefix /tmp/iptables \
--with-xt-lock-name=/tmp/iptables/run/xtables.lock &&
make -j64 &&
make install &&
sudo strace -e open,flock /tmp/iptables/sbin/iptables -L foo
...
open("/tmp/iptables/run/xtables.lock", O_RDONLY|O_CREAT, 0600) = 3
flock(3, LOCK_EX|LOCK_NB) = 0
iptables: No chain/target/match by that name.
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
(cherry picked from commit b91af533f4da15854893ba5cc082e1df6bcf9a97)
Bug: 36108349
Test: see top of change stack.
Change-Id: Ia834d3f3043822031220aeaffc5a75cc48c4fa83
/external/iptables/iptables/xshared.c
|
| e493a2243415f8984c5eed132168d0fcfb4fd522 |
|
06-Feb-2017 |
Liping Zhang <zlpnobody@gmail.com> |
xshared: using the blocking file lock request when we wait indefinitely
When using "-w" to avoid concurrent instances, we try to do flock() every
one second until it success. But one second maybe too long in some
situations, and it's hard to select a suitable interval time. So when
using "iptables -w" to wait indefinitely, it's better to block until
it become success.
Now do some performance tests. First, flush all the iptables rules in
filter table, and run "iptables -w -S" endlessly:
# iptables -F
# iptables -X
# while : ; do
iptables -w -S >&- &
done
Second, after adding and deleting the iptables rules 100 times, measure
the time cost:
# time for i in $(seq 100); do
iptables -w -A INPUT
iptables -w -D INPUT
done
Before this patch:
real 1m15.962s
user 0m0.224s
sys 0m1.475s
Apply this patch:
real 0m1.830s
user 0m0.168s
sys 0m1.130s
Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
(cherry picked from commit 72bb3dbf0ecdf3ec96aee80e5d152c8be4394da1)
Bug: 36108349
Test: see top of change stack.
Change-Id: I2a522dc9a9cb5f5b2c5bbf6b40da525bb1c4e90f
/external/iptables/iptables/xshared.c
|
| f9da6abd2eb2ee4d3e20ea49e8d6707c32da32b6 |
|
05-Feb-2017 |
Liping Zhang <zlpnobody@gmail.com> |
xshared: do not lock again and again if "-w" option is not specified
After running the following commands, some confusing messages was printed
out:
# while : ; do
iptables -A INPUT &
iptables -D INPUT &
done
[...]
Another app is currently holding the xtables lock; still -9s 0us time
ahead to have a chance to grab the lock...
Another app is currently holding the xtables lock; still -29s 0us time
ahead to have a chance to grab the lock...
If "-w" option is not specified, the "wait" will be zero, so we should
check whether the timer_left is less than wait_interval before we call
select to sleep.
Also remove unused "BASE_MICROSECONDS" and "struct timeval waited_time"
introduced by commit e8f857a5a151 ("xtables: Add an interval option for
xtables lock wait").
Fixes: e8f857a5a151 ("xtables: Add an interval option for xtables lock wait")
Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
(cherry picked from commit 24f8174646123c2833bc87967b366796231b04e0)
Bug: 36108349
Test: see top of change stack.
Change-Id: I54da3f5b1390f4e4cc20523575925aa2a428f39b
/external/iptables/iptables/xshared.c
|
| e8f857a5a1514c3e7d0d8ea0f7d2d571f0e37bd1 |
|
24-Jun-2016 |
Subash Abhinov Kasiviswanathan <subashab@codeaurora.org> |
xtables: Add an interval option for xtables lock wait
ip[6]tables currently waits for 1 second for the xtables lock to be
freed if the -w option is used. We have seen that the lock is held
much less than that resulting in unnecessary delay when trying to
acquire the lock. This problem is even severe in case of latency
sensitive applications.
Introduce a new option 'W' to specify the wait interval in microseconds.
If this option is not specified, the command sleeps for 1 second by
default.
v1->v2: Change behavior to take millisecond sleep as an argument to
-w as suggested by Pablo. Also maintain current behavior for -w to
sleep for 1 second as mentioned by Liping.
v2->v3: Move the millisecond behavior to a new option as suggested
by Pablo.
v3->v4: Use select instead of usleep. Sleep every iteration for
the time specified in the "-W" argument. Update man page.
v4->v5: Fix compilation error when enabling nftables
v5->v6: Simplify -W so it only takes the interval wait in microseconds.
Bail out if -W is specific but -w is not.
Joint work with Pablo Neira.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
/external/iptables/iptables/xshared.c
|
| 6dc53c514f1e4683e51a877b3a2f3128cfccef28 |
|
16-Feb-2015 |
Pablo Neira Ayuso <pablo@netfilter.org> |
xshared: calm down compilation warning
xshared.c: In function ‘xtables_lock’:
xshared.c:255:3: warning: implicit declaration of function ‘flock’ [-Wimplicit-function-declaration]
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
/external/iptables/iptables/xshared.c
|
| aa562a660d1555b13cffbac1e744033e91f82707 |
|
16-Jan-2015 |
Pablo Neira Ayuso <pablo@netfilter.org> |
iptables: use flock() instead of abstract unix sockets
Abstract unix sockets cannot be used to synchronize several concurrent
instances of iptables since an unpriviledged process can create them and
prevent the legitimate iptables instance from running.
Use flock() and /run instead as suggested by Lennart Poettering.
Fixes: 93587a0 ("ip[6]tables: Add locking to prevent concurrent instances")
Reported-by: Lennart Poettering <lennart@poettering.net>
Cc: Phil Oester <kernel@linuxace.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
/external/iptables/iptables/xshared.c
|
| aaa4ace72ba1d195bbf436134a336816c33f7bd0 |
|
04-Jul-2014 |
Jiri Popelka <jpopelka@redhat.com> |
iptables: add optional [seconds] argument to -w
This patch adds an optional numeric argument
to -w option (added with 93587a0) so one can
specify how long to wait for an exclusive lock.
If the value isn't specified it works as before,
i.e. program waits indefinitely.
If user specifies it, program exits after
the given time interval passes.
This patch also adds the -w/--wait to nftables
compat code, so the parser doesn't complain.
[ In the original patch, iptables-compat -w X was not working,
I have fixed by adding the dummy code not to break scripts
using the new optional argument --pablo ]
Signed-off-by: Jiri Popelka <jpopelka@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
/external/iptables/iptables/xshared.c
|
| 93587a04d0f2511e108bbc4d87a8b9d28a5c5dd8 |
|
31-May-2013 |
Phil Oester <kernel@linuxace.com> |
ip[6]tables: Add locking to prevent concurrent instances
There have been numerous complaints and bug reports over the years when admins
attempt to run more than one instance of iptables simultaneously. Currently
open bug reports which are related:
325: Parallel execution of the iptables is impossible
758: Retry iptables command on transient failure
764: Doing -Z twice in parallel breaks counters
822: iptables shows negative or other bad packet/byte counts
As Patrick notes in 325: "Since this has been a problem people keep running
into, I'd suggest to simply add some locking to iptables to catch the most
common case."
I started looking into alternatives to add locking, and of course the most
common/obvious solution is to use a pidfile. But this has various downsides,
such as if the application is terminated abnormally and the pidfile isn't
cleaned up. And this also requires a writable filesystem. Using a UNIX domain
socket file (e.g. in /var/run) has similar issues.
Starting in 2.2, Linux added support for abstract sockets. These sockets
require no filesystem, and automatically disappear once the application
terminates. This is the locking solution I chose to implement in ip[6]tables.
As an added bonus, since each network namespace has its own socket pool, an
ip[6]tables instance running in one namespace will not lock out an ip[6]tables
instance running in another namespace. A filesystem approach would have
to recognize and handle multiple network namespaces.
Signed-off-by: Phil Oester <kernel@linuxace.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
/external/iptables/iptables/xshared.c
|
| 14da56743c6cdf25da35b7b5ca7a5d201771990d |
|
27-Aug-2011 |
Jan Engelhardt <jengelh@medozas.de> |
src: resolve old macro names that are indirections
Command used:
git grep -f <(pcregrep -hior
'(?<=#define\s)IP6?(T_\w+)(?=\s+X\1)' include/)
and then fix all occurrences.
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
/external/iptables/iptables/xshared.c
|
| 2dba676b68ef842025f3afecba26cb0b2ae4c09b |
|
18-Jun-2011 |
Jan Engelhardt <jengelh@medozas.de> |
extensions: support for per-extension instance "global" variable space
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
/external/iptables/iptables/xshared.c
|
| 14190986f87301b18bcc473c842bd82d778d87a2 |
|
18-Jun-2011 |
Jan Engelhardt <jengelh@medozas.de> |
iptables: consolidate target/match init call
This is useful for the upcoming patch about per-instance auxiliary
data.
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
/external/iptables/iptables/xshared.c
|
| 780607f8b040a47cd2d4775376e2d30f567dc049 |
|
07-Jun-2011 |
Jan Engelhardt <jengelh@medozas.de> |
option: fix ignored negation before implicit extension loading
`iptables -A INPUT -p tcp ! --syn` forgot the negation, i.e. it
was not present in a subsequent `iptables -S`.
Commit v1.4.11~77^2~9 missed the fact that after autoloading a proto
extension, cs.invert must not be touched until the next getopt call.
This is now fixed by having command_default return a value to indicate
whether to jump or not.
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
/external/iptables/iptables/xshared.c
|
| 033e25a3ad215ee3f5a07f0a3315f74c4abfaced |
|
07-Jun-2011 |
Jan Engelhardt <jengelh@medozas.de> |
src: move all iptables pieces into a separate directory
(Unclutter top-level dir)
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
/external/iptables/iptables/xshared.c
|