| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
| |
zero length ranges now allowed, therefore, update error message to refer
to negative ranges which are not possible.
Fixes: 7a6e16040d65 ("evaluate: allow for zero length ranges")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
f8f32deda31d ("meta: Introduce new conditions 'time', 'day' and 'hour'")
reverses the hour range in case that a cross-day range is used, eg.
meta hour "03:00"-"14:00" counter accept
which results in (Sidney, Australia AEDT time):
meta hour != "14:00"-"03:00" counter accept
kernel handles time in UTC, therefore, cross-day range may not be
obvious according to local time.
The ruleset listing above is not very intuitive to the reader depending
on their timezone, therefore, complete netlink delinearize path to
reverse the cross-day meta range.
Update manpage to recommend to use a range expression when matching meta
hour range. Recommend range expression for meta time and meta day too.
Extend testcases/listing/meta_time to cover for this scenario.
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1737
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The included sample causes a crash because we attempt to
range-merge a prefix expression with a symbolic expression.
The first set is evaluated, the symbol expression evaluation fails
and nft queues an error message ("Could not resolve hostname").
However, nft continues evaluation.
nft then encounters the same set definition again and merges the
new content with the preceeding one.
But the first set structure is dodgy, it still contains the
unresolved symbolic expression.
That then makes nft crash (assert) in the set internals.
There are various different incarnations of this issue, but the low
level set processing code does not allow for any partially transformed
expressions to still remain.
Before:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
BUG: invalid range expression type binop
nft: src/expression.c:1479: range_expr_value_low: Assertion `0' failed.
After:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
invalid_range_expr_type_binop:4:18-25: Error: Could not resolve hostname: Name or service not known
elements = { 1&.141.0.1 - 192.168.0.2}
^^^^^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
| |
replace "‘" (UTF-8, 0xe280 0x98) with "'" (ASCII 0x27).
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
129f9d153279 ("nft: migrate man page examples with `meter` directive to
sets") already replaced meters by dynamic sets.
This patch removes NFT_SET_ANONYMOUS flag from the implicit set that is
instantiated via meter, so the listing shows a dynamic set instead which
is the recommended approach these days.
Therefore, a batch like this:
add table t
add chain t c
add rule t c tcp dport 80 meter m size 128 { ip saddr timeout 1s limit rate 10/second }
gets translated to a dynamic set:
table ip t {
set m {
type ipv4_addr
size 128
flags dynamic,timeout
}
chain c {
tcp dport 80 update @m { ip saddr timeout 1s limit rate 10/second burst 5 packets }
}
}
Check for NFT_SET_ANONYMOUS flag is also relaxed for list and flush
meter commands:
# nft list meter ip t m
table ip t {
set m {
type ipv4_addr
size 128
flags dynamic,timeout
}
}
# nft flush meter ip t m
As a side effect the legacy 'list meter' and 'flush meter' commands allow
to flush a dynamic set to retain backward compatibility.
This patch updates testcases/sets/0022type_selective_flush_0 and
testcases/sets/0038meter_list_0 as well as the json output which now
uses the dynamic set representation.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
AFL found following crash:
table ip filter {
map ipsec_in {
typeof ipsec in reqid . iif : verdict
flags interval
}
chain INPUT {
type filter hook input priority filter; policy drop;
ipsec in reqid . 100 @ipsec_in
}
}
Which yields:
nft: evaluate.c:1213: expr_evaluate_unary: Assertion `!expr_is_constant(arg)' failed.
All existing test cases with constant values use big endian values, but
"iif" expects host endian values.
As raw values were not supported before, concat byteorder conversion
doesn't handle constants.
Fix this:
1. Add constant handling so that the number is converted in-place,
without unary expression.
2. Add the inverse handling on delinearization for non-interval set
types.
When dissecting the concat data soup, watch for integer constants where
the datatype indicates host endian integer.
Last, extend an existing test case with the afl input to cover
in/output.
A new test case is added to test linearization, delinearization and
matching.
Based on original patch from Florian Westphal, patch subject and
description wrote by him.
Fixes: b422b07ab2f9 ("src: permit use of constant values in set lookup keys")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Add unary expression to trigger byteorder conversion for host byteorder
selectors only if selectors length is larger or equal than 2 bytes.
# cat test.nft
table ip x {
set test {
type ipv4_addr . ether_addr . inet_proto
flags interval
}
chain y {
ip saddr . ether saddr . meta l4proto @test counter
}
}
# nft -f test.nft
ip x y
[ meta load iiftype => reg 1 ]
[ cmp eq reg 1 0x00000001 ]
[ payload load 4b @ network header + 12 => reg 1 ]
[ payload load 6b @ link header + 6 => reg 9 ]
[ meta load l4proto => reg 11 ]
[ byteorder reg 11 = hton(reg 11, 2, 1) ] <--- should not be here
[ lookup reg 1 set test ]
[ counter pkts 0 bytes 0 ]
Fixes: 1017d323cafa ("src: support for selectors with different byteorder with interval concatenations")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Example:
nft --debug=all destroy table ip missingtable
Before:
Evaluate unknown
After:
Evaluate destroy
Fixes: e1dfd5cc4c46 ("src: add support to command "destroy"")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
before:
nft: evaluate.c:467: conflict_resolution_gen_dependency: Assertion `expr->payload.base == PROTO_BASE_LL_HDR' failed.
Aborted (core dumped)
conflict_resolution_gen_dependency() can only handle linklayer
conflicts, hence the assert.
Rename it accordingly. Also rename resolve_protocol_conflict, it doesn't
do anything for != PROTO_BASE_LL_HDR and extend the assertion to that
function too.
Callers now enforce PROTO_BASE_LL_HDR prerequisite.
after:
Error: conflicting transport layer protocols specified: comp vs. udp
ip6 nexthdr comp udp dport 4789
^^^^^^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
fixup
Else this gives:
nft: evaluate.c:2983: stmt_evaluate_payload: Assertion `sizeof(data) * BITS_PER_BYTE >= masklen' failed.
For loads, this is already prevented via expr_evaluate_bits() which has:
if (masklen > NFT_REG_SIZE * BITS_PER_BYTE)
return expr_error(ctx->msgs, expr, "mask length %u exceeds allowed maximum of %u\n",
masklen, NFT_REG_SIZE * BITS_PER_BYTE);
But for the store path this isn't called.
The reproducer asks to store a 128 bit integer at bit offset 1, i.e.
17 bytes would need to be munged, but we can only handle up to 16 bytes
(one pseudo-register).
Fixes: 78936d50f306 ("evaluate: add support to set IPv6 non-byte header fields")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Detected when running:
# nft -f tests/shell/testcases/bogons/nft-f/no_integer_basetype_crash
==383222==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x7fe7b54a9e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x7fe7b538b9a9 in __gmp_default_allocate (/lib/x86_64-linux-gnu/libgmp.so.10+0xc9a9)
Fixes: 3671c4897003 ("evaluate: guard against NULL basetype")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
datatype
Detected when running:
# nft -f tests/shell/testcases/bogons/nft-f/mapping_with_invalid_datatype_crash
==382584==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 144 byte(s) in 1 object(s) allocated from:
#0 0x7fde06ca9e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x7fde062924af in xmalloc src/utils.c:31
#2 0x7fde0629266c in xzalloc src/utils.c:70
#3 0x7fde06167299 in expr_alloc src/expression.c:46
#4 0x7fde0616b014 in constant_expr_alloc src/expression.c:420
#5 0x7fde06128e43 in expr_evaluate_map src/evaluate.c:2027
#6 0x7fde06137b06 in expr_evaluate src/evaluate.c:2891
#7 0x7fde06132417 in expr_evaluate_relational src/evaluate.c:2497
#8 0x7fde06137b36 in expr_evaluate src/evaluate.c:2895
#9 0x7fde06137d5f in stmt_evaluate_expr src/evaluate.c:2914
#10 0x7fde061524c8 in stmt_evaluate src/evaluate.c:4646
#11 0x7fde0615c9ee in rule_evaluate src/evaluate.c:5202
#12 0x7fde061600c7 in cmd_evaluate_add src/evaluate.c:5422
Fixes: 70054e6e1c87 ("evaluate: catch implicit map expressions without known datatype")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Iterate over the element list in the anonymous set to validate that all
expressions are concatenations, otherwise bail out.
ruleset.nft:3:46-53: Error: expression is not a concatenation
ip protocol . th dport vmap { tcp / 22 : accept, tcp . 80 : drop}
^^^^^^^^
This is based on a patch from Florian Westphal.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
components
If this is the last expression, then the runaway flag is set on and
evaluation bails in the next iteration, do not fetch next list element
which refers to the list head.
I found this by code inspection, I could not trigger any crash with this
one.
Fixes: ae1d54d1343f ("evaluate: do not crash on runaway number of concatenation components")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
| |
Concatenation is only supported with sets. Moreover, stripping of the
set leads to broken ruleset listing, therefore, skip this optimization
for the concatenations.
Fixes: fa17b17ea74a ("evaluate: revisit anonymous set with single element optimization")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
| |
Else we assert with:
BUG: unknown expression type range
nft: src/netlink_linearize.c:912: netlink_gen_expr: Assertion `0' failed.
While at it, condense meta and exthdr to reuse the same helper.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
Testing for range before evaluation will still crash us later during
netlink linearization, prefixes turn into ranges, symbolic expression
might hide a range/prefix.
So move this after the argument has been evaluated.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
add rule ip6 f i rt2 addr . ip6 daddr { dead:: . dead:: }
... will cause a segmentation fault, we assume expr->dtype is always
set.
rt2 support is incomplete, the template is uninitialised.
This could be fixed up, but rt2 (a subset of the deperecated type 0),
like all other routing headers, lacks correct dependency tracking.
Currently such routing headers are always assumed to be segment routing
headers, we would need to add dependency on 'Routing Type' field in the
routing header, similar to icmp type/code.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
netlink_linearize.c has never supported more than 16 chained binops.
Adding more is possible but overwrites the stack in
netlink_gen_bitwise().
Add a recursion counter to catch this at eval stage.
Its not enough to just abort once the counter hits
NFT_MAX_EXPR_RECURSION.
This is because there are valid test cases that exceed this.
For example, evaluation of 1 | 2 will merge the constans, so even
if there are a dozen recursive eval calls this will not end up
with large binop chain post-evaluation.
v2: allow more than 16 binops iff the evaluation function
did constant-merging.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
BUG: Value export of 512 bytes would overflownft: src/netlink.c:474: netlink_gen_prefix: Assertion `0' failed.
After:
66: Error: Object mapping data should be a value, not prefix
synproxy name ip saddr map { 192.168.1.0/24 : "v*" }
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Alternative would be to refactor this and move this into the parsers
(bison, json) instead of this hidden re-parsing.
Fixes: 627c451b2351 ("src: allow variables in the chain priority specification")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The kernel will reject this too, but unfortunately nft may try
to cram the data into the underlying libnftnl expr.
This causes heap corruption or
BUG: nld buffer overflow: want to copy 132, max 64
After:
Error: Concatenation of size 544 exceeds maximum size of 512
udp length . @th,0,512 . @th,512,512 { 47-63 . 0xe373135363130 . 0x33131303735353203 }
^^^^^^^^^
resp. same warning for an over-sized raw expression.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Else we get:
BUG: unknown expression type range
nft: src/netlink_linearize.c:909: netlink_gen_expr: Assertion `0' failed.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
This breaks existing behaviour, add a test case so this is caught in
the future.
The reverted test case will be brought back once a better fix
is available.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
Before:
nft: gmputil.c:77: mpz_get_uint8: Assertion `cnt <= 1' failed.
After: Error: reject code must be integer in range 0-255
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
BUG: invalid range expression type symbol
nft: expression.c:1494: range_expr_value_high: Assertion `0' failed.
After:
range_expr_value_high_assert:5:20-27: Error: Could not resolve protocol name
elements = { 100-11.0.0.0, }
^^^^^^^^
range_expr_value_high_assert:7:6-7: Error: set definition has conflicting key (ipv4_addr vs inet_proto)
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
nat_concat_map() requires a datamap, else we crash:
set->data is dereferenced.
Also update expr_evaluate_map() so that EXPR_SET_REF is checked there
too.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
| |
The assertion is too strict, as found by afl++:
typeof iifname . ip saddr . meta ipsec
elements = { "eth0" . 10.1.1.2 . 1 }
meta ipsec is boolean (1 bit), but datasize of 1 is set at 8 bit.
Fixes: 22b750aa6dc9 ("src: allow use of base integer types as set keys in concatenations")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
The includes test files cause:
BUG: chain is too large (257, 256 max)nft: netlink.c:418: netlink_gen_chain: Assertion `0' failed.
Error out in evaluation step instead.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch consolidates ctx->stmt_len reset in stmt_evaluate() to avoid
this problem. Note that stmt_evaluate_meta() and stmt_evaluate_ct()
already reset it after the statement evaluation.
Moreover, statement dependency can be generated while evaluating a meta
and ct statement. Payload statement dependency already manually stashes
this before calling stmt_evaluate(). Add a new stmt_dependency_evaluate()
function to stash statement length context when evaluating a new statement
dependency and use it for all of the existing statement dependencies.
Florian also says:
'meta mark set vlan id map { 1 : 0x00000001, 4095 : 0x00004095 }' will
crash. Reason is that the l2 dependency generated here is errounously
expanded to a 32bit-one, so the evaluation path won't recognize this
as a L2 dependency. Therefore, pctx->stacked_ll_count is 0 and
__expr_evaluate_payload() crashes with a null deref when
dereferencing pctx->stacked_ll[0].
nft-test.py gains a fugly hack to tolerate '!map typeof vlan id : meta mark'.
For more generic support we should find something more acceptable, e.g.
!map typeof( everything here is a key or data ) timeout ...
tests/py update and assert(pctx->stacked_ll_count) by Florian Westphal.
Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
| |
tests/shell/testcases/bogons/nft-f/set_definition_with_no_key_assert
BUG: unhandled key type 2
nft: src/intervals.c:59: setelem_expr_to_range: Assertion `0' failed.
This patch adds a new unit tests/shell courtesy of Florian Westphal.
Fixes: 3975430b12d9 ("src: expand table command before evaluation")
Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
| |
We release ->dtype twice, will either segfault or assert
on dtype->refcount != 0 check in datatype_free().
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
mapping_With_invalid_datatype_crash:1:8-65: Error: Implicit map expression without known datatype
bla to tcp dport map { 80 : 1.1.1.1 . 8001, 81 : 2.2.2.2 . 9001 } bla
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
| |
This will crash as set->data is NULL, so check that SET_REF is pointing
to a map:
Error: candidates_ipv4 is not a map
tcp dport 10003 ip saddr . tcp dport @candidates_ipv4 add @candidates_ipv4 { ip saddr . 10 :0004 timeout 1s }
~~~~~~~~~~~~~~~~
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
... this will cause an assertion in netlink linearization, catch this
at eval stage instead.
before:
BUG: unknown expression type range
nft: netlink_linearize.c:908: netlink_gen_expr: Assertion `0' failed.
after:
/unknown_expr_type_range_assert:3:31-40: Error: Meta expression cannot be a range
meta mark set 0x001-3434
^^^^^^^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
| |
prefer
binop_with_different_basetype_assert:3:29-35: Error: Binary operation (<<) with different base types (string vs integer) is not supported
oifname set ip9dscp << 26 | 0x10
^^^^^^^~~~~~~
to assertion failure.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
| |
i->dtype->basetype can be NULL.
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
BUG: invalid mapping expression binop
nft: src/evaluate.c:2027: expr_evaluate_map: Assertion `0' failed.
After:
tests/shell/testcases/bogons/nft-f/invalid_mapping_expr_binop_assert:1:22-25: Error: invalid mapping expression binop
xy mame ip saddr map h& p p
~~~~~~~~ ^^^^
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
| |
large '& VAL' results in:
src/evaluate.c:531: expr_evaluate_bits: Assertion `masklen <= NFT_REG_SIZE * BITS_PER_BYTE' failed.
Turn this into expr_error().
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
| |
Error out instead of 'nft: gmputil.c:67: mpz_get_uint32: Assertion `cnt <= 1' failed.'.
Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
nft --check -f tests/shell/testcases/bogons/nft-f/set_without_key
Segmentation fault (core dumped)
Fixes: 56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
| |
When allocating a unary expression, clone the datatype to deal with
dynamic datatypes.
Fixes: 6b01bb9ff798 ("datatype: concat expression only releases dynamically allocated datatype")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Bail out if flowtable declaration is missing and no devices are
specified.
Otherwise, this reports a bogus error when adding new devices to an
existing flowtable.
# nft -v
nftables v1.0.9 (Old Doc Yak #3)
# ip link add dummy1 type dummy
# ip link set dummy1 up
# nft 'create flowtable inet filter f1 { hook ingress priority 0; counter }'
# nft 'add flowtable inet filter f1 { devices = { dummy1 } ; }'
Error: missing hook and priority in flowtable declaration
add flowtable inet filter f1 { devices = { dummy1 } ; }
^^^^^^^^^^^^^^^^^^^^^^^^
Fixes: 5ad475fce5a1 ("evaluate: bail out if new flowtable does not specify hook and priority")
Reported-by: Martin Gignac <martin.gignac@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
| |
nft replace rule t c handle 3 'jhash ip protocol . ip saddr mod 170 vmap { 0-94 : goto wan1, 95-169 : goto wan2, 170-269 }"'
BUG: unhandled op 2
nft: src/evaluate.c:1748: interval_set_eval: Assertion `0' failed.
Fixes: 81e36530fcac ("src: replace interval segment tree overlap and automerge")
Reported-by: Tino Reichardt <milky-netfilter@mcmilk.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
xmalloc() (and similar x-functions) are used for allocation. They wrap
malloc()/realloc() but will abort the program on ENOMEM.
The meaning of xmalloc() is that it wraps malloc() but aborts on
failure. I don't think x-functions should have the notion, that this
were potentially a different memory allocator that must be paired
with a particular xfree().
Even if the original intent was that the allocator is abstracted (and
possibly not backed by standard malloc()/free()), then that doesn't seem
a good idea. Nowadays libc allocators are pretty good, and we would need
a very special use cases to switch to something else. In other words,
it will never happen that xmalloc() is not backed by malloc().
Also there were a few places, where a xmalloc() was already "wrongly"
paired with free() (for example, iface_cache_release(), exit_cookie(),
nft_run_cmd_from_buffer()).
Or note how pid2name() returns an allocated string from fscanf(), which
needs to be freed with free() (and not xfree()). This requirement
bubbles up the callers portid2name() and name_by_portid(). This case was
actually handled correctly and the buffer was freed with free(). But it
shows that mixing different allocators is cumbersome to get right. Of
course, we don't actually have different allocators and whether to use
free() or xfree() makes no different. The point is that xfree() serves
no actual purpose except raising irrelevant questions about whether
x-functions are correctly paired with xfree().
Note that xfree() also used to accept const pointers. It is bad to
unconditionally for all deallocations. Instead prefer to use plain
free(). To free a const pointer use free_const() which obviously wraps
free, as indicated by the name.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Almost everywhere xmalloc() and friends is used instead of malloc().
This is almost everywhere paired with xfree().
xfree() has two problems. First, it brings the wrong notion that
xmalloc() should be paired with xfree(), as if xmalloc() would not use
the plain malloc() allocator. In practices, xfree() just wraps free(),
and it wouldn't make sense any other way. xfree() should go away. This
will be addressed in the next commit.
The problem addressed by this commit is that xfree() accepts a const
pointer. Paired with the practice of almost always using xfree() instead
of free(), all our calls to xfree() cast away constness of the pointer,
regardless whether that is necessary. Declaring a pointer as const
should help us to catch wrong uses. If the xfree() function always casts
aways const, the compiler doesn't help.
There are many places that rightly cast away const during free. But not
all of them. Add a free_const() macro, which is like free(), but accepts
const pointers. We should always make an intentional choice whether to
use free() or free_const(). Having a free_const() macro makes this very
common choice clearer, instead of adding a (void*) cast at many places.
Note that we now pair xmalloc() allocations with a free() call (instead
of xfree(). That inconsistency will be resolved in the next commit.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
mpz_get_str() (with NULL as first argument) will allocate a buffer using
the allocator functions (mp_set_memory_functions()). We should free
those buffers with the corresponding free function.
Add nft_gmp_free() for that and use it.
The name nft_gmp_free() is chosen because "mini-gmp.c" already has an
internal define called gmp_free(). There wouldn't be a direct conflict,
but using the same name is confusing. And maybe our own defines should
have a clear nft prefix.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use the key from the evaluation context to perform the byteorder
conversion in case that this expression is used for lookups and updates
on explicit sets.
# nft --debug=netlink add rule ip6 t output ip6 dscp @mapv6
ip6 t output
[ payload load 2b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
[ byteorder reg 1 = ntoh(reg 1, 2, 2) ] <-------------- this was missing!
[ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
[ lookup reg 1 set mapv6 ]
Also with set statements (updates from packet path):
# nft --debug=netlink add rule ip6 t output update @mapv6 { ip6 dscp }
ip6 t output
[ payload load 2b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
[ byteorder reg 1 = ntoh(reg 1, 2, 2) ] <------------- also here!
[ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
[ dynset update reg_key 1 set mapv6 ]
Simple matches on values and implicit sets rely on the binary transfer
mechanism to propagate the shift to the constant, no explicit byteorder
is required in such case.
Fixes: 668c18f67203 ("evaluate: place byteorder conversion before rshift in payload statement")
Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
map expression (which is used a key to look up for the mapping) needs to
consider the statement length context, otherwise incorrect bytecode is
generated when {ct,meta} statement is generated.
# nft -f - <<EOF
add table ip6 t
add chain ip6 t c
add map ip6 t mapv6 { typeof ip6 dscp : meta mark; }
EOF
# nft -d netlink add rule ip6 t c meta mark set ip6 dscp map @mapv6
ip6 t c
[ payload load 2b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
... missing byteorder conversion here before shift ...
[ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
[ lookup reg 1 set mapv6 dreg 1 ]
[ meta set mark with reg 1 ]
Reset statement length context only for the mapping side for the
elements in the set.
Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand")
Reported-by: Brian Davidson <davidson.brian@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Consider the following ruleset.
define ext_if = { "eth0", "eth1" }
table ip filter {
chain c {
iifname . tcp dport { $ext_if . 22 } accept
}
}
Attempting to load this ruleset results in:
BUG: invalid expression type 'set' in setnft: netlink.c:304: __netlink_gen_concat_key: Assertion `0' failed.
Aborted (core dumped)
After this patch:
# nft -f ruleset.nft
ruleset.nft:1:17-40: Error: cannot use set in concatenation
define ext_if = { "eth0", "eth1" }
^^^^^^^^^^^^^^^^^^
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1715
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|