Skip to content

Commit e38fbfa

Browse files
Florian Westphalummakynes
authored andcommitted
netfilter: nf_tables: merge nft_rules_old structure and end of ruleblob marker
In order to free the rules in a chain via call_rcu, the rule array used to stash a rcu_head and space for a pointer at the end of the rule array. When the current nft_rule_dp blob format got added in 2c865a8 ("netfilter: nf_tables: add rule blob layout"), this results in a double-trailer: size (unsigned long) struct nft_rule_dp struct nft_expr ... struct nft_rule_dp struct nft_expr ... struct nft_rule_dp (is_last=1) // Trailer The trailer, struct nft_rule_dp (is_last=1), is not accounted for in size, so it can be located via start_addr + size. Because the rcu_head is stored after 'start+size' as well this means the is_last trailer is *aliased* to the rcu_head (struct nft_rules_old). This is harmless, because at this time the nft_do_chain function never evaluates/accesses the trailer, it only checks the address boundary: for (; rule < last_rule; rule = nft_rule_next(rule)) { ... But this way the last_rule address has to be stashed in the jump structure to restore it after returning from a chain. nft_do_chain stack usage has become way too big, so put it on a diet. Without this patch is impossible to use for (; !rule->is_last; rule = nft_rule_next(rule)) { ... because on free, the needed update of the rcu_head will clobber the nft_rule_dp is_last bit. Furthermore, also stash the chain pointer in the trailer, this allows to recover the original chain structure from nf_tables_trace infra without a need to place them in the jump struct. After this patch it is trivial to diet the jump stack structure, done in the next two patches. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
1 parent ca28896 commit e38fbfa

File tree

1 file changed

+27
-28
lines changed

1 file changed

+27
-28
lines changed

net/netfilter/nf_tables_api.c

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2110,38 +2110,41 @@ static void nft_chain_release_hook(struct nft_chain_hook *hook)
21102110
module_put(hook->type->owner);
21112111
}
21122112

2113-
struct nft_rules_old {
2113+
struct nft_rule_dp_last {
2114+
struct nft_rule_dp end; /* end of nft_rule_blob marker */
21142115
struct rcu_head h;
21152116
struct nft_rule_blob *blob;
2117+
const struct nft_chain *chain; /* for tracing */
21162118
};
21172119

2118-
static void nft_last_rule(struct nft_rule_blob *blob, const void *ptr)
2120+
static void nft_last_rule(const struct nft_chain *chain, const void *ptr)
21192121
{
2120-
struct nft_rule_dp *prule;
2122+
struct nft_rule_dp_last *lrule;
2123+
2124+
BUILD_BUG_ON(offsetof(struct nft_rule_dp_last, end) != 0);
21212125

2122-
prule = (struct nft_rule_dp *)ptr;
2123-
prule->is_last = 1;
2126+
lrule = (struct nft_rule_dp_last *)ptr;
2127+
lrule->end.is_last = 1;
2128+
lrule->chain = chain;
21242129
/* blob size does not include the trailer rule */
21252130
}
21262131

2127-
static struct nft_rule_blob *nf_tables_chain_alloc_rules(unsigned int size)
2132+
static struct nft_rule_blob *nf_tables_chain_alloc_rules(const struct nft_chain *chain,
2133+
unsigned int size)
21282134
{
21292135
struct nft_rule_blob *blob;
21302136

2131-
/* size must include room for the last rule */
2132-
if (size < offsetof(struct nft_rule_dp, data))
2133-
return NULL;
2134-
2135-
size += sizeof(struct nft_rule_blob) + sizeof(struct nft_rules_old);
21362137
if (size > INT_MAX)
21372138
return NULL;
21382139

2140+
size += sizeof(struct nft_rule_blob) + sizeof(struct nft_rule_dp_last);
2141+
21392142
blob = kvmalloc(size, GFP_KERNEL_ACCOUNT);
21402143
if (!blob)
21412144
return NULL;
21422145

21432146
blob->size = 0;
2144-
nft_last_rule(blob, blob->data);
2147+
nft_last_rule(chain, blob->data);
21452148

21462149
return blob;
21472150
}
@@ -2220,7 +2223,6 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
22202223
struct nft_rule_blob *blob;
22212224
struct nft_trans *trans;
22222225
struct nft_chain *chain;
2223-
unsigned int data_size;
22242226
int err;
22252227

22262228
if (table->use == UINT_MAX)
@@ -2308,8 +2310,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
23082310
chain->udlen = nla_len(nla[NFTA_CHAIN_USERDATA]);
23092311
}
23102312

2311-
data_size = offsetof(struct nft_rule_dp, data); /* last rule */
2312-
blob = nf_tables_chain_alloc_rules(data_size);
2313+
blob = nf_tables_chain_alloc_rules(chain, 0);
23132314
if (!blob) {
23142315
err = -ENOMEM;
23152316
goto err_destroy_chain;
@@ -8817,9 +8818,8 @@ static int nf_tables_commit_chain_prepare(struct net *net, struct nft_chain *cha
88178818
return -ENOMEM;
88188819
}
88198820
}
8820-
data_size += offsetof(struct nft_rule_dp, data); /* last rule */
88218821

8822-
chain->blob_next = nf_tables_chain_alloc_rules(data_size);
8822+
chain->blob_next = nf_tables_chain_alloc_rules(chain, data_size);
88238823
if (!chain->blob_next)
88248824
return -ENOMEM;
88258825

@@ -8864,12 +8864,11 @@ static int nf_tables_commit_chain_prepare(struct net *net, struct nft_chain *cha
88648864
chain->blob_next->size += (unsigned long)(data - (void *)prule);
88658865
}
88668866

8867-
prule = (struct nft_rule_dp *)data;
8868-
data += offsetof(struct nft_rule_dp, data);
88698867
if (WARN_ON_ONCE(data > data_boundary))
88708868
return -ENOMEM;
88718869

8872-
nft_last_rule(chain->blob_next, prule);
8870+
prule = (struct nft_rule_dp *)data;
8871+
nft_last_rule(chain, prule);
88738872

88748873
return 0;
88758874
}
@@ -8890,22 +8889,22 @@ static void nf_tables_commit_chain_prepare_cancel(struct net *net)
88908889
}
88918890
}
88928891

8893-
static void __nf_tables_commit_chain_free_rules_old(struct rcu_head *h)
8892+
static void __nf_tables_commit_chain_free_rules(struct rcu_head *h)
88948893
{
8895-
struct nft_rules_old *o = container_of(h, struct nft_rules_old, h);
8894+
struct nft_rule_dp_last *l = container_of(h, struct nft_rule_dp_last, h);
88968895

8897-
kvfree(o->blob);
8896+
kvfree(l->blob);
88988897
}
88998898

89008899
static void nf_tables_commit_chain_free_rules_old(struct nft_rule_blob *blob)
89018900
{
8902-
struct nft_rules_old *old;
8901+
struct nft_rule_dp_last *last;
89038902

8904-
/* rcu_head is after end marker */
8905-
old = (void *)blob + sizeof(*blob) + blob->size;
8906-
old->blob = blob;
8903+
/* last rule trailer is after end marker */
8904+
last = (void *)blob + sizeof(*blob) + blob->size;
8905+
last->blob = blob;
89078906

8908-
call_rcu(&old->h, __nf_tables_commit_chain_free_rules_old);
8907+
call_rcu(&last->h, __nf_tables_commit_chain_free_rules);
89098908
}
89108909

89118910
static void nf_tables_commit_chain(struct net *net, struct nft_chain *chain)

0 commit comments

Comments
 (0)