* [PATCH nf] netfilter: nf_tables: fix oops during rule dump [not found] <netfilter-devel> @ 2019-04-30 12:53 ` Florian Westphal 2019-05-01 3:54 ` Taehee Yoo 2019-05-20 19:21 ` Pablo Neira Ayuso 2022-09-06 15:20 ` [PATCH nf-next 0/2] netfilter: nat: avoid long-running loops Florian Westphal 2023-10-06 9:27 ` [PATCH nf-next] netfilter: conntrack: simplify nf_conntrack_alter_reply Florian Westphal 2 siblings, 2 replies; 7+ messages in thread From: Florian Westphal @ 2019-04-30 12:53 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal We can oops in nf_tables_fill_rule_info(). Its not possible to fetch previous element in rcu-protected lists when deletions are not prevented somehow: list_del_rcu poisons the ->prev pointer value. Before rcu-conversion this was safe as dump operations did hold nfnetlink mutex. Pass previous rule as argument, obtained by keeping a pointer to the previous rule during traversal. Fixes: d9adf22a291883 ("netfilter: nf_tables: use call_rcu in netlink dumps") Signed-off-by: Florian Westphal <fw@strlen.de> --- net/netfilter/nf_tables_api.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index aa5e7b00a581..0969f9647927 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -2261,13 +2261,13 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net, u32 flags, int family, const struct nft_table *table, const struct nft_chain *chain, - const struct nft_rule *rule) + const struct nft_rule *rule, + const struct nft_rule *prule) { struct nlmsghdr *nlh; struct nfgenmsg *nfmsg; const struct nft_expr *expr, *next; struct nlattr *list; - const struct nft_rule *prule; u16 type = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event); nlh = nlmsg_put(skb, portid, seq, type, sizeof(struct nfgenmsg), flags); @@ -2287,8 +2287,7 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net, NFTA_RULE_PAD)) goto nla_put_failure; - if ((event != NFT_MSG_DELRULE) && (rule->list.prev != &chain->rules)) { - prule = list_prev_entry(rule, list); + if (event != NFT_MSG_DELRULE && prule) { if (nla_put_be64(skb, NFTA_RULE_POSITION, cpu_to_be64(prule->handle), NFTA_RULE_PAD)) @@ -2335,7 +2334,7 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx, err = nf_tables_fill_rule_info(skb, ctx->net, ctx->portid, ctx->seq, event, 0, ctx->family, ctx->table, - ctx->chain, rule); + ctx->chain, rule, NULL); if (err < 0) { kfree_skb(skb); goto err; @@ -2360,12 +2359,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, const struct nft_chain *chain) { struct net *net = sock_net(skb->sk); + const struct nft_rule *rule, *prule; unsigned int s_idx = cb->args[0]; - const struct nft_rule *rule; + prule = NULL; list_for_each_entry_rcu(rule, &chain->rules, list) { if (!nft_is_active(net, rule)) - goto cont; + goto cont_skip; if (*idx < s_idx) goto cont; if (*idx > s_idx) { @@ -2377,11 +2377,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, NFT_MSG_NEWRULE, NLM_F_MULTI | NLM_F_APPEND, table->family, - table, chain, rule) < 0) + table, chain, rule, prule) < 0) return 1; nl_dump_check_consistent(cb, nlmsg_hdr(skb)); cont: + prule = rule; +cont_skip: (*idx)++; } return 0; @@ -2537,7 +2539,7 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk, err = nf_tables_fill_rule_info(skb2, net, NETLINK_CB(skb).portid, nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0, - family, table, chain, rule); + family, table, chain, rule, NULL); if (err < 0) goto err; -- 2.21.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: fix oops during rule dump 2019-04-30 12:53 ` [PATCH nf] netfilter: nf_tables: fix oops during rule dump Florian Westphal @ 2019-05-01 3:54 ` Taehee Yoo 2019-05-20 19:21 ` Pablo Neira Ayuso 1 sibling, 0 replies; 7+ messages in thread From: Taehee Yoo @ 2019-05-01 3:54 UTC (permalink / raw) To: Florian Westphal; +Cc: Netfilter Developer Mailing List On Tue, 30 Apr 2019 at 21:52, Florian Westphal <fw@strlen.de> wrote: > > We can oops in nf_tables_fill_rule_info(). > > Its not possible to fetch previous element in rcu-protected lists > when deletions are not prevented somehow: list_del_rcu poisons > the ->prev pointer value. > > Before rcu-conversion this was safe as dump operations did hold > nfnetlink mutex. > > Pass previous rule as argument, obtained by keeping a pointer to > the previous rule during traversal. > Hi Florian, I have tested this patch and I think this patch works well. I would like to share my test method. SHELL#1 while : do nft flush ruleset nft -f test.nft done SHELL#2 while : do nft list ruleset -a iptables-nft -vnL done This test method could make panic. [ 1887.092089] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI [ 1887.100127] CPU: 0 PID: 1652 Comm: nft Not tainted 5.1.0-rc5+ #76 [ 1887.106982] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/08/2015 [ 1887.117769] RIP: 0010:nf_tables_fill_rule_info.isra.59+0x362/0x860 [nf_tables] [ 1887.125892] Code: 8b 84 24 d8 00 00 00 4d 8b 65 08 48 83 c0 10 49 39 c4 74 5a 49 8d 7c 24 10 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 9c 04 00 00 48 b8 ff ff ff ff ff 03 00 00 49 23 [ 1887.146996] RSP: 0018:ffff88810c0872a0 EFLAGS: 00010206 [ 1887.152878] RAX: dffffc0000000000 RBX: ffff88810ea0b1c0 RCX: 0000000000000000 [ 1887.160894] RDX: 1bd5a00000000042 RSI: ffff88810c0872f8 RDI: dead000000000210 [ 1887.168909] RBP: ffff8880b4b78000 R08: ffffed101696f008 R09: ffffed101696f008 [ 1887.176933] R10: 0000000000000002 R11: ffffed101696f007 R12: dead000000000200 [ 1887.184957] R13: ffff8880b8142688 R14: ffff8880b8142698 R15: ffff88810c0872f0 [ 1887.192973] FS: 00007fb475d8a740(0000) GS:ffff888116600000(0000) knlGS:0000000000000000 [ 1887.202064] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1887.208527] CR2: 0000557d208c4088 CR3: 00000001102de000 CR4: 00000000001006f0 [ 1887.216548] Call Trace: [ 1887.219344] ? nft_expr_dump+0x4c0/0x4c0 [nf_tables] [ 1887.224947] ? debug_show_all_locks+0x2d0/0x2d0 [ 1887.230055] ? rcu_read_lock_sched_held+0x114/0x130 [ 1887.235573] __nf_tables_dump_rules+0x27d/0x620 [nf_tables] [ 1887.241892] nf_tables_dump_rules+0x4c4/0xc80 [nf_tables] [ 1887.247967] ? rcu_read_lock_sched_held+0x114/0x130 [ 1887.253466] ? __kmalloc_node_track_caller+0x321/0x360 [ 1887.259285] ? __nf_tables_dump_rules+0x620/0x620 [nf_tables] [ 1887.265759] ? __alloc_skb+0x314/0x500 [ 1887.269990] ? sock_spd_release+0xf0/0xf0 [ 1887.274514] ? check_flags.part.41+0x440/0x440 [ 1887.279532] netlink_dump+0x476/0xea0 [ 1887.283669] ? __netlink_sendskb+0xc0/0xc0 [ 1887.288283] ? __netlink_dump_start+0x165/0x750 [ 1887.293408] __netlink_dump_start+0x537/0x750 [ 1887.298345] nft_netlink_dump_start_rcu.constprop.71+0x97/0x180 [nf_tables] [ 1887.306205] nf_tables_getrule+0x3b6/0x620 [nf_tables] [ 1887.312019] ? nf_tables_rule_notify+0x3e0/0x3e0 [nf_tables] [ 1887.318407] ? nf_tables_dump_obj_start+0x230/0x230 [nf_tables] [ 1887.325086] ? __nf_tables_dump_rules+0x620/0x620 [nf_tables] [ 1887.331569] ? nf_tables_dump_sets_done+0x40/0x40 [nf_tables] [ 1887.338045] ? __nla_parse+0x34/0x310 [ 1887.342198] ? nf_tables_rule_notify+0x3e0/0x3e0 [nf_tables] [ 1887.348576] nfnetlink_rcv_msg+0x3f0/0xd0b [nfnetlink] [ 1887.354366] ? kernel_text_address+0x111/0x120 [ 1887.359391] ? nfnetlink_bind+0x200/0x200 [nfnetlink] [ 1887.365097] ? sched_clock_cpu+0x18/0x170 [ 1887.369616] ? __sys_sendto+0x263/0x300 [ 1887.373929] ? sched_clock_cpu+0x18/0x170 [ 1887.378447] ? do_syscall_64+0x9c/0x3e0 [ 1887.382779] ? __lock_acquire+0xe7c/0x3bd0 [ 1887.387403] ? check_flags.part.41+0x440/0x440 [ 1887.392416] netlink_rcv_skb+0x112/0x330 [ 1887.396841] ? nfnetlink_bind+0x200/0x200 [nfnetlink] [ 1887.402538] ? netlink_ack+0x940/0x940 [ 1887.406776] ? ns_capable_common+0x5c/0xd0 [ 1887.411405] nfnetlink_rcv+0x161/0x320 [nfnetlink] [ 1887.416809] ? nfnetlink_rcv_batch+0x1280/0x1280 [nfnetlink] [ 1887.423200] netlink_unicast+0x3ee/0x5a0 After this patch, I couldn't see this panic message. Thanks! > Fixes: d9adf22a291883 ("netfilter: nf_tables: use call_rcu in netlink dumps") > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > net/netfilter/nf_tables_api.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index aa5e7b00a581..0969f9647927 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -2261,13 +2261,13 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net, > u32 flags, int family, > const struct nft_table *table, > const struct nft_chain *chain, > - const struct nft_rule *rule) > + const struct nft_rule *rule, > + const struct nft_rule *prule) > { > struct nlmsghdr *nlh; > struct nfgenmsg *nfmsg; > const struct nft_expr *expr, *next; > struct nlattr *list; > - const struct nft_rule *prule; > u16 type = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event); > > nlh = nlmsg_put(skb, portid, seq, type, sizeof(struct nfgenmsg), flags); > @@ -2287,8 +2287,7 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net, > NFTA_RULE_PAD)) > goto nla_put_failure; > > - if ((event != NFT_MSG_DELRULE) && (rule->list.prev != &chain->rules)) { > - prule = list_prev_entry(rule, list); > + if (event != NFT_MSG_DELRULE && prule) { > if (nla_put_be64(skb, NFTA_RULE_POSITION, > cpu_to_be64(prule->handle), > NFTA_RULE_PAD)) > @@ -2335,7 +2334,7 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx, > > err = nf_tables_fill_rule_info(skb, ctx->net, ctx->portid, ctx->seq, > event, 0, ctx->family, ctx->table, > - ctx->chain, rule); > + ctx->chain, rule, NULL); > if (err < 0) { > kfree_skb(skb); > goto err; > @@ -2360,12 +2359,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, > const struct nft_chain *chain) > { > struct net *net = sock_net(skb->sk); > + const struct nft_rule *rule, *prule; > unsigned int s_idx = cb->args[0]; > - const struct nft_rule *rule; > > + prule = NULL; > list_for_each_entry_rcu(rule, &chain->rules, list) { > if (!nft_is_active(net, rule)) > - goto cont; > + goto cont_skip; > if (*idx < s_idx) > goto cont; > if (*idx > s_idx) { > @@ -2377,11 +2377,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, > NFT_MSG_NEWRULE, > NLM_F_MULTI | NLM_F_APPEND, > table->family, > - table, chain, rule) < 0) > + table, chain, rule, prule) < 0) > return 1; > > nl_dump_check_consistent(cb, nlmsg_hdr(skb)); > cont: > + prule = rule; > +cont_skip: > (*idx)++; > } > return 0; > @@ -2537,7 +2539,7 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk, > > err = nf_tables_fill_rule_info(skb2, net, NETLINK_CB(skb).portid, > nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0, > - family, table, chain, rule); > + family, table, chain, rule, NULL); > if (err < 0) > goto err; > > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: fix oops during rule dump 2019-04-30 12:53 ` [PATCH nf] netfilter: nf_tables: fix oops during rule dump Florian Westphal 2019-05-01 3:54 ` Taehee Yoo @ 2019-05-20 19:21 ` Pablo Neira Ayuso 1 sibling, 0 replies; 7+ messages in thread From: Pablo Neira Ayuso @ 2019-05-20 19:21 UTC (permalink / raw) To: Florian Westphal; +Cc: netfilter-devel On Tue, Apr 30, 2019 at 02:53:11PM +0200, Florian Westphal wrote: > We can oops in nf_tables_fill_rule_info(). > > Its not possible to fetch previous element in rcu-protected lists > when deletions are not prevented somehow: list_del_rcu poisons > the ->prev pointer value. > > Before rcu-conversion this was safe as dump operations did hold > nfnetlink mutex. > > Pass previous rule as argument, obtained by keeping a pointer to > the previous rule during traversal. Applied, thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH nf-next 0/2] netfilter: nat: avoid long-running loops [not found] <netfilter-devel> 2019-04-30 12:53 ` [PATCH nf] netfilter: nf_tables: fix oops during rule dump Florian Westphal @ 2022-09-06 15:20 ` Florian Westphal 2022-09-06 15:20 ` [PATCH nf-next 1/2] netfilter: nat: move repetitive nat port reserve loop to a helper Florian Westphal 2022-09-06 15:20 ` [PATCH nf-next 2/2] netfilter: nat: avoid long-running port range loop Florian Westphal 2023-10-06 9:27 ` [PATCH nf-next] netfilter: conntrack: simplify nf_conntrack_alter_reply Florian Westphal 2 siblings, 2 replies; 7+ messages in thread From: Florian Westphal @ 2022-09-06 15:20 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal If a majority of ports are in use, trying every available port may take significant amounts of time. Add a upper limit and cancel once we've exhausted all available options. First patch collapses the repetitive reserve-port loop into a helper, second patch changes the helper to only make up to 128 attempts. Florian Westphal (2): netfilter: nat: move repetitive nat port reserve loop to a helper netfilter: nat: avoid long-running port range loop include/net/netfilter/nf_nat_helper.h | 1 + net/ipv4/netfilter/nf_nat_h323.c | 60 ++------------------------- net/netfilter/nf_nat_amanda.c | 14 +------ net/netfilter/nf_nat_ftp.c | 17 +------- net/netfilter/nf_nat_helper.c | 31 ++++++++++++++ net/netfilter/nf_nat_irc.c | 16 +------ net/netfilter/nf_nat_sip.c | 14 +------ 7 files changed, 42 insertions(+), 111 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH nf-next 1/2] netfilter: nat: move repetitive nat port reserve loop to a helper 2022-09-06 15:20 ` [PATCH nf-next 0/2] netfilter: nat: avoid long-running loops Florian Westphal @ 2022-09-06 15:20 ` Florian Westphal 2022-09-06 15:20 ` [PATCH nf-next 2/2] netfilter: nat: avoid long-running port range loop Florian Westphal 1 sibling, 0 replies; 7+ messages in thread From: Florian Westphal @ 2022-09-06 15:20 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal, Pablo Neira Ayuso Almost all nat helpers reserve an expecation port the same way: Try the port inidcated by the peer, then move to next port if that port is already in use. We can squash this into a helper. Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Florian Westphal <fw@strlen.de> --- include/net/netfilter/nf_nat_helper.h | 1 + net/ipv4/netfilter/nf_nat_h323.c | 60 ++------------------------- net/netfilter/nf_nat_amanda.c | 14 +------ net/netfilter/nf_nat_ftp.c | 17 +------- net/netfilter/nf_nat_helper.c | 19 +++++++++ net/netfilter/nf_nat_irc.c | 16 +------ net/netfilter/nf_nat_sip.c | 14 +------ 7 files changed, 30 insertions(+), 111 deletions(-) diff --git a/include/net/netfilter/nf_nat_helper.h b/include/net/netfilter/nf_nat_helper.h index efae84646353..44c421b9be85 100644 --- a/include/net/netfilter/nf_nat_helper.h +++ b/include/net/netfilter/nf_nat_helper.h @@ -38,4 +38,5 @@ bool nf_nat_mangle_udp_packet(struct sk_buff *skb, struct nf_conn *ct, * to port ct->master->saved_proto. */ void nf_nat_follow_master(struct nf_conn *ct, struct nf_conntrack_expect *this); +u16 nf_nat_exp_find_port(struct nf_conntrack_expect *exp, u16 port); #endif diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c index a334f0dcc2d0..faee20af4856 100644 --- a/net/ipv4/netfilter/nf_nat_h323.c +++ b/net/ipv4/netfilter/nf_nat_h323.c @@ -291,20 +291,7 @@ static int nat_t120(struct sk_buff *skb, struct nf_conn *ct, exp->expectfn = nf_nat_follow_master; exp->dir = !dir; - /* Try to get same port: if not, try to change it. */ - for (; nated_port != 0; nated_port++) { - int ret; - - exp->tuple.dst.u.tcp.port = htons(nated_port); - ret = nf_ct_expect_related(exp, 0); - if (ret == 0) - break; - else if (ret != -EBUSY) { - nated_port = 0; - break; - } - } - + nated_port = nf_nat_exp_find_port(exp, nated_port); if (nated_port == 0) { /* No port available */ net_notice_ratelimited("nf_nat_h323: out of TCP ports\n"); return 0; @@ -347,20 +334,7 @@ static int nat_h245(struct sk_buff *skb, struct nf_conn *ct, if (info->sig_port[dir] == port) nated_port = ntohs(info->sig_port[!dir]); - /* Try to get same port: if not, try to change it. */ - for (; nated_port != 0; nated_port++) { - int ret; - - exp->tuple.dst.u.tcp.port = htons(nated_port); - ret = nf_ct_expect_related(exp, 0); - if (ret == 0) - break; - else if (ret != -EBUSY) { - nated_port = 0; - break; - } - } - + nated_port = nf_nat_exp_find_port(exp, nated_port); if (nated_port == 0) { /* No port available */ net_notice_ratelimited("nf_nat_q931: out of TCP ports\n"); return 0; @@ -439,20 +413,7 @@ static int nat_q931(struct sk_buff *skb, struct nf_conn *ct, if (info->sig_port[dir] == port) nated_port = ntohs(info->sig_port[!dir]); - /* Try to get same port: if not, try to change it. */ - for (; nated_port != 0; nated_port++) { - int ret; - - exp->tuple.dst.u.tcp.port = htons(nated_port); - ret = nf_ct_expect_related(exp, 0); - if (ret == 0) - break; - else if (ret != -EBUSY) { - nated_port = 0; - break; - } - } - + nated_port = nf_nat_exp_find_port(exp, nated_port); if (nated_port == 0) { /* No port available */ net_notice_ratelimited("nf_nat_ras: out of TCP ports\n"); return 0; @@ -532,20 +493,7 @@ static int nat_callforwarding(struct sk_buff *skb, struct nf_conn *ct, exp->expectfn = ip_nat_callforwarding_expect; exp->dir = !dir; - /* Try to get same port: if not, try to change it. */ - for (nated_port = ntohs(port); nated_port != 0; nated_port++) { - int ret; - - exp->tuple.dst.u.tcp.port = htons(nated_port); - ret = nf_ct_expect_related(exp, 0); - if (ret == 0) - break; - else if (ret != -EBUSY) { - nated_port = 0; - break; - } - } - + nated_port = nf_nat_exp_find_port(exp, ntohs(port)); if (nated_port == 0) { /* No port available */ net_notice_ratelimited("nf_nat_q931: out of TCP ports\n"); return 0; diff --git a/net/netfilter/nf_nat_amanda.c b/net/netfilter/nf_nat_amanda.c index 3bc7e0854efe..98deef6cde69 100644 --- a/net/netfilter/nf_nat_amanda.c +++ b/net/netfilter/nf_nat_amanda.c @@ -44,19 +44,7 @@ static unsigned int help(struct sk_buff *skb, exp->expectfn = nf_nat_follow_master; /* Try to get same port: if not, try to change it. */ - for (port = ntohs(exp->saved_proto.tcp.port); port != 0; port++) { - int res; - - exp->tuple.dst.u.tcp.port = htons(port); - res = nf_ct_expect_related(exp, 0); - if (res == 0) - break; - else if (res != -EBUSY) { - port = 0; - break; - } - } - + port = nf_nat_exp_find_port(exp, ntohs(exp->saved_proto.tcp.port)); if (port == 0) { nf_ct_helper_log(skb, exp->master, "all ports in use"); return NF_DROP; diff --git a/net/netfilter/nf_nat_ftp.c b/net/netfilter/nf_nat_ftp.c index aace6768a64e..c92a436d9c48 100644 --- a/net/netfilter/nf_nat_ftp.c +++ b/net/netfilter/nf_nat_ftp.c @@ -86,22 +86,9 @@ static unsigned int nf_nat_ftp(struct sk_buff *skb, * this one. */ exp->expectfn = nf_nat_follow_master; - /* Try to get same port: if not, try to change it. */ - for (port = ntohs(exp->saved_proto.tcp.port); port != 0; port++) { - int ret; - - exp->tuple.dst.u.tcp.port = htons(port); - ret = nf_ct_expect_related(exp, 0); - if (ret == 0) - break; - else if (ret != -EBUSY) { - port = 0; - break; - } - } - + port = nf_nat_exp_find_port(exp, ntohs(exp->saved_proto.tcp.port)); if (port == 0) { - nf_ct_helper_log(skb, ct, "all ports in use"); + nf_ct_helper_log(skb, exp->master, "all ports in use"); return NF_DROP; } diff --git a/net/netfilter/nf_nat_helper.c b/net/netfilter/nf_nat_helper.c index a263505455fc..067d6d6f6b7d 100644 --- a/net/netfilter/nf_nat_helper.c +++ b/net/netfilter/nf_nat_helper.c @@ -198,3 +198,22 @@ void nf_nat_follow_master(struct nf_conn *ct, nf_nat_setup_info(ct, &range, NF_NAT_MANIP_DST); } EXPORT_SYMBOL(nf_nat_follow_master); + +u16 nf_nat_exp_find_port(struct nf_conntrack_expect *exp, u16 port) +{ + /* Try to get same port: if not, try to change it. */ + for (; port != 0; port++) { + int res; + + exp->tuple.dst.u.tcp.port = htons(port); + res = nf_ct_expect_related(exp, 0); + if (res == 0) + return port; + + if (res != -EBUSY) + break; + } + + return 0; +} +EXPORT_SYMBOL_GPL(nf_nat_exp_find_port); diff --git a/net/netfilter/nf_nat_irc.c b/net/netfilter/nf_nat_irc.c index c691ab8d234c..19c4fcc60c50 100644 --- a/net/netfilter/nf_nat_irc.c +++ b/net/netfilter/nf_nat_irc.c @@ -48,20 +48,8 @@ static unsigned int help(struct sk_buff *skb, exp->dir = IP_CT_DIR_REPLY; exp->expectfn = nf_nat_follow_master; - /* Try to get same port: if not, try to change it. */ - for (port = ntohs(exp->saved_proto.tcp.port); port != 0; port++) { - int ret; - - exp->tuple.dst.u.tcp.port = htons(port); - ret = nf_ct_expect_related(exp, 0); - if (ret == 0) - break; - else if (ret != -EBUSY) { - port = 0; - break; - } - } - + port = nf_nat_exp_find_port(exp, + ntohs(exp->saved_proto.tcp.port)); if (port == 0) { nf_ct_helper_log(skb, ct, "all ports in use"); return NF_DROP; diff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c index f0a735e86851..cf4aeb299bde 100644 --- a/net/netfilter/nf_nat_sip.c +++ b/net/netfilter/nf_nat_sip.c @@ -410,19 +410,7 @@ static unsigned int nf_nat_sip_expect(struct sk_buff *skb, unsigned int protoff, exp->dir = !dir; exp->expectfn = nf_nat_sip_expected; - for (; port != 0; port++) { - int ret; - - exp->tuple.dst.u.udp.port = htons(port); - ret = nf_ct_expect_related(exp, NF_CT_EXP_F_SKIP_MASTER); - if (ret == 0) - break; - else if (ret != -EBUSY) { - port = 0; - break; - } - } - + port = nf_nat_exp_find_port(exp, port); if (port == 0) { nf_ct_helper_log(skb, ct, "all ports in use for SIP"); return NF_DROP; -- 2.35.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH nf-next 2/2] netfilter: nat: avoid long-running port range loop 2022-09-06 15:20 ` [PATCH nf-next 0/2] netfilter: nat: avoid long-running loops Florian Westphal 2022-09-06 15:20 ` [PATCH nf-next 1/2] netfilter: nat: move repetitive nat port reserve loop to a helper Florian Westphal @ 2022-09-06 15:20 ` Florian Westphal 1 sibling, 0 replies; 7+ messages in thread From: Florian Westphal @ 2022-09-06 15:20 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal Looping a large port range takes too long. Instead select a random offset within [ntohs(exp->saved_proto.tcp.port), 65535] and try 128 ports. This is a rehash of an erlier patch to do the same, but generalized to handle other helpers as well. Link: https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210920204439.13179-2-Cole.Dishington@alliedtelesis.co.nz/ Signed-off-by: Florian Westphal <fw@strlen.de> --- net/netfilter/nf_nat_helper.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/net/netfilter/nf_nat_helper.c b/net/netfilter/nf_nat_helper.c index 067d6d6f6b7d..a95a25196943 100644 --- a/net/netfilter/nf_nat_helper.c +++ b/net/netfilter/nf_nat_helper.c @@ -201,8 +201,18 @@ EXPORT_SYMBOL(nf_nat_follow_master); u16 nf_nat_exp_find_port(struct nf_conntrack_expect *exp, u16 port) { + static const unsigned int max_attempts = 128; + int range, attempts_left; + u16 min = port; + + range = USHRT_MAX - port; + attempts_left = range; + + if (attempts_left > max_attempts) + attempts_left = max_attempts; + /* Try to get same port: if not, try to change it. */ - for (; port != 0; port++) { + for (;;) { int res; exp->tuple.dst.u.tcp.port = htons(port); @@ -210,8 +220,10 @@ u16 nf_nat_exp_find_port(struct nf_conntrack_expect *exp, u16 port) if (res == 0) return port; - if (res != -EBUSY) + if (res != -EBUSY || (--attempts_left < 0)) break; + + port = min + prandom_u32_max(range); } return 0; -- 2.35.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH nf-next] netfilter: conntrack: simplify nf_conntrack_alter_reply [not found] <netfilter-devel> 2019-04-30 12:53 ` [PATCH nf] netfilter: nf_tables: fix oops during rule dump Florian Westphal 2022-09-06 15:20 ` [PATCH nf-next 0/2] netfilter: nat: avoid long-running loops Florian Westphal @ 2023-10-06 9:27 ` Florian Westphal 2 siblings, 0 replies; 7+ messages in thread From: Florian Westphal @ 2023-10-06 9:27 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal nf_conntrack_alter_reply doesn't do helper reassignment anymore. Remove the comments that make this claim. Furthermore, remove dead code from the function and place ot in nf_conntrack.h. Signed-off-by: Florian Westphal <fw@strlen.de> --- include/net/netfilter/nf_conntrack.h | 14 ++++++++++---- net/netfilter/nf_conntrack_core.c | 18 ------------------ net/netfilter/nf_conntrack_helper.c | 7 +------ 3 files changed, 11 insertions(+), 28 deletions(-) diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h index 4085765c3370..cba3ccf03fcc 100644 --- a/include/net/netfilter/nf_conntrack.h +++ b/include/net/netfilter/nf_conntrack.h @@ -160,10 +160,6 @@ static inline struct net *nf_ct_net(const struct nf_conn *ct) return read_pnet(&ct->ct_net); } -/* Alter reply tuple (maybe alter helper). */ -void nf_conntrack_alter_reply(struct nf_conn *ct, - const struct nf_conntrack_tuple *newreply); - /* Is this tuple taken? (ignoring any belonging to the given conntrack). */ int nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple, @@ -284,6 +280,16 @@ static inline bool nf_is_loopback_packet(const struct sk_buff *skb) return skb->dev && skb->skb_iif && skb->dev->flags & IFF_LOOPBACK; } +static inline void nf_conntrack_alter_reply(struct nf_conn *ct, + const struct nf_conntrack_tuple *newreply) +{ + /* Must be unconfirmed, so not in hash table yet */ + if (WARN_ON(nf_ct_is_confirmed(ct))) + return; + + ct->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply; +} + #define nfct_time_stamp ((u32)(jiffies)) /* jiffies until ct expires, 0 if already expired */ diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c index 9f6f2e643575..124136b5a79a 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -2042,24 +2042,6 @@ nf_conntrack_in(struct sk_buff *skb, const struct nf_hook_state *state) } EXPORT_SYMBOL_GPL(nf_conntrack_in); -/* Alter reply tuple (maybe alter helper). This is for NAT, and is - implicitly racy: see __nf_conntrack_confirm */ -void nf_conntrack_alter_reply(struct nf_conn *ct, - const struct nf_conntrack_tuple *newreply) -{ - struct nf_conn_help *help = nfct_help(ct); - - /* Should be unconfirmed, so not in hash table yet */ - WARN_ON(nf_ct_is_confirmed(ct)); - - nf_ct_dump_tuple(newreply); - - ct->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply; - if (ct->master || (help && !hlist_empty(&help->expectations))) - return; -} -EXPORT_SYMBOL_GPL(nf_conntrack_alter_reply); - /* Refresh conntrack for this many jiffies and do accounting if do_acct is 1 */ void __nf_ct_refresh_acct(struct nf_conn *ct, enum ip_conntrack_info ctinfo, diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c index f22691f83853..4ed5878cb25b 100644 --- a/net/netfilter/nf_conntrack_helper.c +++ b/net/netfilter/nf_conntrack_helper.c @@ -194,12 +194,7 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl, struct nf_conntrack_helper *helper = NULL; struct nf_conn_help *help; - /* We already got a helper explicitly attached. The function - * nf_conntrack_alter_reply - in case NAT is in use - asks for looking - * the helper up again. Since now the user is in full control of - * making consistent helper configurations, skip this automatic - * re-lookup, otherwise we'll lose the helper. - */ + /* We already got a helper explicitly attached (e.g. nft_ct) */ if (test_bit(IPS_HELPER_BIT, &ct->status)) return 0; -- 2.41.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-06 9:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <netfilter-devel> 2019-04-30 12:53 ` [PATCH nf] netfilter: nf_tables: fix oops during rule dump Florian Westphal 2019-05-01 3:54 ` Taehee Yoo 2019-05-20 19:21 ` Pablo Neira Ayuso 2022-09-06 15:20 ` [PATCH nf-next 0/2] netfilter: nat: avoid long-running loops Florian Westphal 2022-09-06 15:20 ` [PATCH nf-next 1/2] netfilter: nat: move repetitive nat port reserve loop to a helper Florian Westphal 2022-09-06 15:20 ` [PATCH nf-next 2/2] netfilter: nat: avoid long-running port range loop Florian Westphal 2023-10-06 9:27 ` [PATCH nf-next] netfilter: conntrack: simplify nf_conntrack_alter_reply Florian Westphal
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.