From 90904ff5f958a215cc3d26f957a46e80fa178470 Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Wed, 13 Jun 2018 15:09:18 +0200 Subject: [PATCH 1/4] l2tp: fix pseudo-wire type for sessions created by pppol2tp_connect() Define cfg.pw_type so that the new session is created with its .pwtype field properly set (L2TP_PWTYPE_PPP). Not setting the pseudo-wire type had several annoying effects: * Invalid value returned in the L2TP_ATTR_PW_TYPE attribute when dumping sessions with the netlink API. * Impossibility to delete the session using the netlink API (because l2tp_nl_cmd_session_delete() gets the deletion callback function from an array indexed by the session's pseudo-wire type). Also, there are several cases where we should check a session's pseudo-wire type. For example, pppol2tp_connect() should refuse to connect a session that is not PPPoL2TP, but that requires the session's .pwtype field to be properly set. Fixes: f7faffa3ff8e ("l2tp: Add L2TPv3 protocol support") Signed-off-by: Guillaume Nault Signed-off-by: David S. Miller --- net/l2tp/l2tp_ppp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index b56cb1df4fc07..270a0a999eafc 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -751,6 +751,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, /* Default MTU must allow space for UDP/L2TP/PPP headers */ cfg.mtu = 1500 - PPPOL2TP_HEADER_OVERHEAD; cfg.mru = cfg.mtu; + cfg.pw_type = L2TP_PWTYPE_PPP; session = l2tp_session_create(sizeof(struct pppol2tp_session), tunnel, session_id, From 7ac6ab1f8a38ba7f8d97f95475bb6a2575db4658 Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Wed, 13 Jun 2018 15:09:19 +0200 Subject: [PATCH 2/4] l2tp: only accept PPP sessions in pppol2tp_connect() l2tp_session_priv() returns a struct pppol2tp_session pointer only for PPPoL2TP sessions. In particular, if the session is an L2TP_PWTYPE_ETH pseudo-wire, l2tp_session_priv() returns a pointer to an l2tp_eth_sess structure, which is much smaller than struct pppol2tp_session. This leads to invalid memory dereference when trying to lock ps->sk_lock. Fixes: d9e31d17ceba ("l2tp: Add L2TP ethernet pseudowire support") Signed-off-by: Guillaume Nault Signed-off-by: David S. Miller --- net/l2tp/l2tp_ppp.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 270a0a999eafc..8b3b6947a07d7 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -734,6 +734,12 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, session = l2tp_session_get(sock_net(sk), tunnel, session_id); if (session) { drop_refcnt = true; + + if (session->pwtype != L2TP_PWTYPE_PPP) { + error = -EPROTOTYPE; + goto end; + } + ps = l2tp_session_priv(session); /* Using a pre-existing session is fine as long as it hasn't From 3e1bc8bf974e2d4e7beb842a4c801c2542eff3bd Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Wed, 13 Jun 2018 15:09:20 +0200 Subject: [PATCH 3/4] l2tp: prevent pppol2tp_connect() from creating kernel sockets If 'fd' is negative, l2tp_tunnel_create() creates a tunnel socket using the configuration passed in 'tcfg'. Currently, pppol2tp_connect() sets the relevant fields to zero, tricking l2tp_tunnel_create() into setting up an unusable kernel socket. We can't set 'tcfg' with the required fields because there's no way to get them from the current connect() parameters. So let's restrict kernel sockets creation to the netlink API, which is the original use case. Fixes: 789a4a2c61d8 ("l2tp: Add support for static unmanaged L2TPv3 tunnels") Signed-off-by: Guillaume Nault Signed-off-by: David S. Miller --- net/l2tp/l2tp_ppp.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 8b3b6947a07d7..1b24f76ae210e 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -701,6 +701,15 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, .encap = L2TP_ENCAPTYPE_UDP, .debug = 0, }; + + /* Prevent l2tp_tunnel_register() from trying to set up + * a kernel socket. + */ + if (fd < 0) { + error = -EBADF; + goto end; + } + error = l2tp_tunnel_create(sock_net(sk), fd, ver, tunnel_id, peer_tunnel_id, &tcfg, &tunnel); if (error < 0) goto end; From bda06be2158c7aa7e41b15500c4d3840369c19a6 Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Wed, 13 Jun 2018 15:09:21 +0200 Subject: [PATCH 4/4] l2tp: clean up stale tunnel or session in pppol2tp_connect's error path pppol2tp_connect() may create a tunnel or a session. Remove them in case of error. Fixes: fd558d186df2 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts") Signed-off-by: Guillaume Nault Signed-off-by: David S. Miller --- net/l2tp/l2tp_ppp.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 1b24f76ae210e..f429fed06a1e7 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -612,6 +612,8 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, u32 session_id, peer_session_id; bool drop_refcnt = false; bool drop_tunnel = false; + bool new_session = false; + bool new_tunnel = false; int ver = 2; int fd; @@ -722,6 +724,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, goto end; } drop_tunnel = true; + new_tunnel = true; } } else { /* Error if we can't find the tunnel */ @@ -788,6 +791,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, goto end; } drop_refcnt = true; + new_session = true; } /* Special case: if source & dest session_id == 0x0000, this @@ -834,6 +838,12 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, session->name); end: + if (error) { + if (new_session) + l2tp_session_delete(session); + if (new_tunnel) + l2tp_tunnel_delete(tunnel); + } if (drop_refcnt) l2tp_session_dec_refcount(session); if (drop_tunnel)