Skip to content

Commit e358654

Browse files
fw-strlenfw-strlen
committed
netfilter: nf_tables: honor set timeout and garbage collection updates
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2164485 Upstream Status: commit 123b996 Conflicts: net/netfilter/nf_tables_api.c RHEL9 intentionally lacks 33c7aba ("netfilter: nf_tables: do not set up extensions for end interval"), because this commit triggers erroneous rejects of new set elements from the nft_set_rbtree backend. The existing overlap detection depends on timeout extension being set also for the end interval. This can be backported later once the new rbtree overlap detection has had some test exposure upstream. IOW, keep the RHEL9 conditional to add timeout extension to end intervals too. commit 123b996 Author: Pablo Neira Ayuso <pablo@netfilter.org> Date: Mon Dec 19 20:10:12 2022 +0100 netfilter: nf_tables: honor set timeout and garbage collection updates Set timeout and garbage collection interval updates are ignored on updates. Add transaction to update global set element timeout and garbage collection interval. Fixes: 9651851 ("netfilter: add nftables") Suggested-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Florian Westphal <fwestpha@redhat.com>
1 parent 17374d1 commit e358654

File tree

2 files changed

+57
-19
lines changed

2 files changed

+57
-19
lines changed

‎include/net/netfilter/nf_tables.h‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,9 @@ void *nft_set_catchall_gc(const struct nft_set *set);
560560

561561
static inline unsigned long nft_set_gc_interval(const struct nft_set *set)
562562
{
563-
return set->gc_int ? msecs_to_jiffies(set->gc_int) : HZ;
563+
u32 gc_int = READ_ONCE(set->gc_int);
564+
565+
return gc_int ? msecs_to_jiffies(gc_int) : HZ;
564566
}
565567

566568
/**
@@ -1513,6 +1515,9 @@ struct nft_trans_rule {
15131515
struct nft_trans_set {
15141516
struct nft_set *set;
15151517
u32 set_id;
1518+
u32 gc_int;
1519+
u64 timeout;
1520+
bool update;
15161521
bool bound;
15171522
};
15181523

@@ -1522,6 +1527,12 @@ struct nft_trans_set {
15221527
(((struct nft_trans_set *)trans->data)->set_id)
15231528
#define nft_trans_set_bound(trans) \
15241529
(((struct nft_trans_set *)trans->data)->bound)
1530+
#define nft_trans_set_update(trans) \
1531+
(((struct nft_trans_set *)trans->data)->update)
1532+
#define nft_trans_set_timeout(trans) \
1533+
(((struct nft_trans_set *)trans->data)->timeout)
1534+
#define nft_trans_set_gc_int(trans) \
1535+
(((struct nft_trans_set *)trans->data)->gc_int)
15251536

15261537
struct nft_trans_chain {
15271538
bool update;

‎net/netfilter/nf_tables_api.c‎

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -465,26 +465,38 @@ static int nft_delrule_by_chain(struct nft_ctx *ctx)
465465
return 0;
466466
}
467467

468-
static int nft_trans_set_add(const struct nft_ctx *ctx, int msg_type,
469-
struct nft_set *set)
468+
static int __nft_trans_set_add(const struct nft_ctx *ctx, int msg_type,
469+
struct nft_set *set,
470+
const struct nft_set_desc *desc)
470471
{
471472
struct nft_trans *trans;
472473

473474
trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_set));
474475
if (trans == NULL)
475476
return -ENOMEM;
476477

477-
if (msg_type == NFT_MSG_NEWSET && ctx->nla[NFTA_SET_ID] != NULL) {
478+
if (msg_type == NFT_MSG_NEWSET && ctx->nla[NFTA_SET_ID] && !desc) {
478479
nft_trans_set_id(trans) =
479480
ntohl(nla_get_be32(ctx->nla[NFTA_SET_ID]));
480481
nft_activate_next(ctx->net, set);
481482
}
482483
nft_trans_set(trans) = set;
484+
if (desc) {
485+
nft_trans_set_update(trans) = true;
486+
nft_trans_set_gc_int(trans) = desc->gc_int;
487+
nft_trans_set_timeout(trans) = desc->timeout;
488+
}
483489
nft_trans_commit_list_add_tail(ctx->net, trans);
484490

485491
return 0;
486492
}
487493

494+
static int nft_trans_set_add(const struct nft_ctx *ctx, int msg_type,
495+
struct nft_set *set)
496+
{
497+
return __nft_trans_set_add(ctx, msg_type, set, NULL);
498+
}
499+
488500
static int nft_delset(const struct nft_ctx *ctx, struct nft_set *set)
489501
{
490502
int err;
@@ -3899,8 +3911,10 @@ static int nf_tables_fill_set_concat(struct sk_buff *skb,
38993911
static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
39003912
const struct nft_set *set, u16 event, u16 flags)
39013913
{
3902-
struct nlmsghdr *nlh;
3914+
u64 timeout = READ_ONCE(set->timeout);
3915+
u32 gc_int = READ_ONCE(set->gc_int);
39033916
u32 portid = ctx->portid;
3917+
struct nlmsghdr *nlh;
39043918
struct nlattr *nest;
39053919
u32 seq = ctx->seq;
39063920
int i;
@@ -3936,13 +3950,13 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
39363950
nla_put_be32(skb, NFTA_SET_OBJ_TYPE, htonl(set->objtype)))
39373951
goto nla_put_failure;
39383952

3939-
if (set->timeout &&
3953+
if (timeout &&
39403954
nla_put_be64(skb, NFTA_SET_TIMEOUT,
3941-
nf_jiffies64_to_msecs(set->timeout),
3955+
nf_jiffies64_to_msecs(timeout),
39423956
NFTA_SET_PAD))
39433957
goto nla_put_failure;
3944-
if (set->gc_int &&
3945-
nla_put_be32(skb, NFTA_SET_GC_INTERVAL, htonl(set->gc_int)))
3958+
if (gc_int &&
3959+
nla_put_be32(skb, NFTA_SET_GC_INTERVAL, htonl(gc_int)))
39463960
goto nla_put_failure;
39473961

39483962
if (set->policy != NFT_SET_POL_PERFORMANCE) {
@@ -4487,7 +4501,10 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,
44874501
for (i = 0; i < num_exprs; i++)
44884502
nft_expr_destroy(&ctx, exprs[i]);
44894503

4490-
return err;
4504+
if (err < 0)
4505+
return err;
4506+
4507+
return __nft_trans_set_add(&ctx, NFT_MSG_NEWSET, set, &desc);
44914508
}
44924509

44934510
if (!(info->nlh->nlmsg_flags & NLM_F_CREATE))
@@ -5890,7 +5907,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
58905907
if (err)
58915908
return err;
58925909
} else if (set->flags & NFT_SET_TIMEOUT) {
5893-
timeout = set->timeout;
5910+
timeout = READ_ONCE(set->timeout);
58945911
}
58955912

58965913
expiration = 0;
@@ -5990,7 +6007,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
59906007
if (err < 0)
59916008
goto err_parse_key_end;
59926009

5993-
if (timeout != set->timeout) {
6010+
if (timeout != READ_ONCE(set->timeout)) {
59946011
err = nft_set_ext_add(&tmpl, NFT_SET_EXT_TIMEOUT);
59956012
if (err < 0)
59966013
goto err_parse_key_end;
@@ -8845,14 +8862,20 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
88458862
nft_flow_rule_destroy(nft_trans_flow_rule(trans));
88468863
break;
88478864
case NFT_MSG_NEWSET:
8848-
nft_clear(net, nft_trans_set(trans));
8849-
/* This avoids hitting -EBUSY when deleting the table
8850-
* from the transaction.
8851-
*/
8852-
if (nft_set_is_anonymous(nft_trans_set(trans)) &&
8853-
!list_empty(&nft_trans_set(trans)->bindings))
8854-
trans->ctx.table->use--;
8865+
if (nft_trans_set_update(trans)) {
8866+
struct nft_set *set = nft_trans_set(trans);
88558867

8868+
WRITE_ONCE(set->timeout, nft_trans_set_timeout(trans));
8869+
WRITE_ONCE(set->gc_int, nft_trans_set_gc_int(trans));
8870+
} else {
8871+
nft_clear(net, nft_trans_set(trans));
8872+
/* This avoids hitting -EBUSY when deleting the table
8873+
* from the transaction.
8874+
*/
8875+
if (nft_set_is_anonymous(nft_trans_set(trans)) &&
8876+
!list_empty(&nft_trans_set(trans)->bindings))
8877+
trans->ctx.table->use--;
8878+
}
88568879
nf_tables_set_notify(&trans->ctx, nft_trans_set(trans),
88578880
NFT_MSG_NEWSET, GFP_KERNEL);
88588881
nft_trans_destroy(trans);
@@ -9074,6 +9097,10 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
90749097
nft_trans_destroy(trans);
90759098
break;
90769099
case NFT_MSG_NEWSET:
9100+
if (nft_trans_set_update(trans)) {
9101+
nft_trans_destroy(trans);
9102+
break;
9103+
}
90779104
trans->ctx.table->use--;
90789105
if (nft_trans_set_bound(trans)) {
90799106
nft_trans_destroy(trans);

0 commit comments

Comments
 (0)