netdev - Re: [PATCH net-next v4 05/13] flow_dissector: UDP encap infrastructure

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20240824191819.GU2164@kernel.org>
Date: Sat, 24 Aug 2024 20:18:19 +0100
From: Simon Horman <horms@...nel.org>
To: Tom Herbert <tom@...bertland.com>
Cc: davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com,
	netdev@...r.kernel.org, felipe@...anda.io,
	willemdebruijn.kernel@...il.com, pablo@...filter.org,
	laforge@...monks.org, xeb@...l.ru
Subject: Re: [PATCH net-next v4 05/13] flow_dissector: UDP encap
 infrastructure

On Fri, Aug 23, 2024 at 01:15:49PM -0700, Tom Herbert wrote:
> Add infrastructure for parsing into UDP encapsulations
> 
> Add function __skb_flow_dissect_udp that is called for IPPROTO_UDP.
> The flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS enables parsing of UDP
> encapsulations. If the flag is set when parsing a UDP packet then
> a socket lookup is performed. The offset of the base network header,
> either an IPv4 or IPv6 header, is tracked and passed to
> __skb_flow_dissect_udp so that it can perform the socket lookup
> 
> If a socket is found and it's for a UDP encapsulation (encap_type is
> set in the UDP socket) then a switch is performed on the encap_type
> value (cases are UDP_ENCAP_* values)
> 
> An encapsulated packet in UDP can either be indicated by an
> EtherType or IP protocol. The processing for dissecting a UDP encap
> protocol returns a flow dissector return code. If
> FLOW_DISSECT_RET_PROTO_AGAIN or FLOW_DISSECT_RET_IPPROTO_AGAIN is
> returned then the corresponding  encapsulated protocol is dissected.
> The nhoff is set to point to the header to process.  In the case
> FLOW_DISSECT_RET_PROTO_AGAIN the EtherType protocol is returned and
> the IP protocol is set to zero. In the case of
> FLOW_DISSECT_RET_IPPROTO_AGAIN, the IP protocol is returned and
> the EtherType protocol is returned unchanged
> 
> Signed-off-by: Tom Herbert <tom@...bertland.com>

...

> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c

...

> @@ -806,6 +807,134 @@ __skb_flow_dissect_batadv(const struct sk_buff *skb,
>  	return FLOW_DISSECT_RET_PROTO_AGAIN;
>  }
>  
> +static enum flow_dissect_ret
> +__skb_flow_dissect_udp(const struct sk_buff *skb, const struct net *net,
> +		       struct flow_dissector *flow_dissector,
> +		       void *target_container, const void *data,
> +		       int *p_nhoff, int hlen, __be16 *p_proto,
> +		       u8 *p_ip_proto, int base_nhoff, unsigned int flags,
> +		       unsigned int num_hdrs)
> +{
> +	enum flow_dissect_ret ret;
> +	struct udphdr _udph;
> +	int nhoff;
> +
> +	if (!(flags & FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS))
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	/* Check that the netns for the skb device is the same as the caller's,
> +	 * and only dissect UDP if we haven't yet encountered any encapsulation.
> +	 * The goal is to ensure that the socket lookup is being done in the
> +	 * right netns. Encapsulations may push packets into different name
> +	 * spaces, so this scheme is restricting UDP dissection to cases where
> +	 * they are in the same name spaces or at least the original name space.
> +	 * This should capture the majority of use cases for UDP encaps, and
> +	 * if we do encounter a UDP encapsulation within a different namespace
> +	 * then the only effect is we don't attempt UDP dissection
> +	 */
> +	if (dev_net(skb->dev) != net || num_hdrs > 0)
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	switch (*p_proto) {
> +#ifdef CONFIG_INET
> +	case htons(ETH_P_IP): {
> +		const struct udphdr *udph;
> +		const struct iphdr *iph;
> +		struct iphdr _iph;
> +		struct sock *sk;
> +
> +		iph = __skb_header_pointer(skb, base_nhoff, sizeof(_iph), data,
> +					   hlen, &_iph);
> +		if (!iph)
> +			return FLOW_DISSECT_RET_OUT_BAD;
> +
> +		udph = __skb_header_pointer(skb, *p_nhoff, sizeof(_udph), data,
> +					    hlen, &_udph);
> +		if (!udph)
> +			return FLOW_DISSECT_RET_OUT_BAD;
> +
> +		rcu_read_lock();
> +		/* Look up the UDPv4 socket and get the encap_type */
> +		sk = __udp4_lib_lookup(net, iph->saddr, udph->source,
> +				       iph->daddr, udph->dest,
> +				       inet_iif(skb), inet_sdif(skb),
> +				       net->ipv4.udp_table, NULL);
> +		if (!sk || !udp_sk(sk)->encap_type) {
> +			rcu_read_unlock();
> +			return FLOW_DISSECT_RET_OUT_GOOD;
> +		}
> +
> +		encap_type = udp_sk(sk)->encap_type;

Hi Tom,

I guess a local change went astray, or something like that,
because encap_type isn't declared in this scope.

> +		rcu_read_unlock();
> +
> +		break;
> +	}

...

-- 
pw-bot: cr