When creating a new socket, l2tp_tunnel_create() ensures that
such socket is connected, but when using a socket provided by
the user space, no check is done on the socket state.
This may foul the later check for ipv6 sockets that are
ipv4-mapped, e.g. in case of unconnected ipv6 socket bound to
ipv4 address.
Moreover the connection status and/or peer of a user-space
controlled socket may change at runtime.
This change addresses the issues:
* explicitly checking for TCP_ESTABLISHED for user space provided sockets
* dropping the v4mapped flag usage - it can become outdated - and
explicitly invoking ipv6_addr_v4mapped() instead
* refreshing the inet_sk copy of ipv4-mapped ipv6 address at xmit time.
The issue is apparently there since ancient times.
v1 -> v2: (many thanks to Guillaume)
- with csum issue introduced in v1
- replace pr_err with pr_debug
- fix build issue with IPV6 disabled
- move l2tp_sk_is_v4mapped in l2tp_core.c
Reported-and-tested-by: syzbot+92fa328176eb07e4a...@syzkaller.appspotmail.com
Fixes: 3557baabf280 ("[L2TP]: PPP over L2TP driver core")
Signed-off-by: Paolo Abeni <pab...@redhat.com>
---
net/l2tp/l2tp_core.c | 65 ++++++++++++++++++++++++++++++++++++++++------------
net/l2tp/l2tp_core.h | 3 ---
2 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 83421c6f0bef..9726e3f37745 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -111,6 +111,19 @@ struct l2tp_net {
spinlock_t l2tp_session_hlist_lock;
};
+#if IS_ENABLED(CONFIG_IPV6)
+static bool l2tp_sk_is_v4mapped(struct sock *sk)
+{
+ return sk->sk_family == PF_INET6 &&
+ ipv6_addr_v4mapped(&sk->sk_v6_daddr);
+}
+
+static bool l2tp_sk_is_v6(struct sock *sk)
+{
+ return sk->sk_family == PF_INET6 &&
+ !ipv6_addr_v4mapped(&sk->sk_v6_daddr);
+}
+#endif
static inline struct l2tp_tunnel *l2tp_tunnel(struct sock *sk)
{
@@ -1049,7 +1062,7 @@ static int l2tp_xmit_core(struct l2tp_session *session,
struct sk_buff *skb,
/* Queue the packet to IP for output */
skb->ignore_df = 1;
#if IS_ENABLED(CONFIG_IPV6)
- if (tunnel->sock->sk_family == PF_INET6 && !tunnel->v4mapped)
+ if (l2tp_sk_is_v6(tunnel->sock))
error = inet6_csk_xmit(tunnel->sock, skb, NULL);
else
#endif
@@ -1112,11 +1125,32 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct
sk_buff *skb, int hdr_len
goto out_unlock;
}
+ /* The user-space may change the connection status for the user-space
+ * provided socket at run time: we must check it under the socket lock
+ */
+ inet = inet_sk(sk);
+ if (tunnel->fd >= 0) {
+ if (sk->sk_state != TCP_ESTABLISHED) {
+ ret = NET_XMIT_DROP;
+ goto out_unlock;
+ }
+
+#if IS_ENABLED(CONFIG_IPV6)
+ /* If the uses space changes the ipv4-mapped ipv6 address,
+ * the kernel copy of the ipv4 address is not updated.
+ * Refresh it only if needed, to avoid dirtying the socket
+ * on each packet.
+ */
+ if (l2tp_sk_is_v4mapped(sk) &&
+ inet->inet_daddr != sk->sk_v6_daddr.s6_addr32[3])
+ inet->inet_daddr = sk->sk_v6_daddr.s6_addr32[3];
+#endif
+ }
+
/* Get routing info from the tunnel socket */
skb_dst_drop(skb);
skb_dst_set(skb, dst_clone(__sk_dst_check(sk, 0)));
- inet = inet_sk(sk);
fl = &inet->cork.fl;
switch (tunnel->encap) {
case L2TP_ENCAPTYPE_UDP:
@@ -1131,7 +1165,7 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct
sk_buff *skb, int hdr_len
/* Calculate UDP checksum if configured to do so */
#if IS_ENABLED(CONFIG_IPV6)
- if (sk->sk_family == PF_INET6 && !tunnel->v4mapped)
+ if (l2tp_sk_is_v6(sk))
udp6_set_csum(udp_get_no_check6_tx(sk),
skb, &inet6_sk(sk)->saddr,
&sk->sk_v6_daddr, udp_len);
@@ -1449,6 +1483,14 @@ int l2tp_tunnel_create(struct net *net, int fd, int
version, u32 tunnel_id, u32
err = -EINVAL;
goto err;
}
+
+ /* Reject unconnected sockets */
+ if (sock->sk->sk_state != TCP_ESTABLISHED) {
+ pr_debug("tunl %u: sock fd=%d is unconnected\n",
+ tunnel_id, fd);
+ err = -EINVAL;
+ goto err;
+ }
}
sk = sock->sk;
@@ -1508,20 +1550,13 @@ int l2tp_tunnel_create(struct net *net, int fd, int
version, u32 tunnel_id, u32
tunnel->debug = cfg->debug;
#if IS_ENABLED(CONFIG_IPV6)
- if (sk->sk_family == PF_INET6) {
+ if (l2tp_sk_is_v4mapped(sk)) {
struct ipv6_pinfo *np = inet6_sk(sk);
+ struct inet_sock *inet = inet_sk(sk);
- if (ipv6_addr_v4mapped(&np->saddr) &&
- ipv6_addr_v4mapped(&sk->sk_v6_daddr)) {
- struct inet_sock *inet = inet_sk(sk);
-
- tunnel->v4mapped = true;
- inet->inet_saddr = np->saddr.s6_addr32[3];
- inet->inet_rcv_saddr = sk->sk_v6_rcv_saddr.s6_addr32[3];
- inet->inet_daddr = sk->sk_v6_daddr.s6_addr32[3];
- } else {
- tunnel->v4mapped = false;
- }
+ inet->inet_saddr = np->saddr.s6_addr32[3];
+ inet->inet_rcv_saddr = sk->sk_v6_rcv_saddr.s6_addr32[3];
+ inet->inet_daddr = sk->sk_v6_daddr.s6_addr32[3];
}
#endif
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index a1aa9550f04e..2718d0b284d0 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -188,9 +188,6 @@ struct l2tp_tunnel {
struct sock *sock; /* Parent socket */
int fd; /* Parent fd, if tunnel socket
* was created by userspace */
-#if IS_ENABLED(CONFIG_IPV6)
- bool v4mapped;
-#endif
struct work_struct del_work;
--
2.14.3
