| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Wrap every call to $XT_MULTI with valgrind, or actually a wrapper script
which does the valgrind wrap and stores the log if it contains something
relevant.
Carefully name the wrapper script(s) so that test cases' checks on
$XT_MULTI name stay intact.
This mode slows down testsuite execution horribly. Luckily, it's not
meant for constant use, though.
For now, ignore commands with non-zero exit status - error paths
typically hit direct exit() calls and therefore leave reachable memory
in place.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
| |
When restoring a dump which contains an explicit flush command,
previously added rules are removed from cache and the following commit
will try to create netlink messages based on freed memory.
Fix this by weeding any rule-based commands from obj_list if they
address the same chain.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When flushing all chains and verbose mode is not enabled,
nft_rule_flush() uses a shortcut: It doesn't specify a chain name for
NFT_MSG_DELRULE, so the kernel will flush all existing chains without
user space needing to know which they are.
The above allows to avoid a chain cache, but there's a caveat:
nft_xt_builtin_init() will create base chains as it assumes they are
missing and thereby possibly overrides any non-default chain policies.
Solve this by making nft_xt_builtin_init() cache-aware: If a command
doesn't need a chain cache, there's no need to bother with creating any
non-existing builtin chains, either. For the sake of completeness, also
do nothing if cache is not initialized (although that shouldn't happen).
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Iterate over command list and collect chains to cache. Insert them into
a sorted list to pass to __nft_build_cache().
If a command is interested in all chains (e.g., --list), cmd->chain
remains unset. To record this case reliably, use a boolean
('all_chains'). Otherwise, it is hard to distinguish between first call
to nft_cache_level_set() and previous command with NULL cmd->chain
value.
When caching only specific chains, manually add builtin ones for the
given table as well - otherwise nft_xt_builtin_init() will act as if
they don't exist and possibly override non-default chain policies.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
Extract the inner part of fetch_chain_cache() into a dedicated function,
preparing for individual chain caching.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Restore per-table operation of cache routines as initially implemented
in commit e2883c5531e6e ("nft-cache: Support partial cache per table").
As before, this doesn't limit fetching of tables (their number is
supposed to be low) but instead limits fetching of sets, chains and
rules to the specified table.
For this to behave correctly when restoring without flushing over
multiple tables, cache must be freed fully after each commit - otherwise
the previous table's cache level is reused for the current one. The
exception being fake cache, used for flushing restore: NFT_CL_FAKE is
set just once at program startup, so it must stay set otherwise
consecutive tables cause pointless cache fetching.
The sole use-case requiring a multi-table cache, iptables-save, is
indicated by req->table being NULL. Therefore, req->table assignment is
a bit sloppy: All calls to nft_cache_level_set() are assumed to set the
same table value, collision detection exists merely to catch programming
mistakes.
Make nft_fini() call nft_release_cache() instead of flush_chain_cache(),
the former does a full cache deinit including cache_req contents.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
| |
This embedded struct collects cache requirement info gathered from parsed
nft_cmds and is interpreted by __nft_build_cache().
While being at it, remove unused parameters passed to the latter
function, nft_handle pointer is sufficient.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
With NFT_CL_FAKE being highest cache level while at the same time
__nft_build_cache() treating it equal to NFT_CL_TABLES, no special
handling for fake cache is required anymore.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since no incremental cache fetching happens anymore, code fetching rules
for chains or elements for sets may safely assume that whatever is in
cache also didn't get populated with rules or elements before.
Therefore no (optional) chain name needs to be passed on to
fetch_rule_cache() and fetch_set_cache() doesn't have to select for
which sets in a table to call set_fetch_elem_cb().
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
| |
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
| |
The cache requirements are now calculated once from the parsing phase.
There is no need to call __nft_build_cache() from several spots in the
codepath anymore.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
Update among support to work again with the new parser and cache logic.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch uses the new list of commands to calculate the cache
requirements, the rationale after this updates is the following:
#1 Parsing, that builds the list of commands and it also calculates
cache level requirements.
#2 Cache building.
#3 Translate commands to jobs
#4 Translate jobs to netlink
This patch removes the pre-parsing code in xtables-restore.c to
calculate the cache.
After this patch, cache is calculated only once, there is no need
to cancel and refetch for an in-transit transaction.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch updates the parser to generate a list of command objects.
This list of commands is then transformed to a list of netlink jobs.
This new command object stores the rule using the nftnl representation
via nft_rule_new().
To reduce the number of updates in this patch, the nft_*_rule_find()
functions have been updated to restore the native representation to
skip the update of the rule comparison code.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Cache code is suited for holding multiple tables' data at once. The only
users of that are xtables-save and ebtables-restore with its support for
multiple tables and lack of explicit COMMIT lines.
Remove the second user by introducing implicit commits upon table line
parsing. This would allow to make cache single table only, but then
xtables-save would fetch cache multiple times (once for each table) and
therefore lose atomicity with regards to the acquired kernel ruleset
image.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
Kernel accepts a table name when dumping sets, so make use of that in
case a table was passed to fetch_set_cache() but no set name.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
This simplifies code a bit and also aligns set and chain lists handling
in cache.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
The function is always called immediately after fetch_table_cache(), so
merge it into the latter.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
At least since flushing xtables-restore doesn't fetch chains from kernel
anymore, problems with pending policy rule delete jobs can't happen
anymore.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Ensures that each logged line is flushed to stdout after it's
written, and not held in any buffer.
Places to modify found via:
git grep -C5 'fputs[(]buffer, stdout[)];'
On Android iptables-restore -v is run as netd daemon's child process
and fed actions via pipe. '#PING' is used to verify the child
is still responsive, and thus needs to be unbuffered.
Luckily if you're running iptables-restore in verbose mode you
probably either don't care about performance or - like Android
- actually need this.
Test: builds, required on Android for ip6?tables-restore netd
subprocess health monitoring.
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
This is present in bionic header files regardless of compiler
being used (likely clang)
Test: builds
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
| |
Problem is fixed since commit c550c81fd373e ("nft: cache: Fix
nft_release_cache() under stress"), looks like another case of
use-after-free.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
| |
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This fixes clang compiler warnings:
iptables/xshared.h:176:50: error: declaration of 'struct timeval' will not be visible outside of this function [-Werror,-Wvisibility]
extern int xtables_lock_or_exit(int wait, struct timeval *tv);
^
iptables/xshared.h:179:57: error: declaration of 'struct timeval' will not be visible outside of this function [-Werror,-Wvisibility]
void parse_wait_interval(int argc, char *argv[], struct timeval *wait_interval);
^
Test: builds with less warnings
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This resolves clang compiler warnings:
extensions/libext4_srcs/gen/gensrcs/external/iptables/extensions/libipt_ULOG.c:89:32: error: implicit declaration of function 'ffs' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
printf(" --ulog-nlgroup %d", ffs(loginfo->nl_group));
^
extensions/libext4_srcs/gen/gensrcs/external/iptables/extensions/libipt_ULOG.c:105:9: error: implicit declaration of function 'ffs' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
ffs(loginfo->nl_group));
^
extensions/libext_srcs/gen/gensrcs/external/iptables/extensions/libxt_addrtype.c:263:14: error: implicit declaration of function 'ffs' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
int first = ffs(val);
^
Test: builds with less warnings
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
While not really useful, iptables-nft-restore shouldn't segfault either.
This tests the problem described in nfbz#1407.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
| |
Add a second table to dump/restore. This triggers failures after
reverting c550c81fd373e ("nft: cache: Fix nft_release_cache() under
stress"), hence acts as a reproducer for the bug fixed by that commit as
well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
Instead of reading from stdin, pass dump file as regular parameter. This
way dump file name occurs in 'bash -x' output which helps finding out
where things fail.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
| |
This must be a leftover from a previous cleanup.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Introduce "--alarm" option for idletimer rule.
If it is present, hardidle-timer is used, else default timer.
The default idletimer starts a deferrable timer or in other
words the timer will cease to run when cpu is in suspended
state. This change introduces the option to start a
non-deferrable or alarm timer which will continue to run even
when the cpu is in suspended state.
Signed-off-by: Manoj Basapathi <manojbm@codeaurora.org>
Signed-off-by: Sauvik Saha <ssaha@codeaurora.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
The command_jump() function leaves cs->target unset if the target is not
found. Let's check if the jumpto string mismatches only in this case.
https://bugzilla.netfilter.org/show_bug.cgi?id=1422
Tested-by: Etienne Champetier <etienne.champetier@anevia.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
| |
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Adjust the mode eBPF programs are opened in so 0400 pinned bpf programs
work without requiring CAP_DAC_OVERRIDE.
This matches Linux 5.2's:
commit e547ff3f803e779a3898f1f48447b29f43c54085
Author: Chenbo Feng <fengc@google.com>
Date: Tue May 14 19:42:57 2019 -0700
bpf: relax inode permission check for retrieving bpf program
For iptable module to load a bpf program from a pinned location, it
only retrieve a loaded program and cannot change the program content so
requiring a write permission for it might not be necessary.
Also when adding or removing an unrelated iptable rule, it might need to
flush and reload the xt_bpf related rules as well and triggers the inode
permission check. It might be better to remove the write premission
check for the inode so we won't need to grant write access to all the
processes that flush and restore iptables rules.
kernel/bpf/inode.c:
- int ret = inode_permission(inode, MAY_READ | MAY_WRITE);
+ int ret = inode_permission(inode, MAY_READ);
In practice, AFAICT, the xt_bpf match .fd field isn't even used by new
kernels, but I believe it might be needed for compatibility with old ones
(though I'm pretty sure table modifications on them will outright fail).
Test: builds, passes Android test suite (albeit on an older iptables base),
git grep bpf_obj_get - finds no other users
Cc: Chenbo Feng <fengc@google.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Willem de Bruijn <willemb@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If kernel ruleset is constantly changing, code called by
nft_is_table_compatible() may crash: For each item in table's chain
list, nft_is_chain_compatible() is called. This in turn calls
nft_build_cache() to fetch chain's rules. Though if kernel genid has changed
meanwhile, cache is flushed and rebuilt from scratch, thereby freeing
table's chain list - the foreach loop in nft_is_table_compatible() then
operates on freed memory.
A simple reproducer (may need a few calls):
| RULESET='*filter
| :INPUT ACCEPT [10517:1483527]
| :FORWARD ACCEPT [0:0]
| :OUTPUT ACCEPT [1714:105671]
| COMMIT
| '
|
| for ((i = 0; i < 100; i++)); do
| iptables-nft-restore <<< "$RULESET" &
| done &
| iptables-nft-save
To fix the problem, basically revert commit ab1cd3b510fa5 ("nft: ensure
cache consistency") so that __nft_build_cache() no longer flushes the
cache. Instead just record kernel's genid when fetching for the first
time. If kernel rule set changes until the changes are committed, the
commit simply fails and local cache is being rebuilt.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
| |
Loop index variable was left in place after removing the loops.
Fixes: 39ec645093baa ("nft: cache: Simplify chain list allocation")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
| |
While fixing for iptables-nft-restore under stress, I managed to hit
NULL-pointer deref in flush_cache(). Given that nftnl_*_list_free()
functions are not NULL-pointer tolerant, better make sure such are not
passed by accident.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
| |
Allocate chain lists right after fetching table cache, regardless of
whether partial cache is fetched or not. Chain list pointers reside in
struct nft_cache's table array and hence are present irrespective of
actual tables in kernel. Given the small number of tables, there wasn't
much overhead avoided by the conditional in fetch_chain_cache().
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
| |
If transaction needed a refresh in nft_action(), restore with flush
would fetch a full cache instead of merely refreshing table list
contained in "fake" cache.
To fix this, nft_rebuild_cache() must distinguish between fake cache and
full rule cache. Therefore introduce NFT_CL_FAKE to be distinguished
from NFT_CL_RULES.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
iptables-nft-restore calls nft_action(h, NFT_COMPAT_COMMIT) for each
COMMIT line in input. When restoring a dump containing multiple large
tables, chances are nft_rebuild_cache() has to run multiple times.
If the above happens, consecutive table contents are added to __cache[1]
which nft_rebuild_cache() then frees, so next commit attempt accesses
invalid memory.
Fix this by making nft_release_cache() (called after each successful
commit) return things into pre-rebuild state again, but keeping the
fresh cache copy.
Fixes: f6ad231d698c7 ("nft: keep original cache in case of ERESTART")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Existing code is a bit quirky: If no connlabel.conf was found, the local
function connlabel_value_parse() is called which tries to interpret
given label as a number. If the config exists though,
nfct_labelmap_get_bit() is called instead which doesn't care about
"undefined" connlabel names. So unless installed connlabel.conf contains
entries for all possible numeric labels, rules added by users may stop
working if a connlabel.conf is created.
Related man page snippet states: "Using a number always overrides
connlabel.conf", so try numeric parsing and fall back to nfct only if
that failed.
Fixes: 51340f7b6a110 ("extensions: libxt_connlabel: use libnetfilter_conntrack")
Fixes: 3a3bb480a738a ("extensions: connlabel: Fallback on missing connlabel.conf")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Move common code into nft_init(), such as:
* initial zeroing nft_handle fields
* family ops lookup and assignment to 'ops' field
* setting of 'family' field
This requires minor adjustments in xtables_restore_main() so extra field
initialization doesn't happen before nft_init() call.
As a side-effect, this fixes segfaulting xtables-monitor binary when
printing rules for trace event as in that code-path 'ops' field wasn't
initialized.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
| |
Legacy tools don't support those options, either.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
| |
Legacy iptables doesn't accept -4 or -6 if they don't match the
symlink's native family. The only exception to that is iptables-restore
which simply ignores the lines introduced by non-matching options, which
is useful to create combined dump files for feeding into both
iptables-restore and ip6tables-restore.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
| |
In some cases, the script still called repo binaries. Avoid this when in
--host mode to allow testing without the need to compile sources in
beforehand.
Fixes: 1b5d762c1865e ("iptables-test: Support testing host binaries")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
| |
No need to set 'i' to zero here, it is not used before the next
assignment.
Fixes: 77e6a93d5c9dc ("xtables: add and set "implict" flag on transaction objects")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Powered by Stefano's support for concatenated ranges, a full among match
replacement can be implemented. The trick is to add MAC-only elements as
a concatenation of MAC and zero-length prefix, i.e. a range from
0.0.0.0 till 255.255.255.255.
Although not quite needed, detection of pure MAC-only matches is left in
place. For those, no implicit 'meta protocol' match is added (which is
required otherwise at least to keep nft output correct) and no concat
type is used for the set.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
| |
In legacy iptables, only the last plus sign remains special, any
previous ones are taken literally. Therefore xtables-translate must not
replace all of them with asterisk but just the last one.
Fixes: e179e87a1179e ("xtables-translate: Fix for interface name corner-cases")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
| |
When testing host binaries, XT_MULTI variable contains just the program
name without path component which most skip checks didn't expect. Fix
them, and while being at it also reduce indenting level in two scripts
by moving the skip check up front with an early exit call.
Fixes: 416898e335322 ("tests/shell: Support testing host binaries")
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Lookahead buffer used for cache requirements estimate in restore
--noflush separates individual lines with nul-chars. Two consecutive
nul-chars are interpreted as end of buffer and remaining buffer content
is skipped.
Sadly, reading an empty line (i.e., one containing a newline character
only) caused double nul-chars to appear in buffer as well, leading to
premature stop when reading cached lines from buffer.
To fix that, make use of xtables_restore_parse_line() skipping empty
lines without calling strtok() and just leave the newline character in
place. A more intuitive approach, namely skipping empty lines while
buffering, is deliberately not chosen as that would cause wrong values
in 'line' variable.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1400
Fixes: 09cb517949e69 ("xtables-restore: Improve performance of --noflush operation")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Arturo Borrero Gonzalez <arturo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There are two special situations xlate_ifname() didn't cover for:
* Interface name containing '*': This went unchanged, creating a command
nft wouldn't accept. Instead translate into '\*' which doesn't change
semantics.
* Interface name being '+': Can't translate into nft wildcard character
as nft doesn't accept asterisk-only interface names. Instead decide
what to do based on 'invert' value: Skip match creation if false,
match against an invalid interface name if true.
Also add a test to make sure future changes to this behaviour are
noticed.
Signed-off-by: Phil Sutter <phil@nwl.cc>
|