|
| 1 | +netfilter: nf_tables: prefer nft_chain_validate |
| 2 | + |
| 3 | +jira LE-3201 |
| 4 | +cve CVE-2024-41042 |
| 5 | +Rebuild_History Non-Buildable kernel-rt-4.18.0-553.27.1.rt7.368.el8_10 |
| 6 | +commit-author Florian Westphal < [email protected]> |
| 7 | +commit cff3bd012a9512ac5ed858d38e6ed65f6391008c |
| 8 | +Empty-Commit: Cherry-Pick Conflicts during history rebuild. |
| 9 | +Will be included in final tarball splat. Ref for failed cherry-pick at: |
| 10 | +ciq/ciq_backports/kernel-rt-4.18.0-553.27.1.rt7.368.el8_10/cff3bd01.failed |
| 11 | + |
| 12 | +nft_chain_validate already performs loop detection because a cycle will |
| 13 | +result in a call stack overflow (ctx->level >= NFT_JUMP_STACK_SIZE). |
| 14 | + |
| 15 | +It also follows maps via ->validate callback in nft_lookup, so there |
| 16 | +appears no reason to iterate the maps again. |
| 17 | + |
| 18 | +nf_tables_check_loops() and all its helper functions can be removed. |
| 19 | +This improves ruleset load time significantly, from 23s down to 12s. |
| 20 | + |
| 21 | +This also fixes a crash bug. Old loop detection code can result in |
| 22 | +unbounded recursion: |
| 23 | + |
| 24 | +BUG: TASK stack guard page was hit at .... |
| 25 | +Oops: stack guard page: 0000 [#1] PREEMPT SMP KASAN |
| 26 | +CPU: 4 PID: 1539 Comm: nft Not tainted 6.10.0-rc5+ #1 |
| 27 | +[..] |
| 28 | + |
| 29 | +with a suitable ruleset during validation of register stores. |
| 30 | + |
| 31 | +I can't see any actual reason to attempt to check for this from |
| 32 | +nft_validate_register_store(), at this point the transaction is still in |
| 33 | +progress, so we don't have a full picture of the rule graph. |
| 34 | + |
| 35 | +For nf-next it might make sense to either remove it or make this depend |
| 36 | +on table->validate_state in case we could catch an error earlier |
| 37 | +(for improved error reporting to userspace). |
| 38 | + |
| 39 | +Fixes: 20a69341f2d0 ("netfilter: nf_tables: add netlink set API") |
| 40 | + Signed-off-by: Florian Westphal < [email protected]> |
| 41 | + Signed-off-by: Pablo Neira Ayuso < [email protected]> |
| 42 | +(cherry picked from commit cff3bd012a9512ac5ed858d38e6ed65f6391008c) |
| 43 | + Signed-off-by: Jonathan Maple < [email protected]> |
| 44 | + |
| 45 | +# Conflicts: |
| 46 | +# net/netfilter/nf_tables_api.c |
| 47 | +diff --cc net/netfilter/nf_tables_api.c |
| 48 | +index a8d03cec29c3,91cc3a81ba8f..000000000000 |
| 49 | +--- a/net/netfilter/nf_tables_api.c |
| 50 | ++++ b/net/netfilter/nf_tables_api.c |
| 51 | +@@@ -8781,107 -10821,6 +8793,110 @@@ int nft_chain_validate_hooks(const stru |
| 52 | + } |
| 53 | + EXPORT_SYMBOL_GPL(nft_chain_validate_hooks); |
| 54 | + |
| 55 | +++<<<<<<< HEAD |
| 56 | + +/* |
| 57 | + + * Loop detection - walk through the ruleset beginning at the destination chain |
| 58 | + + * of a new jump until either the source chain is reached (loop) or all |
| 59 | + + * reachable chains have been traversed. |
| 60 | + + * |
| 61 | + + * The loop check is performed whenever a new jump verdict is added to an |
| 62 | + + * expression or verdict map or a verdict map is bound to a new chain. |
| 63 | + + */ |
| 64 | + + |
| 65 | + +static int nf_tables_check_loops(const struct nft_ctx *ctx, |
| 66 | + + const struct nft_chain *chain); |
| 67 | + + |
| 68 | + +static int nf_tables_loop_check_setelem(const struct nft_ctx *ctx, |
| 69 | + + struct nft_set *set, |
| 70 | + + const struct nft_set_iter *iter, |
| 71 | + + struct nft_set_elem *elem) |
| 72 | + +{ |
| 73 | + + const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv); |
| 74 | + + const struct nft_data *data; |
| 75 | + + |
| 76 | + + if (nft_set_ext_exists(ext, NFT_SET_EXT_FLAGS) && |
| 77 | + + *nft_set_ext_flags(ext) & NFT_SET_ELEM_INTERVAL_END) |
| 78 | + + return 0; |
| 79 | + + |
| 80 | + + data = nft_set_ext_data(ext); |
| 81 | + + switch (data->verdict.code) { |
| 82 | + + case NFT_JUMP: |
| 83 | + + case NFT_GOTO: |
| 84 | + + return nf_tables_check_loops(ctx, data->verdict.chain); |
| 85 | + + default: |
| 86 | + + return 0; |
| 87 | + + } |
| 88 | + +} |
| 89 | + + |
| 90 | + +static int nf_tables_check_loops(const struct nft_ctx *ctx, |
| 91 | + + const struct nft_chain *chain) |
| 92 | + +{ |
| 93 | + + const struct nft_rule *rule; |
| 94 | + + const struct nft_expr *expr, *last; |
| 95 | + + struct nft_set *set; |
| 96 | + + struct nft_set_binding *binding; |
| 97 | + + struct nft_set_iter iter; |
| 98 | + + |
| 99 | + + if (ctx->chain == chain) |
| 100 | + + return -ELOOP; |
| 101 | + + |
| 102 | + + list_for_each_entry(rule, &chain->rules, list) { |
| 103 | + + nft_rule_for_each_expr(expr, last, rule) { |
| 104 | + + struct nft_immediate_expr *priv; |
| 105 | + + const struct nft_data *data; |
| 106 | + + int err; |
| 107 | + + |
| 108 | + + if (strcmp(expr->ops->type->name, "immediate")) |
| 109 | + + continue; |
| 110 | + + |
| 111 | + + priv = nft_expr_priv(expr); |
| 112 | + + if (priv->dreg != NFT_REG_VERDICT) |
| 113 | + + continue; |
| 114 | + + |
| 115 | + + data = &priv->data; |
| 116 | + + switch (data->verdict.code) { |
| 117 | + + case NFT_JUMP: |
| 118 | + + case NFT_GOTO: |
| 119 | + + err = nf_tables_check_loops(ctx, |
| 120 | + + data->verdict.chain); |
| 121 | + + if (err < 0) |
| 122 | + + return err; |
| 123 | + + break; |
| 124 | + + default: |
| 125 | + + break; |
| 126 | + + } |
| 127 | + + } |
| 128 | + + } |
| 129 | + + |
| 130 | + + list_for_each_entry(set, &ctx->table->sets, list) { |
| 131 | + + if (!nft_is_active_next(ctx->net, set)) |
| 132 | + + continue; |
| 133 | + + if (!(set->flags & NFT_SET_MAP) || |
| 134 | + + set->dtype != NFT_DATA_VERDICT) |
| 135 | + + continue; |
| 136 | + + |
| 137 | + + list_for_each_entry(binding, &set->bindings, list) { |
| 138 | + + if (!(binding->flags & NFT_SET_MAP) || |
| 139 | + + binding->chain != chain) |
| 140 | + + continue; |
| 141 | + + |
| 142 | + + iter.genmask = nft_genmask_next(ctx->net); |
| 143 | + + iter.skip = 0; |
| 144 | + + iter.count = 0; |
| 145 | + + iter.err = 0; |
| 146 | + + iter.fn = nf_tables_loop_check_setelem; |
| 147 | + + |
| 148 | + + set->ops->walk(ctx, set, &iter); |
| 149 | + + if (iter.err < 0) |
| 150 | + + return iter.err; |
| 151 | + + } |
| 152 | + + } |
| 153 | + + |
| 154 | + + return 0; |
| 155 | + +} |
| 156 | + + |
| 157 | +++======= |
| 158 | +++>>>>>>> cff3bd012a95 (netfilter: nf_tables: prefer nft_chain_validate) |
| 159 | + /** |
| 160 | + * nft_parse_u32_check - fetch u32 attribute and check for maximum value |
| 161 | + * |
| 162 | +* Unmerged path net/netfilter/nf_tables_api.c |
0 commit comments