Commit f3c66d4e authored by Guillaume Nault's avatar Guillaume Nault Committed by David S. Miller

l2tp: prevent creation of sessions on terminated tunnels

l2tp_tunnel_destruct() sets tunnel->sock to NULL, then removes the
tunnel from the pernet list and finally closes all its sessions.
Therefore, it's possible to add a session to a tunnel that is still
reachable, but for which tunnel->sock has already been reset. This can
make l2tp_session_create() dereference a NULL pointer when calling
sock_hold(tunnel->sock).

This patch adds the .acpt_newsess field to struct l2tp_tunnel, which is
used by l2tp_tunnel_closeall() to prevent addition of new sessions to
tunnels. Resetting tunnel->sock is done after l2tp_tunnel_closeall()
returned, so that l2tp_session_add_to_tunnel() can safely take a
reference on it when .acpt_newsess is true.

The .acpt_newsess field is modified in l2tp_tunnel_closeall(), rather
than in l2tp_tunnel_destruct(), so that it benefits all tunnel removal
mechanisms. E.g. on UDP tunnels, a session could be added to a tunnel
after l2tp_udp_encap_destroy() proceeded. This would prevent the tunnel
from being removed because of the references held by this new session
on the tunnel and its socket. Even though the session could be removed
manually later on, this defeats the purpose of
commit 9980d001 ("l2tp: add udp encap socket destroy handler").

Fixes: fd558d18 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
Signed-off-by: default avatarGuillaume Nault <g.nault@alphalink.fr>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 4113f36b
...@@ -329,13 +329,21 @@ static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel, ...@@ -329,13 +329,21 @@ static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel,
struct hlist_head *g_head; struct hlist_head *g_head;
struct hlist_head *head; struct hlist_head *head;
struct l2tp_net *pn; struct l2tp_net *pn;
int err;
head = l2tp_session_id_hash(tunnel, session->session_id); head = l2tp_session_id_hash(tunnel, session->session_id);
write_lock_bh(&tunnel->hlist_lock); write_lock_bh(&tunnel->hlist_lock);
if (!tunnel->acpt_newsess) {
err = -ENODEV;
goto err_tlock;
}
hlist_for_each_entry(session_walk, head, hlist) hlist_for_each_entry(session_walk, head, hlist)
if (session_walk->session_id == session->session_id) if (session_walk->session_id == session->session_id) {
goto exist; err = -EEXIST;
goto err_tlock;
}
if (tunnel->version == L2TP_HDR_VER_3) { if (tunnel->version == L2TP_HDR_VER_3) {
pn = l2tp_pernet(tunnel->l2tp_net); pn = l2tp_pernet(tunnel->l2tp_net);
...@@ -343,12 +351,21 @@ static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel, ...@@ -343,12 +351,21 @@ static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel,
session->session_id); session->session_id);
spin_lock_bh(&pn->l2tp_session_hlist_lock); spin_lock_bh(&pn->l2tp_session_hlist_lock);
hlist_for_each_entry(session_walk, g_head, global_hlist) hlist_for_each_entry(session_walk, g_head, global_hlist)
if (session_walk->session_id == session->session_id) if (session_walk->session_id == session->session_id) {
goto exist_glob; err = -EEXIST;
goto err_tlock_pnlock;
}
l2tp_tunnel_inc_refcount(tunnel);
sock_hold(tunnel->sock);
hlist_add_head_rcu(&session->global_hlist, g_head); hlist_add_head_rcu(&session->global_hlist, g_head);
spin_unlock_bh(&pn->l2tp_session_hlist_lock); spin_unlock_bh(&pn->l2tp_session_hlist_lock);
} else {
l2tp_tunnel_inc_refcount(tunnel);
sock_hold(tunnel->sock);
} }
hlist_add_head(&session->hlist, head); hlist_add_head(&session->hlist, head);
...@@ -356,12 +373,12 @@ static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel, ...@@ -356,12 +373,12 @@ static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel,
return 0; return 0;
exist_glob: err_tlock_pnlock:
spin_unlock_bh(&pn->l2tp_session_hlist_lock); spin_unlock_bh(&pn->l2tp_session_hlist_lock);
exist: err_tlock:
write_unlock_bh(&tunnel->hlist_lock); write_unlock_bh(&tunnel->hlist_lock);
return -EEXIST; return err;
} }
/* Lookup a tunnel by id /* Lookup a tunnel by id
...@@ -1251,7 +1268,6 @@ static void l2tp_tunnel_destruct(struct sock *sk) ...@@ -1251,7 +1268,6 @@ static void l2tp_tunnel_destruct(struct sock *sk)
/* Remove hooks into tunnel socket */ /* Remove hooks into tunnel socket */
sk->sk_destruct = tunnel->old_sk_destruct; sk->sk_destruct = tunnel->old_sk_destruct;
sk->sk_user_data = NULL; sk->sk_user_data = NULL;
tunnel->sock = NULL;
/* Remove the tunnel struct from the tunnel list */ /* Remove the tunnel struct from the tunnel list */
pn = l2tp_pernet(tunnel->l2tp_net); pn = l2tp_pernet(tunnel->l2tp_net);
...@@ -1261,6 +1277,8 @@ static void l2tp_tunnel_destruct(struct sock *sk) ...@@ -1261,6 +1277,8 @@ static void l2tp_tunnel_destruct(struct sock *sk)
atomic_dec(&l2tp_tunnel_count); atomic_dec(&l2tp_tunnel_count);
l2tp_tunnel_closeall(tunnel); l2tp_tunnel_closeall(tunnel);
tunnel->sock = NULL;
l2tp_tunnel_dec_refcount(tunnel); l2tp_tunnel_dec_refcount(tunnel);
/* Call the original destructor */ /* Call the original destructor */
...@@ -1285,6 +1303,7 @@ void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel) ...@@ -1285,6 +1303,7 @@ void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
tunnel->name); tunnel->name);
write_lock_bh(&tunnel->hlist_lock); write_lock_bh(&tunnel->hlist_lock);
tunnel->acpt_newsess = false;
for (hash = 0; hash < L2TP_HASH_SIZE; hash++) { for (hash = 0; hash < L2TP_HASH_SIZE; hash++) {
again: again:
hlist_for_each_safe(walk, tmp, &tunnel->session_hlist[hash]) { hlist_for_each_safe(walk, tmp, &tunnel->session_hlist[hash]) {
...@@ -1581,6 +1600,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 ...@@ -1581,6 +1600,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
tunnel->magic = L2TP_TUNNEL_MAGIC; tunnel->magic = L2TP_TUNNEL_MAGIC;
sprintf(&tunnel->name[0], "tunl %u", tunnel_id); sprintf(&tunnel->name[0], "tunl %u", tunnel_id);
rwlock_init(&tunnel->hlist_lock); rwlock_init(&tunnel->hlist_lock);
tunnel->acpt_newsess = true;
/* The net we belong to */ /* The net we belong to */
tunnel->l2tp_net = net; tunnel->l2tp_net = net;
...@@ -1829,11 +1849,6 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn ...@@ -1829,11 +1849,6 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
return ERR_PTR(err); return ERR_PTR(err);
} }
l2tp_tunnel_inc_refcount(tunnel);
/* Ensure tunnel socket isn't deleted */
sock_hold(tunnel->sock);
/* Ignore management session in session count value */ /* Ignore management session in session count value */
if (session->session_id != 0) if (session->session_id != 0)
atomic_inc(&l2tp_session_count); atomic_inc(&l2tp_session_count);
......
...@@ -162,6 +162,10 @@ struct l2tp_tunnel { ...@@ -162,6 +162,10 @@ struct l2tp_tunnel {
int magic; /* Should be L2TP_TUNNEL_MAGIC */ int magic; /* Should be L2TP_TUNNEL_MAGIC */
struct rcu_head rcu; struct rcu_head rcu;
rwlock_t hlist_lock; /* protect session_hlist */ rwlock_t hlist_lock; /* protect session_hlist */
bool acpt_newsess; /* Indicates whether this
* tunnel accepts new sessions.
* Protected by hlist_lock.
*/
struct hlist_head session_hlist[L2TP_HASH_SIZE]; struct hlist_head session_hlist[L2TP_HASH_SIZE];
/* hashed list of sessions, /* hashed list of sessions,
* hashed by id */ * hashed by id */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment