diff mbox series

[nf-next,5/6] netfilter: use nf_ip6_check_hbh_len in nf_ct_skb_network_trim

Message ID 5411027934a79f0430edb905ad4b434ec6b8396e.1677888566.git.lucien.xin@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series netfilter: handle ipv6 jumbo packets properly for bridge ovs and tc | expand

Checks

Context Check Description
netdev/series_format warning Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: coreteam@netfilter.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 29 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Xin Long March 4, 2023, 12:12 a.m. UTC
For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0,
and its real payload_len ( > 65535) is saved in hbh exthdr. With 0
length for the jumbo packets, all data and exthdr will be trimmed
in nf_ct_skb_network_trim().

This patch is to call nf_ip6_check_hbh_len() to get real pkt_len
of the IPv6 packet, similar to br_validate_ipv6().

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/netfilter/nf_conntrack_ovs.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Simon Horman March 6, 2023, 4:35 p.m. UTC | #1
On Fri, Mar 03, 2023 at 07:12:41PM -0500, Xin Long wrote:
> For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0,
> and its real payload_len ( > 65535) is saved in hbh exthdr. With 0
> length for the jumbo packets, all data and exthdr will be trimmed
> in nf_ct_skb_network_trim().
> 
> This patch is to call nf_ip6_check_hbh_len() to get real pkt_len
> of the IPv6 packet, similar to br_validate_ipv6().
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

...

> diff --git a/net/netfilter/nf_conntrack_ovs.c b/net/netfilter/nf_conntrack_ovs.c
> index 52b776bdf526..2016a3b05f86 100644
> --- a/net/netfilter/nf_conntrack_ovs.c
> +++ b/net/netfilter/nf_conntrack_ovs.c

...

> @@ -114,14 +115,20 @@ EXPORT_SYMBOL_GPL(nf_ct_add_helper);
>  int nf_ct_skb_network_trim(struct sk_buff *skb, int family)
>  {
>  	unsigned int len;
> +	int err;
>  
>  	switch (family) {
>  	case NFPROTO_IPV4:
>  		len = skb_ip_totlen(skb);
>  		break;
>  	case NFPROTO_IPV6:
> -		len = sizeof(struct ipv6hdr)
> -			+ ntohs(ipv6_hdr(skb)->payload_len);
> +		len = ntohs(ipv6_hdr(skb)->payload_len);
> +		if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP) {

nit: if you spin a v2,
     you may consider reducing the scope of err to this block.

> +			err = nf_ip6_check_hbh_len(skb, &len);
> +			if (err)
> +				return err;
> +		}
> +		len += sizeof(struct ipv6hdr);
>  		break;
>  	default:
>  		len = skb->len;
> -- 
> 2.39.1
>
Nikolay Aleksandrov March 7, 2023, 9:22 a.m. UTC | #2
On 04/03/2023 02:12, Xin Long wrote:
> For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0,
> and its real payload_len ( > 65535) is saved in hbh exthdr. With 0
> length for the jumbo packets, all data and exthdr will be trimmed
> in nf_ct_skb_network_trim().
> 
> This patch is to call nf_ip6_check_hbh_len() to get real pkt_len
> of the IPv6 packet, similar to br_validate_ipv6().
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/netfilter/nf_conntrack_ovs.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 

I'd also recommend to change the scope of "err" as Simon already pointed out.
Other than that looks good to me.

Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Aaron Conole March 7, 2023, 6:31 p.m. UTC | #3
Xin Long <lucien.xin@gmail.com> writes:

> For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0,
> and its real payload_len ( > 65535) is saved in hbh exthdr. With 0
> length for the jumbo packets, all data and exthdr will be trimmed
> in nf_ct_skb_network_trim().
>
> This patch is to call nf_ip6_check_hbh_len() to get real pkt_len
> of the IPv6 packet, similar to br_validate_ipv6().
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---

Reviewed-by: Aaron Conole <aconole@redhat.com>

>  net/netfilter/nf_conntrack_ovs.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_ovs.c b/net/netfilter/nf_conntrack_ovs.c
> index 52b776bdf526..2016a3b05f86 100644
> --- a/net/netfilter/nf_conntrack_ovs.c
> +++ b/net/netfilter/nf_conntrack_ovs.c
> @@ -6,6 +6,7 @@
>  #include <net/netfilter/ipv6/nf_defrag_ipv6.h>
>  #include <net/ipv6_frag.h>
>  #include <net/ip.h>
> +#include <linux/netfilter_ipv6.h>
>  
>  /* 'skb' should already be pulled to nh_ofs. */
>  int nf_ct_helper(struct sk_buff *skb, struct nf_conn *ct,
> @@ -114,14 +115,20 @@ EXPORT_SYMBOL_GPL(nf_ct_add_helper);
>  int nf_ct_skb_network_trim(struct sk_buff *skb, int family)
>  {
>  	unsigned int len;
> +	int err;
>  
>  	switch (family) {
>  	case NFPROTO_IPV4:
>  		len = skb_ip_totlen(skb);
>  		break;
>  	case NFPROTO_IPV6:
> -		len = sizeof(struct ipv6hdr)
> -			+ ntohs(ipv6_hdr(skb)->payload_len);
> +		len = ntohs(ipv6_hdr(skb)->payload_len);
> +		if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP) {
> +			err = nf_ip6_check_hbh_len(skb, &len);
> +			if (err)
> +				return err;
> +		}
> +		len += sizeof(struct ipv6hdr);
>  		break;
>  	default:
>  		len = skb->len;
Xin Long March 7, 2023, 8:58 p.m. UTC | #4
On Mon, Mar 6, 2023 at 11:35 AM Simon Horman <simon.horman@corigine.com> wrote:
>
> On Fri, Mar 03, 2023 at 07:12:41PM -0500, Xin Long wrote:
> > For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0,
> > and its real payload_len ( > 65535) is saved in hbh exthdr. With 0
> > length for the jumbo packets, all data and exthdr will be trimmed
> > in nf_ct_skb_network_trim().
> >
> > This patch is to call nf_ip6_check_hbh_len() to get real pkt_len
> > of the IPv6 packet, similar to br_validate_ipv6().
> >
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
>
> ...
>
> > diff --git a/net/netfilter/nf_conntrack_ovs.c b/net/netfilter/nf_conntrack_ovs.c
> > index 52b776bdf526..2016a3b05f86 100644
> > --- a/net/netfilter/nf_conntrack_ovs.c
> > +++ b/net/netfilter/nf_conntrack_ovs.c
>
> ...
>
> > @@ -114,14 +115,20 @@ EXPORT_SYMBOL_GPL(nf_ct_add_helper);
> >  int nf_ct_skb_network_trim(struct sk_buff *skb, int family)
> >  {
> >       unsigned int len;
> > +     int err;
> >
> >       switch (family) {
> >       case NFPROTO_IPV4:
> >               len = skb_ip_totlen(skb);
> >               break;
> >       case NFPROTO_IPV6:
> > -             len = sizeof(struct ipv6hdr)
> > -                     + ntohs(ipv6_hdr(skb)->payload_len);
> > +             len = ntohs(ipv6_hdr(skb)->payload_len);
> > +             if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP) {
>
> nit: if you spin a v2,
>      you may consider reducing the scope of err to this block.
>
Hi, Simon,

I will post v2 with your suggestions including those in another 3 patches. :)

Thanks.

> > +                     err = nf_ip6_check_hbh_len(skb, &len);
> > +                     if (err)
> > +                             return err;
> > +             }
> > +             len += sizeof(struct ipv6hdr);
> >               break;
> >       default:
> >               len = skb->len;
> > --
> > 2.39.1
> >
diff mbox series

Patch

diff --git a/net/netfilter/nf_conntrack_ovs.c b/net/netfilter/nf_conntrack_ovs.c
index 52b776bdf526..2016a3b05f86 100644
--- a/net/netfilter/nf_conntrack_ovs.c
+++ b/net/netfilter/nf_conntrack_ovs.c
@@ -6,6 +6,7 @@ 
 #include <net/netfilter/ipv6/nf_defrag_ipv6.h>
 #include <net/ipv6_frag.h>
 #include <net/ip.h>
+#include <linux/netfilter_ipv6.h>
 
 /* 'skb' should already be pulled to nh_ofs. */
 int nf_ct_helper(struct sk_buff *skb, struct nf_conn *ct,
@@ -114,14 +115,20 @@  EXPORT_SYMBOL_GPL(nf_ct_add_helper);
 int nf_ct_skb_network_trim(struct sk_buff *skb, int family)
 {
 	unsigned int len;
+	int err;
 
 	switch (family) {
 	case NFPROTO_IPV4:
 		len = skb_ip_totlen(skb);
 		break;
 	case NFPROTO_IPV6:
-		len = sizeof(struct ipv6hdr)
-			+ ntohs(ipv6_hdr(skb)->payload_len);
+		len = ntohs(ipv6_hdr(skb)->payload_len);
+		if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP) {
+			err = nf_ip6_check_hbh_len(skb, &len);
+			if (err)
+				return err;
+		}
+		len += sizeof(struct ipv6hdr);
 		break;
 	default:
 		len = skb->len;