Skip to content

Commit 5910544

Browse files
ummakynesummakynes
committed
netfilter: nf_tables: revisit chain/object refcounting from elements
Andreas reports that the following incremental update using our commit protocol doesn't work. # nft -f incremental-update.nft delete element ip filter client_to_any { 10.180.86.22 : goto CIn_1 } delete chain ip filter CIn_1 ... Error: Could not process rule: Device or resource busy The existing code is not well-integrated into the commit phase protocol, since element deletions do not result in refcount decrement from the preparation phase. This results in bogus EBUSY errors like the one above. Two new functions come with this patch: * nft_set_elem_activate() function is used from the abort path, to restore the set element refcounting on objects that occurred from the preparation phase. * nft_set_elem_deactivate() that is called from nft_del_setelem() to decrement set element refcounting on objects from the preparation phase in the commit protocol. The nft_data_uninit() has been renamed to nft_data_release() since this function does not uninitialize any data store in the data register, instead just releases the references to objects. Moreover, a new function nft_data_hold() has been introduced to be used from nft_set_elem_activate(). Reported-by: Andreas Schultz <aschultz@tpip.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
1 parent 71df14b commit 5910544

File tree

6 files changed

+81
-18
lines changed

6 files changed

+81
-18
lines changed

Diff for: ‎include/net/netfilter/nf_tables.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ struct nft_data_desc {
176176
int nft_data_init(const struct nft_ctx *ctx,
177177
struct nft_data *data, unsigned int size,
178178
struct nft_data_desc *desc, const struct nlattr *nla);
179-
void nft_data_uninit(const struct nft_data *data, enum nft_data_types type);
179+
void nft_data_release(const struct nft_data *data, enum nft_data_types type);
180180
int nft_data_dump(struct sk_buff *skb, int attr, const struct nft_data *data,
181181
enum nft_data_types type, unsigned int len);
182182

Diff for: ‎net/netfilter/nf_tables_api.c

+72-10
Original file line numberDiff line numberDiff line change
@@ -3627,9 +3627,9 @@ void nft_set_elem_destroy(const struct nft_set *set, void *elem,
36273627
{
36283628
struct nft_set_ext *ext = nft_set_elem_ext(set, elem);
36293629

3630-
nft_data_uninit(nft_set_ext_key(ext), NFT_DATA_VALUE);
3630+
nft_data_release(nft_set_ext_key(ext), NFT_DATA_VALUE);
36313631
if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
3632-
nft_data_uninit(nft_set_ext_data(ext), set->dtype);
3632+
nft_data_release(nft_set_ext_data(ext), set->dtype);
36333633
if (destroy_expr && nft_set_ext_exists(ext, NFT_SET_EXT_EXPR))
36343634
nf_tables_expr_destroy(NULL, nft_set_ext_expr(ext));
36353635
if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
@@ -3638,6 +3638,18 @@ void nft_set_elem_destroy(const struct nft_set *set, void *elem,
36383638
}
36393639
EXPORT_SYMBOL_GPL(nft_set_elem_destroy);
36403640

3641+
/* Only called from commit path, nft_set_elem_deactivate() already deals with
3642+
* the refcounting from the preparation phase.
3643+
*/
3644+
static void nf_tables_set_elem_destroy(const struct nft_set *set, void *elem)
3645+
{
3646+
struct nft_set_ext *ext = nft_set_elem_ext(set, elem);
3647+
3648+
if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPR))
3649+
nf_tables_expr_destroy(NULL, nft_set_ext_expr(ext));
3650+
kfree(elem);
3651+
}
3652+
36413653
static int nft_setelem_parse_flags(const struct nft_set *set,
36423654
const struct nlattr *attr, u32 *flags)
36433655
{
@@ -3849,9 +3861,9 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
38493861
kfree(elem.priv);
38503862
err3:
38513863
if (nla[NFTA_SET_ELEM_DATA] != NULL)
3852-
nft_data_uninit(&data, d2.type);
3864+
nft_data_release(&data, d2.type);
38533865
err2:
3854-
nft_data_uninit(&elem.key.val, d1.type);
3866+
nft_data_release(&elem.key.val, d1.type);
38553867
err1:
38563868
return err;
38573869
}
@@ -3896,6 +3908,53 @@ static int nf_tables_newsetelem(struct net *net, struct sock *nlsk,
38963908
return err;
38973909
}
38983910

3911+
/**
3912+
* nft_data_hold - hold a nft_data item
3913+
*
3914+
* @data: struct nft_data to release
3915+
* @type: type of data
3916+
*
3917+
* Hold a nft_data item. NFT_DATA_VALUE types can be silently discarded,
3918+
* NFT_DATA_VERDICT bumps the reference to chains in case of NFT_JUMP and
3919+
* NFT_GOTO verdicts. This function must be called on active data objects
3920+
* from the second phase of the commit protocol.
3921+
*/
3922+
static void nft_data_hold(const struct nft_data *data, enum nft_data_types type)
3923+
{
3924+
if (type == NFT_DATA_VERDICT) {
3925+
switch (data->verdict.code) {
3926+
case NFT_JUMP:
3927+
case NFT_GOTO:
3928+
data->verdict.chain->use++;
3929+
break;
3930+
}
3931+
}
3932+
}
3933+
3934+
static void nft_set_elem_activate(const struct net *net,
3935+
const struct nft_set *set,
3936+
struct nft_set_elem *elem)
3937+
{
3938+
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
3939+
3940+
if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
3941+
nft_data_hold(nft_set_ext_data(ext), set->dtype);
3942+
if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
3943+
(*nft_set_ext_obj(ext))->use++;
3944+
}
3945+
3946+
static void nft_set_elem_deactivate(const struct net *net,
3947+
const struct nft_set *set,
3948+
struct nft_set_elem *elem)
3949+
{
3950+
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
3951+
3952+
if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
3953+
nft_data_release(nft_set_ext_data(ext), set->dtype);
3954+
if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
3955+
(*nft_set_ext_obj(ext))->use--;
3956+
}
3957+
38993958
static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
39003959
const struct nlattr *attr)
39013960
{
@@ -3961,6 +4020,8 @@ static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
39614020
kfree(elem.priv);
39624021
elem.priv = priv;
39634022

4023+
nft_set_elem_deactivate(ctx->net, set, &elem);
4024+
39644025
nft_trans_elem(trans) = elem;
39654026
list_add_tail(&trans->list, &ctx->net->nft.commit_list);
39664027
return 0;
@@ -3970,7 +4031,7 @@ static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
39704031
err3:
39714032
kfree(elem.priv);
39724033
err2:
3973-
nft_data_uninit(&elem.key.val, desc.type);
4034+
nft_data_release(&elem.key.val, desc.type);
39744035
err1:
39754036
return err;
39764037
}
@@ -4777,8 +4838,8 @@ static void nf_tables_commit_release(struct nft_trans *trans)
47774838
nft_set_destroy(nft_trans_set(trans));
47784839
break;
47794840
case NFT_MSG_DELSETELEM:
4780-
nft_set_elem_destroy(nft_trans_elem_set(trans),
4781-
nft_trans_elem(trans).priv, true);
4841+
nf_tables_set_elem_destroy(nft_trans_elem_set(trans),
4842+
nft_trans_elem(trans).priv);
47824843
break;
47834844
case NFT_MSG_DELOBJ:
47844845
nft_obj_destroy(nft_trans_obj(trans));
@@ -5013,6 +5074,7 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb)
50135074
case NFT_MSG_DELSETELEM:
50145075
te = (struct nft_trans_elem *)trans->data;
50155076

5077+
nft_set_elem_activate(net, te->set, &te->elem);
50165078
te->set->ops->activate(net, te->set, &te->elem);
50175079
te->set->ndeact--;
50185080

@@ -5498,15 +5560,15 @@ int nft_data_init(const struct nft_ctx *ctx,
54985560
EXPORT_SYMBOL_GPL(nft_data_init);
54995561

55005562
/**
5501-
* nft_data_uninit - release a nft_data item
5563+
* nft_data_release - release a nft_data item
55025564
*
55035565
* @data: struct nft_data to release
55045566
* @type: type of data
55055567
*
55065568
* Release a nft_data item. NFT_DATA_VALUE types can be silently discarded,
55075569
* all others need to be released by calling this function.
55085570
*/
5509-
void nft_data_uninit(const struct nft_data *data, enum nft_data_types type)
5571+
void nft_data_release(const struct nft_data *data, enum nft_data_types type)
55105572
{
55115573
if (type < NFT_DATA_VERDICT)
55125574
return;
@@ -5517,7 +5579,7 @@ void nft_data_uninit(const struct nft_data *data, enum nft_data_types type)
55175579
WARN_ON(1);
55185580
}
55195581
}
5520-
EXPORT_SYMBOL_GPL(nft_data_uninit);
5582+
EXPORT_SYMBOL_GPL(nft_data_release);
55215583

55225584
int nft_data_dump(struct sk_buff *skb, int attr, const struct nft_data *data,
55235585
enum nft_data_types type, unsigned int len)

Diff for: ‎net/netfilter/nft_bitwise.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ static int nft_bitwise_init(const struct nft_ctx *ctx,
9999

100100
return 0;
101101
err2:
102-
nft_data_uninit(&priv->xor, d2.type);
102+
nft_data_release(&priv->xor, d2.type);
103103
err1:
104-
nft_data_uninit(&priv->mask, d1.type);
104+
nft_data_release(&priv->mask, d1.type);
105105
return err;
106106
}
107107

Diff for: ‎net/netfilter/nft_cmp.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ nft_cmp_select_ops(const struct nft_ctx *ctx, const struct nlattr * const tb[])
211211

212212
return &nft_cmp_ops;
213213
err1:
214-
nft_data_uninit(&data, desc.type);
214+
nft_data_release(&data, desc.type);
215215
return ERR_PTR(-EINVAL);
216216
}
217217

Diff for: ‎net/netfilter/nft_immediate.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,16 @@ static int nft_immediate_init(const struct nft_ctx *ctx,
6565
return 0;
6666

6767
err1:
68-
nft_data_uninit(&priv->data, desc.type);
68+
nft_data_release(&priv->data, desc.type);
6969
return err;
7070
}
7171

7272
static void nft_immediate_destroy(const struct nft_ctx *ctx,
7373
const struct nft_expr *expr)
7474
{
7575
const struct nft_immediate_expr *priv = nft_expr_priv(expr);
76-
return nft_data_uninit(&priv->data, nft_dreg_to_type(priv->dreg));
76+
77+
return nft_data_release(&priv->data, nft_dreg_to_type(priv->dreg));
7778
}
7879

7980
static int nft_immediate_dump(struct sk_buff *skb, const struct nft_expr *expr)

Diff for: ‎net/netfilter/nft_range.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ static int nft_range_init(const struct nft_ctx *ctx, const struct nft_expr *expr
102102
priv->len = desc_from.len;
103103
return 0;
104104
err2:
105-
nft_data_uninit(&priv->data_to, desc_to.type);
105+
nft_data_release(&priv->data_to, desc_to.type);
106106
err1:
107-
nft_data_uninit(&priv->data_from, desc_from.type);
107+
nft_data_release(&priv->data_from, desc_from.type);
108108
return err;
109109
}
110110

0 commit comments

Comments
 (0)