* [PATCH net 0/2] Netfilter fixes for net
@ 2023-04-21 10:56 Pablo Neira Ayuso
2023-04-21 10:56 ` [PATCH net 1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert() Pablo Neira Ayuso
2023-04-21 10:57 ` [PATCH net 2/2] netfilter: conntrack: fix wrong ct->timeout value Pablo Neira Ayuso
0 siblings, 2 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2023-04-21 10:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet
Hi,
The following patchset contains late Netfilter fixes for net:
1) Set on IPS_CONFIRMED before change_status() otherwise EBUSY is
bogusly hit. This bug was introduced in the 6.3 release cycle.
2) Fix nfnetlink_queue conntrack support: Set/dump timeout
accordingly for unconfirmed conntrack entries. Make sure this
is done after IPS_CONFIRMED is set on. This is an old bug, it
happens since the introduction of this feature.
Please, pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-23-04-21
Thanks.
----------------------------------------------------------------
The following changes since commit 92e8c732d8518588ac34b4cb3feaf37d2cb87555:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf (2023-04-18 20:46:31 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-23-04-21
for you to fetch changes up to 73db1b8f2bb6725b7391e85aab41fdf592b3c0c1:
netfilter: conntrack: fix wrong ct->timeout value (2023-04-19 12:08:38 +0200)
----------------------------------------------------------------
netfilter pull request
----------------------------------------------------------------
Pablo Neira Ayuso (1):
netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert()
Tzung-Bi Shih (1):
netfilter: conntrack: fix wrong ct->timeout value
include/net/netfilter/nf_conntrack_core.h | 6 +++++-
net/netfilter/nf_conntrack_bpf.c | 1 +
net/netfilter/nf_conntrack_core.c | 1 -
net/netfilter/nf_conntrack_netlink.c | 16 ++++++++++++----
4 files changed, 18 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net 1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert()
2023-04-21 10:56 [PATCH net 0/2] Netfilter fixes for net Pablo Neira Ayuso
@ 2023-04-21 10:56 ` Pablo Neira Ayuso
2023-04-21 15:00 ` patchwork-bot+netdevbpf
2023-04-21 10:57 ` [PATCH net 2/2] netfilter: conntrack: fix wrong ct->timeout value Pablo Neira Ayuso
1 sibling, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2023-04-21 10:56 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet
e6d57e9ff0ae ("netfilter: conntrack: fix rmmod double-free race")
consolidates IPS_CONFIRMED bit set in nf_conntrack_hash_check_insert().
However, this breaks ctnetlink:
# conntrack -I -p tcp --timeout 123 --src 1.2.3.4 --dst 5.6.7.8 --state ESTABLISHED --sport 1 --dport 4 -u SEEN_REPLY
conntrack v1.4.6 (conntrack-tools): Operation failed: Device or resource busy
This is a partial revert of the aforementioned commit to restore
IPS_CONFIRMED.
Fixes: e6d57e9ff0ae ("netfilter: conntrack: fix rmmod double-free race")
Reported-by: Stéphane Graber <stgraber@stgraber.org>
Tested-by: Stéphane Graber <stgraber@stgraber.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_bpf.c | 1 +
net/netfilter/nf_conntrack_core.c | 1 -
net/netfilter/nf_conntrack_netlink.c | 3 +++
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index cd99e6dc1f35..34913521c385 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -381,6 +381,7 @@ __bpf_kfunc struct nf_conn *bpf_ct_insert_entry(struct nf_conn___init *nfct_i)
struct nf_conn *nfct = (struct nf_conn *)nfct_i;
int err;
+ nfct->status |= IPS_CONFIRMED;
err = nf_conntrack_hash_check_insert(nfct);
if (err < 0) {
nf_conntrack_free(nfct);
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index c6a6a6099b4e..7ba6ab9b54b5 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -932,7 +932,6 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct)
goto out;
}
- ct->status |= IPS_CONFIRMED;
smp_wmb();
/* The caller holds a reference to this object */
refcount_set(&ct->ct_general.use, 2);
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index bfc3aaa2c872..d3ee18854698 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2316,6 +2316,9 @@ ctnetlink_create_conntrack(struct net *net,
nfct_seqadj_ext_add(ct);
nfct_synproxy_ext_add(ct);
+ /* we must add conntrack extensions before confirmation. */
+ ct->status |= IPS_CONFIRMED;
+
if (cda[CTA_STATUS]) {
err = ctnetlink_change_status(ct, cda);
if (err < 0)
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net 1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert()
2023-04-21 10:56 ` [PATCH net 1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert() Pablo Neira Ayuso
@ 2023-04-21 15:00 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-04-21 15:00 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, davem, netdev, kuba, pabeni, edumazet
Hello:
This series was applied to netdev/net.git (main)
by Pablo Neira Ayuso <pablo@netfilter.org>:
On Fri, 21 Apr 2023 12:56:59 +0200 you wrote:
> e6d57e9ff0ae ("netfilter: conntrack: fix rmmod double-free race")
> consolidates IPS_CONFIRMED bit set in nf_conntrack_hash_check_insert().
> However, this breaks ctnetlink:
>
> # conntrack -I -p tcp --timeout 123 --src 1.2.3.4 --dst 5.6.7.8 --state ESTABLISHED --sport 1 --dport 4 -u SEEN_REPLY
> conntrack v1.4.6 (conntrack-tools): Operation failed: Device or resource busy
>
> [...]
Here is the summary with links:
- [net,1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert()
https://git.kernel.org/netdev/net/c/2cdaa3eefed8
- [net,2/2] netfilter: conntrack: fix wrong ct->timeout value
https://git.kernel.org/netdev/net/c/73db1b8f2bb6
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net 2/2] netfilter: conntrack: fix wrong ct->timeout value
2023-04-21 10:56 [PATCH net 0/2] Netfilter fixes for net Pablo Neira Ayuso
2023-04-21 10:56 ` [PATCH net 1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert() Pablo Neira Ayuso
@ 2023-04-21 10:57 ` Pablo Neira Ayuso
1 sibling, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2023-04-21 10:57 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet
From: Tzung-Bi Shih <tzungbi@kernel.org>
(struct nf_conn)->timeout is an interval before the conntrack
confirmed. After confirmed, it becomes a timestamp.
It is observed that timeout of an unconfirmed conntrack:
- Set by calling ctnetlink_change_timeout(). As a result,
`nfct_time_stamp` was wrongly added to `ct->timeout` twice.
- Get by calling ctnetlink_dump_timeout(). As a result,
`nfct_time_stamp` was wrongly subtracted.
Call Trace:
<TASK>
dump_stack_lvl
ctnetlink_dump_timeout
__ctnetlink_glue_build
ctnetlink_glue_build
__nfqnl_enqueue_packet
nf_queue
nf_hook_slow
ip_mc_output
? __pfx_ip_finish_output
ip_send_skb
? __pfx_dst_output
udp_send_skb
udp_sendmsg
? __pfx_ip_generic_getfrag
sock_sendmsg
Separate the 2 cases in:
- Setting `ct->timeout` in __nf_ct_set_timeout().
- Getting `ct->timeout` in ctnetlink_dump_timeout().
Pablo appends:
Update ctnetlink to set up the timeout _after_ the IPS_CONFIRMED flag is
set on, otherwise conntrack creation via ctnetlink breaks.
Note that the problem described in this patch occurs since the
introduction of the nfnetlink_queue conntrack support, select a
sufficiently old Fixes: tag for -stable kernel to pick up this fix.
Fixes: a4b4766c3ceb ("netfilter: nfnetlink_queue: rename related to nfqueue attaching conntrack info")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/net/netfilter/nf_conntrack_core.h | 6 +++++-
net/netfilter/nf_conntrack_netlink.c | 13 +++++++++----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index 71d1269fe4d4..3384859a8921 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -89,7 +89,11 @@ static inline void __nf_ct_set_timeout(struct nf_conn *ct, u64 timeout)
{
if (timeout > INT_MAX)
timeout = INT_MAX;
- WRITE_ONCE(ct->timeout, nfct_time_stamp + (u32)timeout);
+
+ if (nf_ct_is_confirmed(ct))
+ WRITE_ONCE(ct->timeout, nfct_time_stamp + (u32)timeout);
+ else
+ ct->timeout = (u32)timeout;
}
int __nf_ct_change_timeout(struct nf_conn *ct, u64 cta_timeout);
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index d3ee18854698..6f3b23a6653c 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -176,7 +176,12 @@ static int ctnetlink_dump_status(struct sk_buff *skb, const struct nf_conn *ct)
static int ctnetlink_dump_timeout(struct sk_buff *skb, const struct nf_conn *ct,
bool skip_zero)
{
- long timeout = nf_ct_expires(ct) / HZ;
+ long timeout;
+
+ if (nf_ct_is_confirmed(ct))
+ timeout = nf_ct_expires(ct) / HZ;
+ else
+ timeout = ct->timeout / HZ;
if (skip_zero && timeout == 0)
return 0;
@@ -2253,9 +2258,6 @@ ctnetlink_create_conntrack(struct net *net,
if (!cda[CTA_TIMEOUT])
goto err1;
- timeout = (u64)ntohl(nla_get_be32(cda[CTA_TIMEOUT])) * HZ;
- __nf_ct_set_timeout(ct, timeout);
-
rcu_read_lock();
if (cda[CTA_HELP]) {
char *helpname = NULL;
@@ -2319,6 +2321,9 @@ ctnetlink_create_conntrack(struct net *net,
/* we must add conntrack extensions before confirmation. */
ct->status |= IPS_CONFIRMED;
+ timeout = (u64)ntohl(nla_get_be32(cda[CTA_TIMEOUT])) * HZ;
+ __nf_ct_set_timeout(ct, timeout);
+
if (cda[CTA_STATUS]) {
err = ctnetlink_change_status(ct, cda);
if (err < 0)
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net 0/2] Netfilter fixes for net
@ 2023-04-20 17:06 Pablo Neira Ayuso
2023-04-20 17:06 ` [PATCH net 1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert() Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2023-04-20 17:06 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet
Hi,
The following patchset contains late Netfilter fixes for net:
1) Set on IPS_CONFIRMED before change_status() otherwise EBUSY is
bogusly hit. This bug was introduced in the 6.3 release cycle.
2) Fix nfnetlink_queue conntrack support: Set/dump timeout
accordingly for unconfirmed conntrack entries. Make sure this
is done after IPS_CONFIRMED is set on. This is an old bug, it
happens since the introduction of this feature.
Please, pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git
Thanks.
----------------------------------------------------------------
The following changes since commit 92e8c732d8518588ac34b4cb3feaf37d2cb87555:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf (2023-04-18 20:46:31 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git HEAD
for you to fetch changes up to 73db1b8f2bb6725b7391e85aab41fdf592b3c0c1:
netfilter: conntrack: fix wrong ct->timeout value (2023-04-19 12:08:38 +0200)
----------------------------------------------------------------
Pablo Neira Ayuso (1):
netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert()
Tzung-Bi Shih (1):
netfilter: conntrack: fix wrong ct->timeout value
include/net/netfilter/nf_conntrack_core.h | 6 +++++-
net/netfilter/nf_conntrack_bpf.c | 1 +
net/netfilter/nf_conntrack_core.c | 1 -
net/netfilter/nf_conntrack_netlink.c | 16 ++++++++++++----
4 files changed, 18 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net 1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert()
2023-04-20 17:06 [PATCH net 0/2] Netfilter fixes for net Pablo Neira Ayuso
@ 2023-04-20 17:06 ` Pablo Neira Ayuso
0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2023-04-20 17:06 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet
e6d57e9ff0ae ("netfilter: conntrack: fix rmmod double-free race")
consolidates IPS_CONFIRMED bit set in nf_conntrack_hash_check_insert().
However, this breaks ctnetlink:
# conntrack -I -p tcp --timeout 123 --src 1.2.3.4 --dst 5.6.7.8 --state ESTABLISHED --sport 1 --dport 4 -u SEEN_REPLY
conntrack v1.4.6 (conntrack-tools): Operation failed: Device or resource busy
This is a partial revert of the aforementioned commit to restore
IPS_CONFIRMED.
Fixes: e6d57e9ff0ae ("netfilter: conntrack: fix rmmod double-free race")
Reported-by: Stéphane Graber <stgraber@stgraber.org>
Tested-by: Stéphane Graber <stgraber@stgraber.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_bpf.c | 1 +
net/netfilter/nf_conntrack_core.c | 1 -
net/netfilter/nf_conntrack_netlink.c | 3 +++
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index cd99e6dc1f35..34913521c385 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -381,6 +381,7 @@ __bpf_kfunc struct nf_conn *bpf_ct_insert_entry(struct nf_conn___init *nfct_i)
struct nf_conn *nfct = (struct nf_conn *)nfct_i;
int err;
+ nfct->status |= IPS_CONFIRMED;
err = nf_conntrack_hash_check_insert(nfct);
if (err < 0) {
nf_conntrack_free(nfct);
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index c6a6a6099b4e..7ba6ab9b54b5 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -932,7 +932,6 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct)
goto out;
}
- ct->status |= IPS_CONFIRMED;
smp_wmb();
/* The caller holds a reference to this object */
refcount_set(&ct->ct_general.use, 2);
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index bfc3aaa2c872..d3ee18854698 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2316,6 +2316,9 @@ ctnetlink_create_conntrack(struct net *net,
nfct_seqadj_ext_add(ct);
nfct_synproxy_ext_add(ct);
+ /* we must add conntrack extensions before confirmation. */
+ ct->status |= IPS_CONFIRMED;
+
if (cda[CTA_STATUS]) {
err = ctnetlink_change_status(ct, cda);
if (err < 0)
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-04-21 15:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-21 10:56 [PATCH net 0/2] Netfilter fixes for net Pablo Neira Ayuso
2023-04-21 10:56 ` [PATCH net 1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert() Pablo Neira Ayuso
2023-04-21 15:00 ` patchwork-bot+netdevbpf
2023-04-21 10:57 ` [PATCH net 2/2] netfilter: conntrack: fix wrong ct->timeout value Pablo Neira Ayuso
-- strict thread matches above, loose matches on Subject: below --
2023-04-20 17:06 [PATCH net 0/2] Netfilter fixes for net Pablo Neira Ayuso
2023-04-20 17:06 ` [PATCH net 1/2] netfilter: conntrack: restore IPS_CONFIRMED out of nf_conntrack_hash_check_insert() Pablo Neira Ayuso
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.