* [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.