netdev - Re: [PATCH 10/12] flow_dissector: Parse Geneve in UDP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <66ae82aa32061_2a823d294ae@willemb.c.googlers.com.notmuch>
Date: Sat, 03 Aug 2024 15:19:06 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 Tom Herbert <tom@...bertland.com>, 
 davem@...emloft.net, 
 kuba@...nel.org, 
 edumazet@...gle.com, 
 netdev@...r.kernel.org, 
 felipe@...anda.io
Cc: Tom Herbert <tom@...bertland.com>
Subject: Re: [PATCH 10/12] flow_dissector: Parse Geneve in UDP

Willem de Bruijn wrote:
> Tom Herbert wrote:
> > Parse Geneve in a UDP encapsulation
> > 
> > Signed-off-by: Tom Herbert <tom@...bertland.com>
> > ---
> >  net/core/flow_dissector.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> > index 3766dc4d5b23..4fff60233992 100644
> > --- a/net/core/flow_dissector.c
> > +++ b/net/core/flow_dissector.c
> > @@ -11,6 +11,7 @@
> >  #include <net/fou.h>
> >  #include <net/ip.h>
> >  #include <net/ipv6.h>
> > +#include <net/geneve.h>
> >  #include <net/gre.h>
> >  #include <net/pptp.h>
> >  #include <net/tipc.h>
> > @@ -808,6 +809,29 @@ __skb_flow_dissect_vxlan(const struct sk_buff *skb,
> >  	return FLOW_DISSECT_RET_PROTO_AGAIN;
> >  }
> >  
> > +static enum flow_dissect_ret
> > +__skb_flow_dissect_geneve(const struct sk_buff *skb,
> > +			  struct flow_dissector *flow_dissector,
> > +			  void *target_container, const void *data,
> > +			  __be16 *p_proto, int *p_nhoff, int hlen,
> > +			  unsigned int flags)
> > +{
> > +	struct genevehdr *hdr, _hdr;
> > +
> > +	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr), data, hlen,
> > +				   &_hdr);
> > +	if (!hdr)
> > +		return FLOW_DISSECT_RET_OUT_BAD;
> > +
> > +	if (hdr->ver != 0)
> > +		return FLOW_DISSECT_RET_OUT_GOOD;
> > +
> > +	*p_proto = hdr->proto_type;
> > +	*p_nhoff += sizeof(struct genevehdr) + (hdr->opt_len * 4);
> > +
> > +	return FLOW_DISSECT_RET_PROTO_AGAIN;
> 
> Do you want to return FLOW_DISSECT_RET_OUT_GOOD if IPPROTO 59.
> 
> Per your spec: "IP protocol number 59 ("No next header") may be set to
> indicate that the GUE payload does not begin with the header of an IP
> protocol."
> 
> Admittedly pendantic. No idea if any implementation actually sets
> this.

And reply to the wrong patch. I meant this for GUE.

Also not sure how useful the separate __skb_direct_ip_dissect is if
then have to catch the error special case in the caller.