linux-kernel - Re: [PATCH net-next v11 08/23] ovpn: implement basic TX path (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: <4fe9f0d5-a8ac-4f2e-aee7-00cbeaf2f0aa@gmail.com>
Date: Mon, 11 Nov 2024 01:54:51 +0200
From: Sergey Ryazanov <ryazanov.s.a@...il.com>
To: Antonio Quartulli <antonio@...nvpn.net>
Cc: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Donald Hunter <donald.hunter@...il.com>,
 Shuah Khan <shuah@...nel.org>, sd@...asysnail.net,
 Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next v11 08/23] ovpn: implement basic TX path (UDP)

Another one forgotten question, sorry about this. Please find the 
question inlined.

On 29.10.2024 12:47, Antonio Quartulli wrote:
>   /* Send user data to the network
>    */
>   netdev_tx_t ovpn_net_xmit(struct sk_buff *skb, struct net_device *dev)
>   {
> +	struct ovpn_struct *ovpn = netdev_priv(dev);
> +	struct sk_buff *segments, *curr, *next;
> +	struct sk_buff_head skb_list;
> +	__be16 proto;
> +	int ret;
> +
> +	/* reset netfilter state */
> +	nf_reset_ct(skb);
> +
> +	/* verify IP header size in network packet */
> +	proto = ovpn_ip_check_protocol(skb);
> +	if (unlikely(!proto || skb->protocol != proto)) {
> +		net_err_ratelimited("%s: dropping malformed payload packet\n",
> +				    dev->name);
> +		dev_core_stats_tx_dropped_inc(ovpn->dev);
> +		goto drop;
> +	}

The above check implies that kernel can feed a network device with 
skb->protocol value mismatches actual skb content. Can you share any 
example of such case?

If you just want to be sure that the user packet is either IPv4 or IPv6 
then it can be done like this and without error messages:

/* Support only IPv4 or IPv6 traffic transporting */
if (unlikely(skb->protocol == ETH_P_IP || skb->protocol == ETH_P_IPV6))
     goto drop;

> +
> +	if (skb_is_gso(skb)) {
> +		segments = skb_gso_segment(skb, 0);
> +		if (IS_ERR(segments)) {
> +			ret = PTR_ERR(segments);
> +			net_err_ratelimited("%s: cannot segment packet: %d\n",
> +					    dev->name, ret);
> +			dev_core_stats_tx_dropped_inc(ovpn->dev);
> +			goto drop;
> +		}
> +
> +		consume_skb(skb);
> +		skb = segments;
> +	}
> +
> +	/* from this moment on, "skb" might be a list */
> +
> +	__skb_queue_head_init(&skb_list);
> +	skb_list_walk_safe(skb, curr, next) {
> +		skb_mark_not_on_list(curr);
> +
> +		curr = skb_share_check(curr, GFP_ATOMIC);
> +		if (unlikely(!curr)) {
> +			net_err_ratelimited("%s: skb_share_check failed\n",
> +					    dev->name);
> +			dev_core_stats_tx_dropped_inc(ovpn->dev);
> +			continue;
> +		}
> +
> +		__skb_queue_tail(&skb_list, curr);
> +	}
> +	skb_list.prev->next = NULL;
> +
> +	ovpn_send(ovpn, skb_list.next, NULL);
> +
> +	return NETDEV_TX_OK;
> +
> +drop:
>   	skb_tx_error(skb);
> -	kfree_skb(skb);
> +	kfree_skb_list(skb);
>   	return NET_XMIT_DROP;
>   }

--
Sergey