3.2.99-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Guillaume Nault g.nault@alphalink.fr
commit 3953ae7b218df4d1e544b98a393666f9ae58a78c upstream.
Sessions created by l2tp_session_create() aren't fully initialised: some pseudo-wire specific operations need to be done before making the session usable. Therefore the PPP and Ethernet pseudo-wires continue working on the returned l2tp session while it's already been exposed to the rest of the system. This can lead to various issues. In particular, the session may enter the deletion process before having been fully initialised, which will confuse the session removal code.
This patch moves session registration out of l2tp_session_create(), so that callers can control when the session is exposed to the rest of the system. This is done by the new l2tp_session_register() function.
Only pppol2tp_session_create() can be easily converted to avoid modifying its session after registration (the debug message is dropped in order to avoid the need for holding a reference on the session).
For pppol2tp_connect() and l2tp_eth_create()), more work is needed. That'll be done in followup patches. For now, let's just register the session right after its creation, like it was done before. The only difference is that we can easily take a reference on the session before registering it, so, at least, we're sure it's not going to be freed while we're working on it.
Signed-off-by: Guillaume Nault g.nault@alphalink.fr Signed-off-by: David S. Miller davem@davemloft.net [bwh: Backported to 3.2: adjust context] Signed-off-by: Ben Hutchings ben@decadent.org.uk --- net/l2tp/l2tp_core.c | 21 +++++++-------------- net/l2tp/l2tp_core.h | 3 +++ net/l2tp/l2tp_eth.c | 9 +++++++++ net/l2tp/l2tp_ppp.c | 23 +++++++++++++++++------ 4 files changed, 36 insertions(+), 20 deletions(-)
--- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -327,8 +327,8 @@ struct l2tp_session *l2tp_session_get_by } EXPORT_SYMBOL_GPL(l2tp_session_get_by_ifname);
-static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel, - struct l2tp_session *session) +int l2tp_session_register(struct l2tp_session *session, + struct l2tp_tunnel *tunnel) { struct l2tp_session *session_walk; struct hlist_head *g_head; @@ -377,6 +377,10 @@ static int l2tp_session_add_to_tunnel(st hlist_add_head(&session->hlist, head); write_unlock_bh(&tunnel->hlist_lock);
+ /* Ignore management session in session count value */ + if (session->session_id != 0) + atomic_inc(&l2tp_session_count); + return 0;
err_tlock_pnlock: @@ -386,6 +390,7 @@ err_tlock:
return err; } +EXPORT_SYMBOL_GPL(l2tp_session_register);
/* Lookup a tunnel by id */ @@ -1703,7 +1708,6 @@ static void 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 *session; - int err;
session = kzalloc(sizeof(struct l2tp_session) + priv_size, GFP_KERNEL); if (session != NULL) { @@ -1752,17 +1756,6 @@ struct l2tp_session *l2tp_session_create
l2tp_session_inc_refcount(session);
- err = l2tp_session_add_to_tunnel(tunnel, session); - if (err) { - kfree(session); - - return ERR_PTR(err); - } - - /* Ignore management session in session count value */ - if (session->session_id != 0) - atomic_inc(&l2tp_session_count); - return session; }
--- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -246,6 +246,8 @@ extern struct l2tp_tunnel *l2tp_tunnel_f extern int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg, struct l2tp_tunnel **tunnelp); extern int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel); extern 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); +int l2tp_session_register(struct l2tp_session *session, + struct l2tp_tunnel *tunnel); extern int l2tp_session_delete(struct l2tp_session *session); extern void l2tp_session_free(struct l2tp_session *session); extern void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, unsigned char *ptr, unsigned char *optr, u16 hdrflags, int length, int (*payload_hook)(struct sk_buff *skb)); --- a/net/l2tp/l2tp_eth.c +++ b/net/l2tp/l2tp_eth.c @@ -194,6 +194,13 @@ static int l2tp_eth_create(struct net *n goto out; }
+ l2tp_session_inc_refcount(session); + rc = l2tp_session_register(session, tunnel); + if (rc < 0) { + kfree(session); + goto out; + } + dev = alloc_netdev(sizeof(*priv), name, l2tp_eth_dev_setup); if (!dev) { rc = -ENOMEM; @@ -227,6 +234,7 @@ static int l2tp_eth_create(struct net *n __module_get(THIS_MODULE); /* Must be done after register_netdev() */ strlcpy(session->ifname, dev->name, IFNAMSIZ); + l2tp_session_dec_refcount(session);
dev_hold(dev);
@@ -237,6 +245,7 @@ out_del_dev: spriv->dev = NULL; out_del_session: l2tp_session_delete(session); + l2tp_session_dec_refcount(session); out: return rc; } --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -734,6 +734,14 @@ static int pppol2tp_connect(struct socke error = PTR_ERR(session); goto end; } + + l2tp_session_inc_refcount(session); + error = l2tp_session_register(session, tunnel); + if (error < 0) { + kfree(session); + goto end; + } + drop_refcnt = true; }
/* Associate session with its PPPoL2TP socket */ @@ -821,7 +829,7 @@ static int pppol2tp_session_create(struc /* Error if tunnel socket is not prepped */ if (!tunnel->sock) { error = -ENOENT; - goto out; + goto err; }
/* Default MTU values. */ @@ -836,18 +844,21 @@ static int pppol2tp_session_create(struc peer_session_id, cfg); if (IS_ERR(session)) { error = PTR_ERR(session); - goto out; + goto err; }
ps = l2tp_session_priv(session); ps->tunnel_sock = tunnel->sock;
- PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO, - "%s: created\n", session->name); - - error = 0; - -out: + error = l2tp_session_register(session, tunnel); + if (error < 0) + goto err_sess; + + return 0; + +err_sess: + kfree(session); +err: return error; }