2023-03-02 11:23:57

by Madhu Koriginja

[permalink] [raw]
Subject: [PATCH] [net:netfilter]: Keep conntrack reference until IPsecv6 policy checks are done

Keep the conntrack reference until policy checks have been performed for
IPsec V6 NAT support. The reference needs to be dropped before a packet is
queued to avoid having the conntrack module unloadable.

Signed-off-by: Madhu Koriginja <[email protected]>
V1-V2: added missing () in ip6_input.c in below condition
if (!(ipprot->flags & INET6_PROTO_NOPOLICY))
---
net/dccp/ipv6.c | 1 +
net/ipv6/ip6_input.c | 14 +++++++-------
net/ipv6/raw.c | 2 +-
net/ipv6/tcp_ipv6.c | 2 ++
net/ipv6/udp.c | 2 ++
5 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 58a401e9cf09..eb503096db6c 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -771,6 +771,7 @@ static int dccp_v6_rcv(struct sk_buff *skb)

if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
goto discard_and_relse;
+ nf_reset(skb);

return __sk_receive_skb(sk, skb, 1, dh->dccph_doff * 4,
refcounted) ? -1 : 0;
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index acf0749ee5bb..10d9c33cbdda 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -374,10 +374,6 @@ static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *sk
/* Only do this once for first final protocol */
have_final = true;

- /* Free reference early: we don't need it any more,
- and it may hold ip_conntrack module loaded
- indefinitely. */
- nf_reset(skb);

skb_postpull_rcsum(skb, skb_network_header(skb),
skb_network_header_len(skb));
@@ -388,9 +384,13 @@ static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *sk
!ipv6_is_mld(skb, nexthdr, skb_network_header_len(skb)))
goto discard;
}
- if (!(ipprot->flags & INET6_PROTO_NOPOLICY) &&
- !xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
- goto discard;
+
+ if (!(ipprot->flags & INET6_PROTO_NOPOLICY)) {
+ if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
+ goto discard;
+
+ nf_reset(skb);
+ }

ret = ipprot->handler(skb);
if (ret > 0) {
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 4856d9320b28..cf68c9418897 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -220,7 +220,6 @@ static bool ipv6_raw_deliver(struct sk_buff *skb, int nexthdr)

/* Not releasing hash table! */
if (clone) {
- nf_reset(clone);
rawv6_rcv(sk, clone);
}
}
@@ -428,6 +427,7 @@ int rawv6_rcv(struct sock *sk, struct sk_buff *skb)
kfree_skb(skb);
return NET_RX_DROP;
}
+ nf_reset(skb);

if (!rp->checksum)
skb->ip_summed = CHECKSUM_UNNECESSARY;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9a117a79af65..0bc959cfbea4 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1534,6 +1534,8 @@ static int tcp_v6_rcv(struct sk_buff *skb)
if (tcp_v6_inbound_md5_hash(sk, skb))
goto discard_and_relse;

+ nf_reset(skb);
+
if (tcp_filter(sk, skb))
goto discard_and_relse;
th = (const struct tcphdr *)skb->data;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 72b2e7809af6..aacb48e977cb 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -567,6 +567,7 @@ static int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)

if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
goto drop;
+ nf_reset(skb);

if (static_branch_unlikely(&udpv6_encap_needed_key) && up->encap_type) {
int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
@@ -860,6 +861,7 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,

if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
goto discard;
+ nf_reset(skb);

if (udp_lib_checksum_complete(skb))
goto csum_error;
--
2.25.1



2023-03-02 11:36:05

by Florian Westphal

[permalink] [raw]
Subject: Re: [PATCH] [net:netfilter]: Keep conntrack reference until IPsecv6 policy checks are done

Madhu Koriginja <[email protected]> wrote:
> Keep the conntrack reference until policy checks have been performed for
> IPsec V6 NAT support. The reference needs to be dropped before a packet is
> queued to avoid having the conntrack module unloadable.

Subject Line should be:

[PATCH net] net: netfilter: Keep conntrack reference until IPsecv6 policy checks are done
or
[PATCH net-next] net: netfilter: Keep ..

see below why net-next makes more sense to me.

> Signed-off-by: Madhu Koriginja <[email protected]>
> V1-V2: added missing () in ip6_input.c in below condition
> if (!(ipprot->flags & INET6_PROTO_NOPOLICY))

This should appear before your signed-off-by, or
> ---
> net/dccp/ipv6.c | 1 +

... here.

I think its fine to place it here because in this case
the mini-changelog doesn't provide any additional context
worth keeping in git.

Paolo, Jakub, David: This is a bug, but its not a regression
either. I would suggest that Madhu resubmits this AFTER
net-next re-opens.

Madhu, if thats the agreed-upon procedure, you may include

Reviewed-by: Florian Westphal <[email protected]>

when you resend this patch as-is.

2023-03-02 14:28:40

by Paolo Abeni

[permalink] [raw]
Subject: Re: [PATCH] [net:netfilter]: Keep conntrack reference until IPsecv6 policy checks are done

On Thu, 2023-03-02 at 16:53 +0530, Madhu Koriginja wrote:
> Keep the conntrack reference until policy checks have been performed for
> IPsec V6 NAT support. The reference needs to be dropped before a packet is
> queued to avoid having the conntrack module unloadable.
>
> Signed-off-by: Madhu Koriginja <[email protected]>
> V1-V2: added missing () in ip6_input.c in below condition
> if (!(ipprot->flags & INET6_PROTO_NOPOLICY))
> ---
> net/dccp/ipv6.c | 1 +
> net/ipv6/ip6_input.c | 14 +++++++-------
> net/ipv6/raw.c | 2 +-
> net/ipv6/tcp_ipv6.c | 2 ++
> net/ipv6/udp.c | 2 ++
> 5 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
> index 58a401e9cf09..eb503096db6c 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
> @@ -771,6 +771,7 @@ static int dccp_v6_rcv(struct sk_buff *skb)
>
> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
> goto discard_and_relse;
> + nf_reset(skb);

nf_reset() is gone since commit 895b5c9f206e ("netfilter: drop bridge
nf reset from nf_reset"), you should use instead nf_reset_ct(): in the
current form the patch does not apply cleanly (nor build after manual
edit).


Cheers,

Paolo


2023-03-03 09:53:30

by Madhu Koriginja

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH] [net:netfilter]: Keep conntrack reference until IPsecv6 policy checks are done

Hi Paolo,
I updated the patch with nf_reset_ct change (nf_reset() replaced with nf_reset_ct) to align with the latest kernel code.

Hi All,
I also want to push this change (with nf_reset) in 4.19 LTS kernel, please can you tell me how can I push to that version? Thanks in advance.

Hi Florian,
In patchv3 updated the subject line as you suggested.

Thanks & Regards,
Madhu K

-----Original Message-----
From: Paolo Abeni <[email protected]>
Sent: Thursday, March 2, 2023 7:58 PM
To: Madhu Koriginja <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Vani Namala <[email protected]>
Subject: [EXT] Re: [PATCH] [net:netfilter]: Keep conntrack reference until IPsecv6 policy checks are done

Caution: EXT Email

On Thu, 2023-03-02 at 16:53 +0530, Madhu Koriginja wrote:
> Keep the conntrack reference until policy checks have been performed
> for IPsec V6 NAT support. The reference needs to be dropped before a
> packet is queued to avoid having the conntrack module unloadable.
>
> Signed-off-by: Madhu Koriginja <[email protected]>
> V1-V2: added missing () in ip6_input.c in below condition
> if (!(ipprot->flags & INET6_PROTO_NOPOLICY))
> ---
> net/dccp/ipv6.c | 1 +
> net/ipv6/ip6_input.c | 14 +++++++-------
> net/ipv6/raw.c | 2 +-
> net/ipv6/tcp_ipv6.c | 2 ++
> net/ipv6/udp.c | 2 ++
> 5 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c index
> 58a401e9cf09..eb503096db6c 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
> @@ -771,6 +771,7 @@ static int dccp_v6_rcv(struct sk_buff *skb)
>
> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
> goto discard_and_relse;
> + nf_reset(skb);

nf_reset() is gone since commit 895b5c9f206e ("netfilter: drop bridge nf reset from nf_reset"), you should use instead nf_reset_ct(): in the current form the patch does not apply cleanly (nor build after manual edit).


Cheers,

Paolo

2023-03-07 06:22:59

by Madhu Koriginja

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH] [net:netfilter]: Keep conntrack reference until IPsecv6 policy checks are done

Hi Florian/Paolo,
As Florian suggested I changed the subject title, due to this patch is pushed in different link. Please find the nf_reset_ct changes patch in the below link
https://lore.kernel.org/lkml/[email protected]/T/

Please can you also suggest me how to push patch on 4.19 LTS kernel.

Thanks & Regards,
Madhu K

-----Original Message-----
From: Madhu Koriginja
Sent: Friday, March 3, 2023 3:23 PM
To: Paolo Abeni <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Vani Namala <[email protected]>
Subject: RE: [EXT] Re: [PATCH] [net:netfilter]: Keep conntrack reference until IPsecv6 policy checks are done

Hi Paolo,
I updated the patch with nf_reset_ct change (nf_reset() replaced with nf_reset_ct) to align with the latest kernel code.

Hi All,
I also want to push this change (with nf_reset) in 4.19 LTS kernel, please can you tell me how can I push to that version? Thanks in advance.

Hi Florian,
In patchv3 updated the subject line as you suggested.

Thanks & Regards,
Madhu K

-----Original Message-----
From: Paolo Abeni <[email protected]>
Sent: Thursday, March 2, 2023 7:58 PM
To: Madhu Koriginja <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Vani Namala <[email protected]>
Subject: [EXT] Re: [PATCH] [net:netfilter]: Keep conntrack reference until IPsecv6 policy checks are done

Caution: EXT Email

On Thu, 2023-03-02 at 16:53 +0530, Madhu Koriginja wrote:
> Keep the conntrack reference until policy checks have been performed
> for IPsec V6 NAT support. The reference needs to be dropped before a
> packet is queued to avoid having the conntrack module unloadable.
>
> Signed-off-by: Madhu Koriginja <[email protected]>
> V1-V2: added missing () in ip6_input.c in below condition
> if (!(ipprot->flags & INET6_PROTO_NOPOLICY))
> ---
> net/dccp/ipv6.c | 1 +
> net/ipv6/ip6_input.c | 14 +++++++-------
> net/ipv6/raw.c | 2 +-
> net/ipv6/tcp_ipv6.c | 2 ++
> net/ipv6/udp.c | 2 ++
> 5 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c index
> 58a401e9cf09..eb503096db6c 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
> @@ -771,6 +771,7 @@ static int dccp_v6_rcv(struct sk_buff *skb)
>
> if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
> goto discard_and_relse;
> + nf_reset(skb);

nf_reset() is gone since commit 895b5c9f206e ("netfilter: drop bridge nf reset from nf_reset"), you should use instead nf_reset_ct(): in the current form the patch does not apply cleanly (nor build after manual edit).


Cheers,

Paolo