summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* tests: add missing json arp operation outputFlorian Westphal2019-06-181-0/+21
| | | | Signed-off-by: Florian Westphal <fw@strlen.de>
* src: add cache level flagsPablo Neira Ayuso2019-06-178-107/+135
| | | | | | | | | | | | | The score approach based on command type is confusing. This patch introduces cache level flags, each flag specifies what kind of object type is needed. These flags are set on/off depending on the list of commands coming in this batch. cache_is_complete() now checks if the cache contains the objects that are needed through these new flags. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* netlink: remove netlink_list_table()Pablo Neira Ayuso2019-06-173-8/+3
| | | | | | Remove this wrapper, call netlink_list_rules() instead. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* rule: skip cache population from do_command_monitor()Pablo Neira Ayuso2019-06-171-32/+0
| | | | | | | | nft_evaluate() already populates the cache before running the monitor command. Remove this code. Fixes: 7df42800cf89 ("src: single cache_update() call to build cache before evaluation") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests: shell: cannot use handle for non-existing rule in kernelPablo Neira Ayuso2019-06-171-1/+1
| | | | | | Do not guess handle for an unexisting rule in the kernel. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: remove useless parameter from cache_flush()Pablo Neira Ayuso2019-06-173-4/+3
| | | | | | Command type is never used in cache_flush(). Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* datatype: fix print of raw numerical symbol valuesFlorian Westphal2019-06-174-11/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The two rules: arp operation 1-2 accept arp operation 256-512 accept are both shown as 256-512: chain in_public { arp operation 256-512 accept arp operation 256-512 accept meta mark "1" tcp flags 2,4 } This is because range expression enforces numeric output, yet nft_print doesn't respect byte order. Behave as if we had no symbol in the first place and call the base type print function instead. This means we now respect format specifier as well: chain in_public { arp operation 1-2 accept arp operation 256-512 accept meta mark 0x00000001 tcp flags 0x2,0x4 } Without fix, added test case will fail: 'add rule arp test-arp input arp operation 1-2': 'arp operation 1-2' mismatches 'arp operation 256-512' v2: in case of -n, also elide quotation marks, just as if we would not have found a symbolic name. Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
* cache: do not populate the cache in case of flush ruleset commandPablo Neira Ayuso2019-06-143-0/+7
| | | | | | | | __CMD_FLUSH_RULESET is a dummy definition that used to skip the netlink dump to populate the cache. This patch is a workaround until we have a better infrastructure to track the state of the cache objects. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: double datatype_free() with dynamic integer datatypesPablo Neira Ayuso2019-06-143-8/+0
| | | | | | datatype_set() already deals with this case, remove this. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: update byteorder only for implicit mapsPablo Neira Ayuso2019-06-141-1/+2
| | | | | | | | The byteorder adjustment for the integer datatype is only required by implicit maps. Fixes: b9b6092304ae ("evaluate: store byteorder for set keys") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: use-after-free in meterPablo Neira Ayuso2019-06-131-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Similar to bbe139fdf5a5 ("evaluate: use-after-free in implicit set"). ==12727== Invalid read of size 4 ==12727== at 0x72DB515: expr_free (expression.c:86) ==12727== by 0x72D3092: set_free (rule.c:367) ==12727== by 0x72DB555: expr_destroy (expression.c:79) ==12727== by 0x72DB555: expr_free (expression.c:95) ==12727== by 0x72D7A35: meter_stmt_destroy (statement.c:137) ==12727== by 0x72D7A07: stmt_free (statement.c:50) ==12727== by 0x72D7AD7: stmt_list_free (statement.c:60) ==12727== by 0x72D32EF: rule_free (rule.c:610) ==12727== by 0x72D3834: chain_free (rule.c:827) ==12727== by 0x72D45D4: table_free (rule.c:1184) ==12727== by 0x72D46A7: __cache_flush (rule.c:293) ==12727== by 0x72D472C: cache_release (rule.c:313) ==12727== by 0x72D4A79: cache_update (rule.c:264) ==12727== Address 0x64f14c8 is 56 bytes inside a block of size 128 free'd ==12727== at 0x4C2CDDB: free (vg_replace_malloc.c:530) ==12727== by 0x72D7A2C: meter_stmt_destroy (statement.c:136) ==12727== by 0x72D7A07: stmt_free (statement.c:50) ==12727== by 0x72D7AD7: stmt_list_free (statement.c:60) ==12727== by 0x72D32EF: rule_free (rule.c:610) ==12727== by 0x72D3834: chain_free (rule.c:827) ==12727== by 0x72D45D4: table_free (rule.c:1184) ==12727== by 0x72D46A7: __cache_flush (rule.c:293) ==12727== by 0x72D472C: cache_release (rule.c:313) ==12727== by 0x72D4A79: cache_update (rule.c:264) ==12727== by 0x72F82CE: nft_evaluate (libnftables.c:388) ==12727== by 0x72F8A8B: nft_run_cmd_from_buffer (libnftables.c:428) Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* monitor: Accept -j flagPhil Sutter2019-06-132-7/+5
| | | | | | | | | | | | Make 'nft -j monitor' equal to 'nft monitor json' and change documentation to use only the first variant since that is more intuitive and also consistent with other commands. While being at it, drop references to XML from monitor section - it was never supported. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* netlink_delinearize: use-after-free in expr_postprocess_string()Pablo Neira Ayuso2019-06-131-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | escaped_string_wildcard_expr_alloc() may reallocate the expression. valgrind reports errors like this one: ==29945== Invalid write of size 4 ==29945== at 0x72EE944: __expr_postprocess_string (netlink_delinearize.c:2006) ==29945== by 0x72EE944: expr_postprocess_string (netlink_delinearize.c:2016) ==29945== by 0x72EE944: expr_postprocess (netlink_delinearize.c:2120) ==29945== by 0x72EE5A7: expr_postprocess (netlink_delinearize.c:2094) ==29945== by 0x72EF23B: stmt_expr_postprocess (netlink_delinearize.c:2289) ==29945== by 0x72EF23B: rule_parse_postprocess (netlink_delinearize.c:2510) ==29945== by 0x72EF23B: netlink_delinearize_rule (netlink_delinearize.c:2650) ==29945== by 0x72E6F63: list_rule_cb (netlink.c:330) ==29945== by 0x7770BD3: nftnl_rule_list_foreach (rule.c:810) ==29945== by 0x72E739E: netlink_list_rules (netlink.c:349) ==29945== by 0x72E739E: netlink_list_table (netlink.c:490) ==29945== by 0x72D4A89: cache_init_objects (rule.c:194) ==29945== by 0x72D4A89: cache_init (rule.c:216) ==29945== by 0x72D4A89: cache_update (rule.c:266) ==29945== by 0x72F829E: nft_evaluate (libnftables.c:388) ==29945== by 0x72F8A5B: nft_run_cmd_from_buffer (libnftables.c:428) Remove expr->len, not needed and it triggers use-after-free errors. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* datatype: dtype_clone() should clone flags tooPablo Neira Ayuso2019-06-131-1/+1
| | | | | | Clone original flags too. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: add reference counter for dynamic datatypesPablo Neira Ayuso2019-06-1314-65/+96
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are two datatypes are using runtime datatype allocation: * Concatenations. * Integer, that require byteorder adjustment. From the evaluation / postprocess step, transformations are common, hence expressions may end up fetching (infering) datatypes from an existing one. This patch adds a reference counter to release the dynamic datatype object when it is shared. The API includes the following helper functions: * datatype_set(expr, datatype), to assign a datatype to an expression. This helper already deals with reference counting for dynamic datatypes. This also drops the reference counter of any previous datatype (to deal with the datatype replacement case). * datatype_get(datatype) bumps the reference counter. This function also deals with nul-pointers, that occurs when the datatype is unset. * datatype_free() drops the reference counter, and it also releases the datatype if there are not more clients of it. Rule of thumb is: The reference counter of any newly allocated datatype is set to zero. This patch also updates every spot to use datatype_set() for non-dynamic datatypes, for consistency. In this case, the helper just makes an simple assignment. Note that expr_alloc() has been updated to call datatype_get() on the datatype that is assigned to this new expression. Moreover, expr_free() calls datatype_free(). This fixes valgrind reports like this one: ==28352== 1,350 (440 direct, 910 indirect) bytes in 5 blocks are definitely lost in loss recor 3 of 3 ==28352== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==28352== by 0x4E79558: xmalloc (utils.c:36) ==28352== by 0x4E7963D: xzalloc (utils.c:65) ==28352== by 0x4E6029B: dtype_alloc (datatype.c:1073) ==28352== by 0x4E6029B: concat_type_alloc (datatype.c:1127) ==28352== by 0x4E6D3B3: netlink_delinearize_set (netlink.c:578) ==28352== by 0x4E6D68E: list_set_cb (netlink.c:648) ==28352== by 0x5D74023: nftnl_set_list_foreach (set.c:780) ==28352== by 0x4E6D6F3: netlink_list_sets (netlink.c:669) ==28352== by 0x4E5A7A3: cache_init_objects (rule.c:159) ==28352== by 0x4E5A7A3: cache_init (rule.c:216) ==28352== by 0x4E5A7A3: cache_update (rule.c:266) ==28352== by 0x4E7E0EE: nft_evaluate (libnftables.c:388) ==28352== by 0x4E7EADD: nft_run_cmd_from_filename (libnftables.c:479) ==28352== by 0x109A53: main (main.c:310) This patch also removes the DTYPE_F_CLONE flag which is broken and not needed anymore since proper reference counting is in place. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* parser_bison: free chain name after creating constant expressionPablo Neira Ayuso2019-06-101-0/+1
| | | | | | | | | | | | | | | ==2330== 2 bytes in 1 blocks are definitely lost in loss record 1 of 1 ==2330== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==2330== by 0x583D3B9: strdup (strdup.c:42) ==2330== by 0x4E7966D: xstrdup (utils.c:75) ==2330== by 0x4E9C283: nft_lex (scanner.l:626) ==2330== by 0x4E8E3C2: nft_parse (parser_bison.c:5297) ==2330== by 0x4E7EAB2: nft_parse_bison_filename (libnftables.c:374) ==2330== by 0x4E7EAB2: nft_run_cmd_from_filename (libnftables.c:475) ==2330== by 0x109A53: main (main.c:310) Fixes: f1e8a129ee42 ("src: Introduce chain_expr in jump and goto statements") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* netlink_delinearize: release expression before calling ↵Pablo Neira Ayuso2019-06-101-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | netlink_parse_concat_expr() netlink_get_register() clones the expression in the register. Release this expression before calling netlink_parse_concat_expr() to deconstruct the concatenation. ==15069== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==15069== by 0x4E79508: xmalloc (utils.c:36) ==15069== by 0x4E795ED: xzalloc (utils.c:65) ==15069== by 0x4E6029B: dtype_alloc (datatype.c:1073) ==15069== by 0x4E6029B: concat_type_alloc (datatype.c:1127) ==15069== by 0x4E6D3B3: netlink_delinearize_set (netlink.c:578) ==15069== by 0x4E6D68E: list_set_cb (netlink.c:648) ==15069== by 0x5F34023: nftnl_set_list_foreach (set.c:780) ==15069== by 0x4E6D6F3: netlink_list_sets (netlink.c:669) ==15069== by 0x4E5A7A3: cache_init_objects (rule.c:159) ==15069== by 0x4E5A7A3: cache_init (rule.c:216) ==15069== by 0x4E5A7A3: cache_update (rule.c:266) ==15069== by 0x4E7E09E: nft_evaluate (libnftables.c:388) ==15069== by 0x4E7E85B: nft_run_cmd_from_buffer (libnftables.c:428) Reported-by: Václav Zindulka <vaclav.zindulka@tlapnet.cz> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* netlink_delinearize: release expressions in context registersPablo Neira Ayuso2019-06-102-4/+6
| | | | | | | | | netlink_release_registers() needs to go a bit further to release the expressions in the register array. This should be safe since netlink_get_register() clones expressions in the context registers. Reported-by: Václav Zindulka <vaclav.zindulka@tlapnet.cz> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* expression: use expr_clone() from verdict_expr_clone()Pablo Neira Ayuso2019-06-101-1/+1
| | | | | | | | | | | | | | | | | | | Chains are now expressions, do not assume a constant value is used. ==26302== Process terminating with default action of signal 11 (SIGSEGV) ==26302== Access not within mapped region at address 0x50 ==26302== at 0x67D7EE7: __gmpz_init_set (in /usr/lib/x86_64-linux-gnu/libgmp.so.10.3.2) ==26302== by 0x4E61224: expr_clone (expression.c:65) ==26302== by 0x4E7898B: interval_map_decompose (segtree.c:943) ==26302== by 0x4E6DDA0: netlink_list_setelems (netlink.c:882) ==26302== by 0x4E5A806: cache_init_objects (rule.c:166) ==26302== by 0x4E5A806: cache_init (rule.c:216) ==26302== by 0x4E5A806: cache_update (rule.c:266) ==26302== by 0x4E7E0EE: nft_evaluate (libnftables.c:388) ==26302== by 0x4E7E8AB: nft_run_cmd_from_buffer (libnftables.c:428) Fixes: f1e8a129ee42 ("src: Introduce chain_expr in jump and goto statements") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: invalid read when importing chain name (trace and json)Pablo Neira Ayuso2019-06-102-3/+2
| | | | | | | Update trace and json too. Fixes: 142350f154c7 ("src: invalid read when importing chain name") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: invalid read when importing chain namePablo Neira Ayuso2019-06-102-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | Use strlen(), otherwise mpz_import_data() reads too much beyond the real chain string. Valgrind reports the following error: ==2759== Invalid read of size 1 ==2759== at 0x67D68D6: __gmpz_import (in /usr/lib/x86_64-linux-gnu/libgmp.so.10.3.2) ==2759== by 0x4E79467: mpz_import_data (gmputil.c:133) ==2759== by 0x4E60A12: constant_expr_alloc (expression.c:375) ==2759== by 0x4E8ED65: nft_parse (parser_bison.y:3825) ==2759== by 0x4E7E850: nft_parse_bison_buffer (libnftables.c:357) ==2759== by 0x4E7E850: nft_run_cmd_from_buffer (libnftables.c:424) ==2759== by 0x1095D4: main (in /tmp/a.out) ==2759== Address 0x6ee1b4a is 0 bytes after a block of size 10 alloc'd ==2759== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==2759== by 0x59FD3B9: strdup (strdup.c:42) ==2759== by 0x4E7963D: xstrdup (utils.c:75) ==2759== by 0x4E9C233: nft_lex (scanner.l:626) ==2759== by 0x4E8E382: nft_parse (parser_bison.c:5297) ==2759== by 0x4E7E850: nft_parse_bison_buffer (libnftables.c:357) ==2759== by 0x4E7E850: nft_run_cmd_from_buffer (libnftables.c:424) Fixes: f1e8a129ee42 ("src: Introduce chain_expr in jump and goto statements") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests/shell: Print unified diffs in dump errorsPhil Sutter2019-06-081-1/+1
| | | | | | | | Non-unified format is useful only if the expected output is printed as well, which is not the case. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests/shell: Fix warning from awk callPhil Sutter2019-06-081-1/+1
| | | | | | | | | Syntax passed to awk in that one testcase caused a warning, fix the syntax. Fixes: e0a9aad024809 ("tests: shell: fix tests for deletion via handle attribute") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests/py: Add missing arp.t JSON equivalentsPhil Sutter2019-06-082-8/+70
| | | | | | Fixes: 4b0f2a712b579 ("src: support for arp sender and target ethernet and IPv4 addresses") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests/py: Fix JSON equivalentsPhil Sutter2019-06-083-80/+86
| | | | | | | | | Recent patch removing single element set use missed to adjust JSON equivalents accordingly. Fixes: 27f6a4c68b4fd ("tests: replace single element sets") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: Support intra-transaction rule referencesPhil Sutter2019-06-078-20/+139
| | | | | | | | | | | | | | | | | | | | | | | A rule may be added before or after another one using index keyword. To support for the other rule being added within the same batch, one has to make use of NFTNL_RULE_ID and NFTNL_RULE_POSITION_ID attributes. This patch does just that among a few more crucial things: * If cache is complete enough to contain rules, update cache when evaluating rule commands so later index references resolve correctly. * Reduce rule_translate_index() to its core code which is the actual linking of rules and consequently rename the function. The removed bits are pulled into the calling rule_evaluate() to reduce code duplication in between cache updates with and without rule reference. * Pass the current command op to rule_evaluate() as indicator whether to insert before or after a referenced rule or at beginning or end of chain in cache. Exploit this from chain_evaluate() to avoid adding the chain's rules a second time. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: Make cache_is_complete() publicPhil Sutter2019-06-072-1/+2
| | | | | Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* rule: Introduce rule_lookup_by_index()Phil Sutter2019-06-072-0/+13
| | | | | | | | In contrast to rule_lookup(), this function returns a chain's rule at a given index instead of by handle. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests/json_echo: Drop needless workaroundPhil Sutter2019-06-071-4/+2
| | | | | | | | With cache issues now resolved, there is no need for the multi add test workaround anymore. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* cache: Fix evaluation for rules with index referencePhil Sutter2019-06-071-7/+1
| | | | | | | | | After parsing input, rule location data (index or handle) is contained in cmd->handle, not yet in cmd->rule->handle. Fixes: 7df42800cf89e ("src: single cache_update() call to build cache before evaluation") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* libnftables: check for errors after evaluationsPablo Neira Ayuso2019-06-071-0/+3
| | | | | | | | Check for state->nerrs after evaluation to restore error reporting when evaluation fails. Fixes: df2f746fb4cf ("libnftables: keep evaluating until parser_max_errors") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* mnl: bogus error when running monitor modePablo Neira Ayuso2019-06-071-3/+1
| | | | | | | | | | Fix bogus error message: # nft monitor Cannot set up netlink socket buffer size to 16777216 bytes, falling back to 16777216 bytes Fixes: bcf60fb819bf ("mnl: add mnl_set_rcvbuffer() and use it") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* libnftables: keep evaluating until parser_max_errorsPablo Neira Ayuso2019-06-071-1/+2
| | | | | | | | | | | | | | | | | | Bail out after parser_max_errors has been reached, eg. # nft -f /tmp/errors.nft /tmp/errors.nft:1:23-23: Error: syntax error, unexpected newline filter input tcp dport ^ /tmp/errors.nft:2:24-26: Error: datatype mismatch, expected internet network service, expression has type Internet protocol filter input tcp dport tcp ~~~~~~~~~ ^^^ /tmp/errors.nft:3:24-26: Error: datatype mismatch, expected internet network service, expression has type Internet protocol filter input tcp sport udp ~~~~~~~~~ ^^^ Fixes: f211921e25e6 ("src: perform evaluation after parsing") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: use-after-free in implicit setPablo Neira Ayuso2019-06-071-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | # cat example.nft table inet test { chain test { ip daddr { 2.2.2.2, 4.4.4.4} counter accept } } # valgrind nft -f example.nft valgrind reports: ==2272== Invalid read of size 4 ==2272== at 0x4E612A5: expr_free (expression.c:86) ==2272== by 0x4E58EA2: set_free (rule.c:367) ==2272== by 0x4E612DA: expr_destroy (expression.c:79) ==2272== by 0x4E612DA: expr_free (expression.c:93) ==2272== by 0x4E612DA: expr_destroy (expression.c:79) ==2272== by 0x4E612DA: expr_free (expression.c:93) ==2272== by 0x4E5D7E7: stmt_free (statement.c:50) ==2272== by 0x4E5D8B7: stmt_list_free (statement.c:60) ==2272== by 0x4E590FF: rule_free (rule.c:610) ==2272== by 0x4E5C094: cmd_free (rule.c:1420) ==2272== by 0x4E7E7EF: nft_run_cmd_from_filename (libnftables.c:490) ==2272== by 0x109A53: main (main.c:310) ==2272== Address 0x65d94c8 is 56 bytes inside a block of size 128 free'd ==2272== at 0x4C2CDDB: free (vg_replace_malloc.c:530) ==2272== by 0x4E6143C: mapping_expr_destroy (expression.c:966) ==2272== by 0x4E612DA: expr_destroy (expression.c:79) ==2272== by 0x4E612DA: expr_free (expression.c:93) ==2272== by 0x4E5D7E7: stmt_free (statement.c:50) ==2272== by 0x4E5D8B7: stmt_list_free (statement.c:60) ==2272== by 0x4E590FF: rule_free (rule.c:610) ==2272== by 0x4E5C094: cmd_free (rule.c:1420) ==2272== by 0x4E7E7EF: nft_run_cmd_from_filename (libnftables.c:490) ==2272== by 0x109A53: main (main.c:310) ==2272== Block was alloc'd at ==2272== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299) ==2272== by 0x4E79248: xmalloc (utils.c:36) ==2272== by 0x4E7932D: xzalloc (utils.c:65) ==2272== by 0x4E60690: expr_alloc (expression.c:45) ==2272== by 0x4E68B1D: payload_expr_alloc (payload.c:159) ==2272== by 0x4E91013: nft_parse (parser_bison.y:4242) ==2272== by 0x4E7E722: nft_parse_bison_filename (libnftables.c:374) ==2272== by 0x4E7E722: nft_run_cmd_from_filename (libnftables.c:471) ==2272== by 0x109A53: main (main.c:310) Fixes: cc7b37d18a68 ("src: Interpret OP_NEQ against a set as OP_LOOKUP") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* rule: ensure cache consistencyPablo Neira Ayuso2019-06-071-1/+8
| | | | | | | Check for generation ID after the cache is populated. In case of interference, release the inconsistent cache and retry. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: generation ID is 32-bit longPablo Neira Ayuso2019-06-074-8/+12
| | | | | | | | Update mnl_genid_get() to return 32-bit long generation ID. Add nft_genid_u16() which allows us to catch ruleset updates from the netlink dump path via 16-bit long nfnetlink resource ID field. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* libnftables: Drop cache in error casePhil Sutter2019-06-061-0/+4
| | | | | | | | | | | | | | | | | | | | | | | If a transaction is rejected by the kernel (for instance due to a semantic error), cache contents are potentially invalid. Release the cache in that case to avoid the inconsistency. The problem is easy to reproduce in an interactive session: | nft> list ruleset | table ip t { | chain c { | } | } | nft> flush ruleset; add rule ip t c accept | Error: No such file or directory | flush ruleset; add rule ip t c accept | ^ | nft> list ruleset | nft> Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: Fix cache_flush() in cache_needs_more() logicPhil Sutter2019-06-061-0/+3
| | | | | | | | | | | | | | | Commit 34a20645d54fa enabled cache updates depending on command causing it. As a side-effect, this disabled measures in cache_flush() preventing a later cache update. Re-establish this by setting cache->cmd in addition to cache->genid after dropping cache entries. While being at it, set cache->cmd in cache_release() as well. This shouldn't be necessary since zeroing cache->genid should suffice for cache_update(), but better be consistent (and future-proof) here. Fixes: eeda228c2d17 ("src: update cache if cmd is more specific") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: single cache_update() call to build cache before evaluationPablo Neira Ayuso2019-06-067-102/+144
| | | | | | | | | | | | | | | This patch allows us to make one single cache_update() call. Thus, there is not need to rebuild an incomplete cache from the middle of the batch processing. Note that nft_run_cmd_from_filename() does not need a full netlink dump to build the cache anymore, this should speed nft -f with incremental updates and very large rulesets. cache_evaluate() calculates the netlink dump to populate the cache that this batch needs. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: Display parser and evaluate errors in one shotPablo Neira Ayuso2019-06-052-15/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch restores 61236968b7a1 ("parser: evaluate commands immediately after parsing") following a different approach. In this patch, the evaluation phase is done if the parsing phase fails, hence the user gets parsing and evaluation errors in one shot, which is the purpose of 61236968b7a1. Note that evaluation errors are now shown after parser errors, the example available in 61236968b7a1 displays with this patch the following error: # nft -f /tmp/bad.nft /tmp/bad.nft:3:32-32: Error: syntax error, unexpected newline add rule filter input tcp dport ^ /tmp/bad.nft:5:37-41: Error: syntax error, unexpected dport, expecting end of file or newline or semicolon add rule filter input tcp dport tcp dport ^^^^^ /tmp/bad.nft:4:33-35: Error: datatype mismatch, expected internet network service, expression has type Internet protocol add rule filter input tcp dport tcp ~~~~~~~~~ ^^^ So evaluation pointing to line 4 happens after line error reporting generated by the parser that points to line 3, while 61236968b7a1 was showing errors per line in order. As a future work, we can sort the error reporting list to restore exactly the same behaviour. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: perform evaluation after parsingPablo Neira Ayuso2019-06-053-31/+27
| | | | | | | | | | | | | | | | | Since 61236968b7a1 ("parser: evaluate commands immediately after parsing"), evaluation is invoked from the parsing phase in order to improve error reporting. However, this approach is problematic from the cache perspective since we don't know if a full or partial netlink dump from the kernel is needed. If the number of objects in the kernel is significant, the netlink dump operation to build the cache may significantly slow down commands. This patch moves the evaluation phase after the parsing phase as a preparation update to allow for a better strategy to build the cache. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: dynamic input_descriptor allocationPablo Neira Ayuso2019-06-055-56/+47
| | | | | | | | | | | | | This patch introduces the input descriptor list, that stores the existing input descriptor objects. These objects are now dynamically allocated and release from scanner_destroy() path. Follow up patches that decouple the parsing and the evaluation phases require this for error reporting as described by b14572f72aac ("erec: Fix input descriptors for included files"), this patch partially reverts such partial. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests/py: Support JSON validationPhil Sutter2019-05-311-1/+24
| | | | | | | | | | Introduce a new flag -s/--schema to nft-test.py which enables validation of any JSON input and output against our schema. Make use of traceback module to get more details if validation fails. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* py: Implement JSON validation in nftables modulePhil Sutter2019-05-314-1/+47
| | | | | | | | | | | | Using jsonschema it is possible to validate any JSON input to make sure it formally conforms with libnftables JSON API requirements. Implement a simple validator class for use within a new Nftables class method 'json_validate' and ship a minimal schema definition along with the package. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* mnl: Simplify mnl_batch_talk()Phil Sutter2019-05-311-16/+13
| | | | | | | | | | | | | | | | | | By mimicking mnl_nft_event_listener() code, mnl_batch_talk() may be simplified quite a bit: * Turn the conditional loop into an unconditional one. * Call select() at loop start, which merges the two call sites. * Check readfds content after select() returned instead of in loop condition - if fd is not set, break to return error state stored in 'err' variable. * Old code checked that select() return code is > 0, but that was redundant: if FD_ISSET() returns true, select return code was 1. * Move 'nlh' helper variable definition into error handling block, it is not used outside of it. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* mnl: Initialize fd_set before select(), not afterPhil Sutter2019-05-311-3/+3
| | | | | | | | | | Calling FD_SET() in between return of select() and call to FD_ISSET() effectively renders the whole thing useless: FD_ISSET() will always return true no matter what select() actually did. Fixes: a72315d2bad47 ("src: add rule batching support") Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests/shell: Test large transaction with echo outputPhil Sutter2019-05-311-0/+14
| | | | | | | | | This reliably triggered ENOBUFS condition in mnl_batch_talk(). With the past changes, it passes even after increasing the number of rules to 300k. Signed-off-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* erec: remove double \n on error when internal_netlink is usedPablo Neira Ayuso2019-05-311-1/+0
| | | | | | Remove double empty line linebreak when printing internal errors. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* mnl: mnl_batch_talk() returns -1 on internal netlink errorsPablo Neira Ayuso2019-05-312-5/+10
| | | | | | Display an error in case internal netlink plumbing hits problems. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* mnl: estimate receiver buffer sizePablo Neira Ayuso2019-05-313-5/+12
| | | | | | | | | | | | | | | | | Set a receiver buffer size based on the number of commands and the average message size, this is useful for the --echo option in order to avoid ENOBUFS errors. On the kernel side, each skbuff consumes truesize from the socket queue (although it uses NLMSG_GOODSIZE to allocate it), which is approximately four times the estimated size per message that we get in turn for each echo message to ensure enough receiver buffer space. We could also explore increasing the buffer and retry if mnl_nft_socket_sendmsg() hits ENOBUFS if we ever hit this problem again. Reported-by: Phil Sutter <phil@nwl.cc> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
generated by cgit v1.2.3 (git 2.25.0) at 2024-11-16 15:22:12 +0000