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

l2tp: fix duplicate session creation

l2tp_session_create() relies on its caller for checking for duplicate
sessions. This is racy since a session can be concurrently inserted
after the caller's verification.

Fix this by letting l2tp_session_create() verify sessions uniqueness
upon insertion. Callers need to be adapted to check for
l2tp_session_create()'s return code instead of calling
l2tp_session_find().

pppol2tp_connect() is a bit special because it has to work on existing
sessions (if they're not connected) or to create a new session if none
is found. When acting on a preexisting session, a reference must be
held or it could go away on us. So we have to use l2tp_session_get()
instead of l2tp_session_find() and drop the reference before exiting.

Fixes: d9e31d17 ("l2tp: Add L2TP ethernet pseudowire support")
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 57377d63
...@@ -374,6 +374,48 @@ struct l2tp_session *l2tp_session_find_by_ifname(struct net *net, char *ifname) ...@@ -374,6 +374,48 @@ struct l2tp_session *l2tp_session_find_by_ifname(struct net *net, char *ifname)
} }
EXPORT_SYMBOL_GPL(l2tp_session_find_by_ifname); EXPORT_SYMBOL_GPL(l2tp_session_find_by_ifname);
static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel,
struct l2tp_session *session)
{
struct l2tp_session *session_walk;
struct hlist_head *g_head;
struct hlist_head *head;
struct l2tp_net *pn;
head = l2tp_session_id_hash(tunnel, session->session_id);
write_lock_bh(&tunnel->hlist_lock);
hlist_for_each_entry(session_walk, head, hlist)
if (session_walk->session_id == session->session_id)
goto exist;
if (tunnel->version == L2TP_HDR_VER_3) {
pn = l2tp_pernet(tunnel->l2tp_net);
g_head = l2tp_session_id_hash_2(l2tp_pernet(tunnel->l2tp_net),
session->session_id);
spin_lock_bh(&pn->l2tp_session_hlist_lock);
hlist_for_each_entry(session_walk, g_head, global_hlist)
if (session_walk->session_id == session->session_id)
goto exist_glob;
hlist_add_head_rcu(&session->global_hlist, g_head);
spin_unlock_bh(&pn->l2tp_session_hlist_lock);
}
hlist_add_head(&session->hlist, head);
write_unlock_bh(&tunnel->hlist_lock);
return 0;
exist_glob:
spin_unlock_bh(&pn->l2tp_session_hlist_lock);
exist:
write_unlock_bh(&tunnel->hlist_lock);
return -EEXIST;
}
/* Lookup a tunnel by id /* Lookup a tunnel by id
*/ */
struct l2tp_tunnel *l2tp_tunnel_find(struct net *net, u32 tunnel_id) struct l2tp_tunnel *l2tp_tunnel_find(struct net *net, u32 tunnel_id)
...@@ -1785,6 +1827,7 @@ EXPORT_SYMBOL_GPL(l2tp_session_set_header_len); ...@@ -1785,6 +1827,7 @@ EXPORT_SYMBOL_GPL(l2tp_session_set_header_len);
struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunnel, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg) struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunnel, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg)
{ {
struct l2tp_session *session; struct l2tp_session *session;
int err;
session = kzalloc(sizeof(struct l2tp_session) + priv_size, GFP_KERNEL); session = kzalloc(sizeof(struct l2tp_session) + priv_size, GFP_KERNEL);
if (session != NULL) { if (session != NULL) {
...@@ -1840,6 +1883,13 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn ...@@ -1840,6 +1883,13 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
l2tp_session_set_header_len(session, tunnel->version); l2tp_session_set_header_len(session, tunnel->version);
err = l2tp_session_add_to_tunnel(tunnel, session);
if (err) {
kfree(session);
return ERR_PTR(err);
}
/* Bump the reference count. The session context is deleted /* Bump the reference count. The session context is deleted
* only when this drops to zero. * only when this drops to zero.
*/ */
...@@ -1849,28 +1899,14 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn ...@@ -1849,28 +1899,14 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
/* Ensure tunnel socket isn't deleted */ /* Ensure tunnel socket isn't deleted */
sock_hold(tunnel->sock); sock_hold(tunnel->sock);
/* Add session to the tunnel's hash list */
write_lock_bh(&tunnel->hlist_lock);
hlist_add_head(&session->hlist,
l2tp_session_id_hash(tunnel, session_id));
write_unlock_bh(&tunnel->hlist_lock);
/* And to the global session list if L2TPv3 */
if (tunnel->version != L2TP_HDR_VER_2) {
struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net);
spin_lock_bh(&pn->l2tp_session_hlist_lock);
hlist_add_head_rcu(&session->global_hlist,
l2tp_session_id_hash_2(pn, session_id));
spin_unlock_bh(&pn->l2tp_session_hlist_lock);
}
/* 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);
}
return session; return session;
}
return ERR_PTR(-ENOMEM);
} }
EXPORT_SYMBOL_GPL(l2tp_session_create); EXPORT_SYMBOL_GPL(l2tp_session_create);
......
...@@ -221,12 +221,6 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p ...@@ -221,12 +221,6 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p
goto out; goto out;
} }
session = l2tp_session_find(net, tunnel, session_id);
if (session) {
rc = -EEXIST;
goto out;
}
if (cfg->ifname) { if (cfg->ifname) {
dev = dev_get_by_name(net, cfg->ifname); dev = dev_get_by_name(net, cfg->ifname);
if (dev) { if (dev) {
...@@ -240,8 +234,8 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p ...@@ -240,8 +234,8 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p
session = l2tp_session_create(sizeof(*spriv), tunnel, session_id, session = l2tp_session_create(sizeof(*spriv), tunnel, session_id,
peer_session_id, cfg); peer_session_id, cfg);
if (!session) { if (IS_ERR(session)) {
rc = -ENOMEM; rc = PTR_ERR(session);
goto out; goto out;
} }
......
...@@ -583,6 +583,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, ...@@ -583,6 +583,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
int error = 0; int error = 0;
u32 tunnel_id, peer_tunnel_id; u32 tunnel_id, peer_tunnel_id;
u32 session_id, peer_session_id; u32 session_id, peer_session_id;
bool drop_refcnt = false;
int ver = 2; int ver = 2;
int fd; int fd;
...@@ -684,37 +685,37 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, ...@@ -684,37 +685,37 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
if (tunnel->peer_tunnel_id == 0) if (tunnel->peer_tunnel_id == 0)
tunnel->peer_tunnel_id = peer_tunnel_id; tunnel->peer_tunnel_id = peer_tunnel_id;
/* Create session if it doesn't already exist. We handle the session = l2tp_session_get(sock_net(sk), tunnel, session_id, false);
* case where a session was previously created by the netlink if (session) {
* interface by checking that the session doesn't already have drop_refcnt = true;
* a socket and its tunnel socket are what we expect. If any ps = l2tp_session_priv(session);
* of those checks fail, return EEXIST to the caller.
*/
session = l2tp_session_find(sock_net(sk), tunnel, session_id);
if (session == NULL) {
/* Default MTU must allow space for UDP/L2TP/PPP
* headers.
*/
cfg.mtu = cfg.mru = 1500 - PPPOL2TP_HEADER_OVERHEAD;
/* Allocate and initialize a new session context. */ /* Using a pre-existing session is fine as long as it hasn't
session = l2tp_session_create(sizeof(struct pppol2tp_session), * been connected yet.
tunnel, session_id, */
peer_session_id, &cfg); if (ps->sock) {
if (session == NULL) { error = -EEXIST;
error = -ENOMEM;
goto end; goto end;
} }
} else {
ps = l2tp_session_priv(session); /* consistency checks */
if (ps->tunnel_sock != tunnel->sock) {
error = -EEXIST; error = -EEXIST;
if (ps->sock != NULL)
goto end; goto end;
}
} else {
/* Default MTU must allow space for UDP/L2TP/PPP headers */
cfg.mtu = 1500 - PPPOL2TP_HEADER_OVERHEAD;
cfg.mru = cfg.mtu;
/* consistency checks */ session = l2tp_session_create(sizeof(struct pppol2tp_session),
if (ps->tunnel_sock != tunnel->sock) tunnel, session_id,
peer_session_id, &cfg);
if (IS_ERR(session)) {
error = PTR_ERR(session);
goto end; goto end;
} }
}
/* Associate session with its PPPoL2TP socket */ /* Associate session with its PPPoL2TP socket */
ps = l2tp_session_priv(session); ps = l2tp_session_priv(session);
...@@ -778,6 +779,8 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, ...@@ -778,6 +779,8 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
session->name); session->name);
end: end:
if (drop_refcnt)
l2tp_session_dec_refcount(session);
release_sock(sk); release_sock(sk);
return error; return error;
...@@ -805,12 +808,6 @@ static int pppol2tp_session_create(struct net *net, u32 tunnel_id, u32 session_i ...@@ -805,12 +808,6 @@ static int pppol2tp_session_create(struct net *net, u32 tunnel_id, u32 session_i
if (tunnel->sock == NULL) if (tunnel->sock == NULL)
goto out; goto out;
/* Check that this session doesn't already exist */
error = -EEXIST;
session = l2tp_session_find(net, tunnel, session_id);
if (session != NULL)
goto out;
/* Default MTU values. */ /* Default MTU values. */
if (cfg->mtu == 0) if (cfg->mtu == 0)
cfg->mtu = 1500 - PPPOL2TP_HEADER_OVERHEAD; cfg->mtu = 1500 - PPPOL2TP_HEADER_OVERHEAD;
...@@ -818,12 +815,13 @@ static int pppol2tp_session_create(struct net *net, u32 tunnel_id, u32 session_i ...@@ -818,12 +815,13 @@ static int pppol2tp_session_create(struct net *net, u32 tunnel_id, u32 session_i
cfg->mru = cfg->mtu; cfg->mru = cfg->mtu;
/* Allocate and initialize a new session context. */ /* Allocate and initialize a new session context. */
error = -ENOMEM;
session = l2tp_session_create(sizeof(struct pppol2tp_session), session = l2tp_session_create(sizeof(struct pppol2tp_session),
tunnel, session_id, tunnel, session_id,
peer_session_id, cfg); peer_session_id, cfg);
if (session == NULL) if (IS_ERR(session)) {
error = PTR_ERR(session);
goto out; goto out;
}
ps = l2tp_session_priv(session); ps = l2tp_session_priv(session);
ps->tunnel_sock = tunnel->sock; ps->tunnel_sock = tunnel->sock;
......
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