netdev - Re: [PATCH] l2tp: Support different protocol versions with same IP/port quadruple

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: <e6939885-7ba3-e46c-779a-ac5f8367d08c@katalix.com>
Date: Thu, 9 May 2024 22:08:26 +0100
From: James Chapman <jchapman@...alix.com>
To: Samuel Thibault <samuel.thibault@...-lyon.org>,
 linux-kernel@...r.kernel.org, tparkin@...alix.com,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH] l2tp: Support different protocol versions with same
 IP/port quadruple

On 09/05/2024 21:58, Samuel Thibault wrote:
> 628bc3e5a1be ("l2tp: Support several sockets with same IP/port quadruple")
> added support for several L2TPv2 tunnels using the same IP/port quadruple,
> but if an L2TPv3 socket exists it could eat all the trafic. We thus have to
> first use the version from the packet to get the proper tunnel, and only
> then check that the version matches.
>
> Signed-off-by: Samuel Thibault <samuel.thibault@...-lyon.org>

LGTM

Reviewed-by: James Chapman <jchapman@...alix.com>


> ---
>   net/l2tp/l2tp_core.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index 2ab45e3f48bf..7d519a46a844 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -820,13 +820,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
>   	/* Get L2TP header flags */
>   	hdrflags = ntohs(*(__be16 *)ptr);
>   
> -	/* Check protocol version */
> +	/* Get protocol version */
>   	version = hdrflags & L2TP_HDR_VER_MASK;
> -	if (version != tunnel->version) {
> -		pr_debug_ratelimited("%s: recv protocol version mismatch: got %d expected %d\n",
> -				     tunnel->name, version, tunnel->version);
> -		goto invalid;
> -	}
>   
>   	/* Get length of L2TP packet */
>   	length = skb->len;
> @@ -838,7 +833,7 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
>   	/* Skip flags */
>   	ptr += 2;
>   
> -	if (tunnel->version == L2TP_HDR_VER_2) {
> +	if (version == L2TP_HDR_VER_2) {
>   		/* If length is present, skip it */
>   		if (hdrflags & L2TP_HDRFLAG_L)
>   			ptr += 2;
> @@ -855,7 +850,7 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
>   			struct l2tp_tunnel *alt_tunnel;
>   
>   			alt_tunnel = l2tp_tunnel_get(tunnel->l2tp_net, tunnel_id);
> -			if (!alt_tunnel || alt_tunnel->version != L2TP_HDR_VER_2)
> +			if (!alt_tunnel)
>   				goto pass;
>   			tunnel = alt_tunnel;
>   		}
> @@ -869,6 +864,13 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
>   		ptr += 4;
>   	}
>   
> +	/* Check protocol version */
> +	if (version != tunnel->version) {
> +		pr_debug_ratelimited("%s: recv protocol version mismatch: got %d expected %d\n",
> +				     tunnel->name, version, tunnel->version);
> +		goto invalid;
> +	}
> +
>   	/* Find the session context */
>   	session = l2tp_tunnel_get_session(tunnel, session_id);
>   	if (!session || !session->recv_skb) {