aboutsummaryrefslogtreecommitdiffstats
path: root/iptables/iptables-restore.c
Commit message (Collapse)AuthorAgeFilesLines
* UPSTREAM: iptables: insist that the lock is held.Lorenzo Colitti2017-05-311-6/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* iptables-restore: support acquiring the lock.Lorenzo Colitti2017-03-221-12/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Update to iptables 1.6.1.Lorenzo Colitti2017-03-221-7/+5
|\ | | | | | | | | | | | | | | | | | | | | | | This merges upstream b013e3e80e96 ("iptables 1.6.1 release") Conflicts: include/libiptc/ipt_kernel_headers.h Bug: 30950746 Bug: 36108349 Test: see top of change stack. Change-Id: Ib2b5ae0e0c330798aa375b153e3e2cba2348bb1c
| * iptables-restore: add missing arguments to usage messageBrian Haley2016-08-231-2/+2
| | | | | | | | | | | | | | | | | | iptables-restore was missing -n, -T and -M from the usage message, added them to match the man page. Cleaned-up other *restore files as well. Signed-off-by: Brian Haley <brian.haley@hpe.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
| * consistently use <errno.h>Felix Janda2015-05-021-1/+1
| | | | | | | | | | | | | | | | | | On glibc, <sys/errno.h> is a synomym for <errno.h>. <errno.h> is specified by POSIX, so use that. Fixes compilation error with musl libc Signed-off-by: Florian Westphal <fw@strlen.de>
| * iptables-{save,restore}: warn that -b/--binary isn't implementedJiri Popelka2014-03-171-5/+3
| | | | | | | | | | | | see also 296dca39be Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* | Revert "Add '-w' option to ip[6]tables-restore"Lorenzo Colitti2017-03-221-15/+3
| | | | | | | | | | | | | | | | | | | | | | This reverts commit 1d12f7bb86fba1d65938ed2c85b7a0f11424281b. This is being reverted to minimize diffs with upstream and will be cherry-picked once upstream is merged. Bug: 36108349 Test: see top of change stack. Change-Id: I1851882e4c388bc139ce718d111e58e702dcc58a
* | Revert "iptables: Change locking semantics."Lorenzo Colitti2017-03-221-15/+7
| | | | | | | | | | | | | | | | | | | | | | This reverts commit d2a1e52615058ef55b65db02aa5e4ad21b635ef0. This is being reverted to minimize diffs with upstream and will be cherry-picked once upstream is merged. Bug: 36108349 Test: see top of change stack. Change-Id: If8136bfd230bf0079884ab94fad0dcdc35a67c47
* | iptables: Change locking semantics.Narayan Kamath2017-01-231-7/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of acquiring a lock before we parse any commands and holding on to it until the process terminates, we now acquire the lock when a new table handle is created (on encountering '*') and release the lock when the table is committed (COMMIT). The "-w" option continues to apply. Note that support for -w in iptables[6]-restore has not been sent upstream yet, so this patch should be sent at the same time as that one. Bug: 32323979 Test: manual Signed-off-by: Narayan Kamath <narayan@google.com> Change-Id: I10094290eff834e076bb03d53e40eae9b96c1fae
* | iptables: flush stdout after every verbose log.Narayan Kamath2017-01-191-1/+3
| | | | | | | | | | | | | | | | | | Ensures that each logged line is flushed to stdout after it's written, and not held in any buffer. Test: manual Bug: 32323979 Change-Id: Iffe2bdc72582eef54843da41c65289faeeb4fe51
* | Add '-w' option to ip[6]tables-restoreYusuke Sato2015-08-211-3/+15
|/ | | | | | | | so ip[6]tables-restrore and ip[6]tables commands can be safely executed in parallel. Bug: 21725996 Change-Id: I4d0c0e5ff9e7881d9ebdfa5d4c733029703bb8de
* ip{6}tables-restore: fix breakage due to new locking approachPablo Neira Ayuso2013-07-081-1/+1
| | | | | | | | | | | | | | | | Since (93587a0 ip[6]tables: Add locking to prevent concurrent instances), ip{6}tables-restore does not work anymore: iptables-restore < x Another app is currently holding the xtables lock. Perhaps you want to use the -w option? do_command{6}(...) is called from ip{6}tables-restore for every iptables command contained in the rule-set file. Thus, hitting the lock error after the second command. Fix it by bypassing the locking in the ip{6}tables-restore path. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* ip[6]tables-restore: cleanup to reduce one level of indentationPablo Neira Ayuso2012-08-031-69/+65
| | | | | | | This patch moves the parameter parsing to one function to reduce one level of indentation. Jan Engelhardt likes this. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* iptables-restore: warn about -t in rule linesJan Engelhardt2012-07-311-2/+4
| | | | | | | save-restore syntax uses *table, not -t table. Signed-off-by: Jan Engelhardt <jengelh@inai.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* iptables-restore: fix parameter parsing (shows up with gcc-4.7)Pablo Neira Ayuso2012-07-301-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch fixes parameter parsing in iptables-restore since time ago. The problem has shown up with gcc-4.7. This version of gcc seem to perform more agressive memory management than previous. Peter Lekensteyn provided the following sample code similar to the one in iptables-restore: int i = 0; for (;;) { char x[5]; x[i] = '0' + i; if (++i == 4) { x[i] = '\0'; /* terminate string with null byte */ printf("%s\n", x); break; } } Many may expect 0123 as output. But GCC 4.7 does not do that when compiling with optimization enabled (-O1 and higher). It instead puts random data in the first bytes of the character array, which becomes: | 0 | 1 | 2 | 3 | 4 | | RANDOM | '3' | '\0' | Since the array is declared inside the scope of loop's body, you can think of it as of a new array being allocated in the automatic storage area for each loop iteration. The correct code should be: char x[5]; for (;;) { x[i] = '0' + i; if (++i == 4) { x[i] = '\0'; /* terminate string with null byte */ printf("%s\n", x); break; } } Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* Revert "iptables-restore: move code to add_param_to_argv, cleanup (fix gcc-4.7)"Pablo Neira Ayuso2012-07-301-65/+68
| | | | | | | | This reverts commit 44191bdbd71e685fba9eab864b9df25e63905220. Apply instead a patch that really clarifies the bug in iptables-restore. This should be good for the record (specifically, for distributors so they can find the fix by googling).
* iptables-restore: move code to add_param_to_argv, cleanup (fix gcc-4.7)Pablo Neira Ayuso2012-07-251-68/+65
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch seems to be a mere cleanup that moves the parameter parsing code to add_param_to_argv. But, in reality, it also fixes iptables when compiled with gcc-4.7. Moving param_buffer declaration out of the loop seems to resolve the issue. gcc-4.7 seems to be generating bad code regarding param_buffer. @@ -380,9 +380,9 @@ quote_open = 0; escaped = 0; param_len = 0; + char param_buffer[1024]; for (curchar = parsestart; *curchar; curchar++) { - char param_buffer[1024]; if (quote_open) { if (escaped) { But I have hard time to apply this patch in such a way. Instead, I came up with the idea of this cleanup, which does not harm after all (and fixes the issue for us). Someone in: https://bugzilla.redhat.com/show_bug.cgi?id=82579 put some light on this: "Yes, I ran into this too. The issue is that the gcc optimizer is optimizing out the code that collects quoted strings in iptables-restore.c at line 396. If inside a quotemark and it hasn't seen another one yet, it executes param_buffer[param_len++] = *curchar; continue; At -O1 or higher, the write to param_buffer[] never happens. It just increments param_len and continues. Moving the definition of char param_buffer[1024]; outside the loop fixes it. Why, I'm not sure. Defining the param_buffer[] inside the loop should simply restrict its scope to inside the loop." Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* ip(6)tables-restore: make sure argv is NULL terminatedFlorian Westphal2012-05-141-1/+1
| | | | | | | | | | Else, argv[argc] may point to free'd memory. Some extensions, e.g. rateest, may fail to parse valid input because argv[optind] (with optind == argc) is not NULL. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* ip6tables-restore: make code look alike with iptables-restoreJan Engelhardt2011-09-111-14/+15
| | | | Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
* src: resolve old macro names that are indirectionsJan Engelhardt2011-09-111-7/+6
| | | | | | | | | | | 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>
* libiptc: combine common types: _handleJan Engelhardt2011-09-111-3/+3
| | | | | | | No real API/ABI change incurred, since the definition of the structs' types is not visible anyhow. Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
* src: remove unused IPTABLES_MULTI defineJan Engelhardt2011-08-261-5/+0
| | | | | | This dead code has been lingering around since commit v1.4.5~7. Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
* iptables: Coverity: REVERSE_INULLJiri Popelka2011-06-221-2/+1
| | | | | | | | | | | | | | ip6tables-restore.c:186: deref_ptr_in_call: Dereferencing pointer "in". ip6tables-restore.c:463: check_after_deref: Dereferencing "in" before a null check. iptables-restore.c:192: deref_ptr_in_call: Dereferencing pointer "in". iptables-restore.c:468: check_after_deref: Dereferencing "in" before a null check. iptables-xml.c:671: deref_ptr_in_call: Dereferencing pointer "in". iptables-xml.c:873: check_after_deref: Dereferencing "in" before a null check. Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
* src: move all iptables pieces into a separate directoryJan Engelhardt2011-06-071-0/+471
(Unclutter top-level dir) Signed-off-by: Jan Engelhardt <jengelh@medozas.de>