netdev - Re: [PATCH net-next] l2tp: fix ICMP error handling for UDP-encap sockets

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: <20240503153214.3432d313@kernel.org>
Date: Fri, 3 May 2024 15:32:14 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Tom Parkin <tparkin@...alix.com>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next] l2tp: fix ICMP error handling for UDP-encap
 sockets

On Tue, 30 Apr 2024 15:03:43 +0100 Tom Parkin wrote:
> Subject: [PATCH net-next] l2tp: fix ICMP error handling for UDP-encap sockets

Seems like we should target it at net? Description indicates it's 
a clear regression.

> +static void l2tp_udp_encap_err_recv(struct sock *sk, struct sk_buff *skb, int err,
> +				    __be16 port, u32 info, u8 *payload)
> +{
> +	struct l2tp_tunnel *tunnel = l2tp_sk_to_tunnel(sk);
> +
> +	if (!tunnel || tunnel->fd < 0)
> +		return;

not: the !tunnel can't happen, right?

> +	sk->sk_err = err;
> +	sk_error_report(sk);
> +
> +	if (ip_hdr(skb)->version == IPVERSION) {
> +		if (inet_test_bit(RECVERR, sk))
> +			return ip_icmp_error(sk, skb, err, port, info, payload);
> +	}
> +#if IS_ENABLED(CONFIG_IPV6)
> +	else
> +		if (inet6_test_bit(RECVERR6, sk))
> +			return ipv6_icmp_error(sk, skb, err, port, info, payload);
> +#endif

nit: mismatch on the braces here, this would be more usual:

+	if (ip_hdr(skb)->version == IPVERSION) {
+		if (inet_test_bit(RECVERR, sk))
+			return ip_icmp_error(sk, skb, err, port, info, payload);
+#if IS_ENABLED(CONFIG_IPV6)
+	} else {
+		if (inet6_test_bit(RECVERR6, sk))
+			return ipv6_icmp_error(sk, skb, err, port, info, payload);
+#endif
+	}

+}