[PATCH 3.16 289/410] l2tp: avoid using ->tunnel_sock for getting session's parent tunnel

From: Ben Hutchings
Date: Thu Jun 07 2018 - 10:19:48 EST


3.16.57-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Guillaume Nault <g.nault@xxxxxxxxxxxx>

commit 7198c77aa05560c257ee377ec1f4796812121580 upstream.

Sessions don't need to use l2tp_sock_to_tunnel(xxx->tunnel_sock) for
accessing their parent tunnel. They have the .tunnel field in the
l2tp_session structure for that. Furthermore, in all these cases, the
session is registered, so we're guaranteed that .tunnel isn't NULL and
that the session properly holds a reference on the tunnel.

Signed-off-by: Guillaume Nault <g.nault@xxxxxxxxxxxx>
Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
net/l2tp/l2tp_ppp.c | 66 +++++++++------------------------------------
1 file changed, 12 insertions(+), 54 deletions(-)

--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -312,7 +312,6 @@ static int pppol2tp_sendmsg(struct kiocb
int error;
struct l2tp_session *session;
struct l2tp_tunnel *tunnel;
- struct pppol2tp_session *ps;
int uhlen;

error = -ENOTCONN;
@@ -325,10 +324,7 @@ static int pppol2tp_sendmsg(struct kiocb
if (session == NULL)
goto error;

- ps = l2tp_session_priv(session);
- tunnel = l2tp_sock_to_tunnel(ps->tunnel_sock);
- if (tunnel == NULL)
- goto error_put_sess;
+ tunnel = session->tunnel;

uhlen = (tunnel->encap == L2TP_ENCAPTYPE_UDP) ? sizeof(struct udphdr) : 0;

@@ -339,7 +335,7 @@ static int pppol2tp_sendmsg(struct kiocb
sizeof(ppph) + total_len,
0, GFP_KERNEL);
if (!skb)
- goto error_put_sess_tun;
+ goto error_put_sess;

/* Reserve space for headers. */
skb_reserve(skb, NET_SKB_PAD);
@@ -358,20 +354,17 @@ static int pppol2tp_sendmsg(struct kiocb
total_len);
if (error < 0) {
kfree_skb(skb);
- goto error_put_sess_tun;
+ goto error_put_sess;
}

local_bh_disable();
l2tp_xmit_skb(session, skb, session->hdr_len);
local_bh_enable();

- sock_put(ps->tunnel_sock);
sock_put(sk);

return total_len;

-error_put_sess_tun:
- sock_put(ps->tunnel_sock);
error_put_sess:
sock_put(sk);
error:
@@ -396,10 +389,8 @@ static int pppol2tp_xmit(struct ppp_chan
{
static const u8 ppph[2] = { 0xff, 0x03 };
struct sock *sk = (struct sock *) chan->private;
- struct sock *sk_tun;
struct l2tp_session *session;
struct l2tp_tunnel *tunnel;
- struct pppol2tp_session *ps;
int uhlen, headroom;

if (sock_flag(sk, SOCK_DEAD) || !(sk->sk_state & PPPOX_CONNECTED))
@@ -410,13 +401,7 @@ static int pppol2tp_xmit(struct ppp_chan
if (session == NULL)
goto abort;

- ps = l2tp_session_priv(session);
- sk_tun = ps->tunnel_sock;
- if (sk_tun == NULL)
- goto abort_put_sess;
- tunnel = l2tp_sock_to_tunnel(sk_tun);
- if (tunnel == NULL)
- goto abort_put_sess;
+ tunnel = session->tunnel;

uhlen = (tunnel->encap == L2TP_ENCAPTYPE_UDP) ? sizeof(struct udphdr) : 0;
headroom = NET_SKB_PAD +
@@ -425,7 +410,7 @@ static int pppol2tp_xmit(struct ppp_chan
session->hdr_len + /* L2TP header */
sizeof(ppph); /* PPP header */
if (skb_cow_head(skb, headroom))
- goto abort_put_sess_tun;
+ goto abort_put_sess;

/* Setup PPP header */
__skb_push(skb, sizeof(ppph));
@@ -436,12 +421,10 @@ static int pppol2tp_xmit(struct ppp_chan
l2tp_xmit_skb(session, skb, session->hdr_len);
local_bh_enable();

- sock_put(sk_tun);
sock_put(sk);
+
return 1;

-abort_put_sess_tun:
- sock_put(sk_tun);
abort_put_sess:
sock_put(sk);
abort:
@@ -938,9 +921,7 @@ static int pppol2tp_getname(struct socke
goto end;

pls = l2tp_session_priv(session);
- tunnel = l2tp_sock_to_tunnel(pls->tunnel_sock);
- if (tunnel == NULL)
- goto end_put_sess;
+ tunnel = session->tunnel;

inet = inet_sk(tunnel->sock);
if ((tunnel->version == 2) && (tunnel->sock->sk_family == AF_INET)) {
@@ -1020,8 +1001,6 @@ static int pppol2tp_getname(struct socke
*usockaddr_len = len;
error = 0;

- sock_put(pls->tunnel_sock);
-end_put_sess:
sock_put(sk);
end:
return error;
@@ -1262,7 +1241,6 @@ static int pppol2tp_ioctl(struct socket
struct sock *sk = sock->sk;
struct l2tp_session *session;
struct l2tp_tunnel *tunnel;
- struct pppol2tp_session *ps;
int err;

if (!sk)
@@ -1286,16 +1264,10 @@ static int pppol2tp_ioctl(struct socket
/* Special case: if session's session_id is zero, treat ioctl as a
* tunnel ioctl
*/
- ps = l2tp_session_priv(session);
if ((session->session_id == 0) &&
(session->peer_session_id == 0)) {
- err = -EBADF;
- tunnel = l2tp_sock_to_tunnel(ps->tunnel_sock);
- if (tunnel == NULL)
- goto end_put_sess;
-
+ tunnel = session->tunnel;
err = pppol2tp_tunnel_ioctl(tunnel, cmd, arg);
- sock_put(ps->tunnel_sock);
goto end_put_sess;
}

@@ -1421,7 +1393,6 @@ static int pppol2tp_setsockopt(struct so
struct sock *sk = sock->sk;
struct l2tp_session *session;
struct l2tp_tunnel *tunnel;
- struct pppol2tp_session *ps;
int val;
int err;

@@ -1446,20 +1417,14 @@ static int pppol2tp_setsockopt(struct so

/* Special case: if session_id == 0x0000, treat as operation on tunnel
*/
- ps = l2tp_session_priv(session);
if ((session->session_id == 0) &&
(session->peer_session_id == 0)) {
- err = -EBADF;
- tunnel = l2tp_sock_to_tunnel(ps->tunnel_sock);
- if (tunnel == NULL)
- goto end_put_sess;
-
+ tunnel = session->tunnel;
err = pppol2tp_tunnel_setsockopt(sk, tunnel, optname, val);
- sock_put(ps->tunnel_sock);
- } else
+ } else {
err = pppol2tp_session_setsockopt(sk, session, optname, val);
+ }

-end_put_sess:
sock_put(sk);
end:
return err;
@@ -1547,7 +1512,6 @@ static int pppol2tp_getsockopt(struct so
struct l2tp_tunnel *tunnel;
int val, len;
int err;
- struct pppol2tp_session *ps;

if (level != SOL_PPPOL2TP)
return -EINVAL;
@@ -1571,16 +1535,10 @@ static int pppol2tp_getsockopt(struct so
goto end;

/* Special case: if session_id == 0x0000, treat as operation on tunnel */
- ps = l2tp_session_priv(session);
if ((session->session_id == 0) &&
(session->peer_session_id == 0)) {
- err = -EBADF;
- tunnel = l2tp_sock_to_tunnel(ps->tunnel_sock);
- if (tunnel == NULL)
- goto end_put_sess;
-
+ tunnel = session->tunnel;
err = pppol2tp_tunnel_getsockopt(sk, tunnel, optname, &val);
- sock_put(ps->tunnel_sock);
if (err)
goto end_put_sess;
} else {