Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 523e6d7f authored by Guillaume Nault's avatar Guillaume Nault Committed by Greg Kroah-Hartman
Browse files

l2tp: hold tunnel while looking up sessions in l2tp_netlink 



commit 54652eb12c1b72e9602d09cb2821d5760939190f upstream.

l2tp_tunnel_find() doesn't take a reference on the returned tunnel.
Therefore, it's unsafe to use it because the returned tunnel can go
away on us anytime.

Fix this by defining l2tp_tunnel_get(), which works like
l2tp_tunnel_find(), but takes a reference on the returned tunnel.
Caller then has to drop this reference using l2tp_tunnel_dec_refcount().

As l2tp_tunnel_dec_refcount() needs to be moved to l2tp_core.h, let's
simplify the patch and not move the L2TP_REFCNT_DEBUG part. This code
has been broken (not even compiling) in May 2012 by
commit a4ca44fa ("net: l2tp: Standardize logging styles")
and fixed more than two years later by
commit 29abe2fd ("l2tp: fix missing line continuation"). So it
doesn't appear to be used by anyone.

Same thing for l2tp_tunnel_free(); instead of moving it to l2tp_core.h,
let's just simplify things and call kfree_rcu() directly in
l2tp_tunnel_dec_refcount(). Extra assertions and debugging code
provided by l2tp_tunnel_free() didn't help catching any of the
reference counting and socket handling issues found while working on
this series.

Backporting Notes

l2tp_core.c: This patch deletes some code / moves some code to
l2tp_core.h and follows the patch (not including in this series) that
switched from atomic to refcount_t. Moved code changed back to atomic.

Fixes: 309795f4 ("l2tp: Add netlink control API for L2TP")
Signed-off-by: default avatarGuillaume Nault <g.nault@alphalink.fr>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarGiuliano Procida <gprocida@google.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 11a08372

net/l2tp/l2tp_core.c

+21 −45
Original line number Original line Diff line number Diff line
@@ -112,7 +112,6 @@ struct l2tp_net { 
	spinlock_t l2tp_session_hlist_lock;

	spinlock_t l2tp_session_hlist_lock;

};

};





static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel);





static inline struct l2tp_tunnel *l2tp_tunnel(struct sock *sk)

static inline struct l2tp_tunnel *l2tp_tunnel(struct sock *sk)

{

{
@@ -126,39 +125,6 @@ static inline struct l2tp_net *l2tp_pernet(const struct net *net) 
	return net_generic(net, l2tp_net_id);

	return net_generic(net, l2tp_net_id);

}

}





/* Tunnel reference counts. Incremented per session that is added to

 * the tunnel.

 */

static inline void l2tp_tunnel_inc_refcount_1(struct l2tp_tunnel *tunnel)

{

	atomic_inc(&tunnel->ref_count);

}



static inline void l2tp_tunnel_dec_refcount_1(struct l2tp_tunnel *tunnel)

{

	if (atomic_dec_and_test(&tunnel->ref_count))

		l2tp_tunnel_free(tunnel);

}

#ifdef L2TP_REFCNT_DEBUG

#define l2tp_tunnel_inc_refcount(_t)					\

do {									\

	pr_debug("l2tp_tunnel_inc_refcount: %s:%d %s: cnt=%d\n",	\

		 __func__, __LINE__, (_t)->name,			\

		 atomic_read(&_t->ref_count));				\

	l2tp_tunnel_inc_refcount_1(_t);					\

} while (0)

#define l2tp_tunnel_dec_refcount(_t)					\

do {									\

	pr_debug("l2tp_tunnel_dec_refcount: %s:%d %s: cnt=%d\n",	\

		 __func__, __LINE__, (_t)->name,			\

		 atomic_read(&_t->ref_count));				\

	l2tp_tunnel_dec_refcount_1(_t);					\

} while (0)

#else

#define l2tp_tunnel_inc_refcount(t) l2tp_tunnel_inc_refcount_1(t)

#define l2tp_tunnel_dec_refcount(t) l2tp_tunnel_dec_refcount_1(t)

#endif



/* Session hash global list for L2TPv3.

/* Session hash global list for L2TPv3.

 * The session_id SHOULD be random according to RFC3931, but several

 * The session_id SHOULD be random according to RFC3931, but several

 * L2TP implementations use incrementing session_ids.  So we do a real

 * L2TP implementations use incrementing session_ids.  So we do a real
@@ -228,6 +194,27 @@ l2tp_session_id_hash(struct l2tp_tunnel *tunnel, u32 session_id) 
	return &tunnel->session_hlist[hash_32(session_id, L2TP_HASH_BITS)];

	return &tunnel->session_hlist[hash_32(session_id, L2TP_HASH_BITS)];

}

}





/* Lookup a tunnel. A new reference is held on the returned tunnel. */

struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id)

{

	const struct l2tp_net *pn = l2tp_pernet(net);

	struct l2tp_tunnel *tunnel;



	rcu_read_lock_bh();

	list_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) {

		if (tunnel->tunnel_id == tunnel_id) {

			l2tp_tunnel_inc_refcount(tunnel);

			rcu_read_unlock_bh();



			return tunnel;

		}

	}

	rcu_read_unlock_bh();



	return NULL;

}

EXPORT_SYMBOL_GPL(l2tp_tunnel_get);



/* Lookup a session. A new reference is held on the returned session.

/* Lookup a session. A new reference is held on the returned session.

 * Optionally calls session->ref() too if do_ref is true.

 * Optionally calls session->ref() too if do_ref is true.

 */

 */
@@ -1346,17 +1333,6 @@ static void l2tp_udp_encap_destroy(struct sock *sk) 
	}

	}

}

}





/* Really kill the tunnel.

 * Come here only when all sessions have been cleared from the tunnel.

 */

static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel)

{

	BUG_ON(atomic_read(&tunnel->ref_count) != 0);

	BUG_ON(tunnel->sock != NULL);

	l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: free...\n", tunnel->name);

	kfree_rcu(tunnel, rcu);

}



/* Workqueue tunnel deletion function */

/* Workqueue tunnel deletion function */

static void l2tp_tunnel_del_work(struct work_struct *work)

static void l2tp_tunnel_del_work(struct work_struct *work)

{

{

net/l2tp/l2tp_core.h

+13 −0
Original line number Original line Diff line number Diff line
@@ -231,6 +231,8 @@ static inline struct l2tp_tunnel *l2tp_sock_to_tunnel(struct sock *sk) 
	return tunnel;

	return tunnel;

}

}





struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);



struct l2tp_session *l2tp_session_get(const struct net *net,

struct l2tp_session *l2tp_session_get(const struct net *net,

				      struct l2tp_tunnel *tunnel,

				      struct l2tp_tunnel *tunnel,

				      u32 session_id, bool do_ref);

				      u32 session_id, bool do_ref);
@@ -269,6 +271,17 @@ int l2tp_nl_register_ops(enum l2tp_pwtype pw_type, 
void l2tp_nl_unregister_ops(enum l2tp_pwtype pw_type);

void l2tp_nl_unregister_ops(enum l2tp_pwtype pw_type);

int l2tp_ioctl(struct sock *sk, int cmd, unsigned long arg);

int l2tp_ioctl(struct sock *sk, int cmd, unsigned long arg);





static inline void l2tp_tunnel_inc_refcount(struct l2tp_tunnel *tunnel)

{

	atomic_inc(&tunnel->ref_count);

}



static inline void l2tp_tunnel_dec_refcount(struct l2tp_tunnel *tunnel)

{

	if (atomic_dec_and_test(&tunnel->ref_count))

		kfree_rcu(tunnel, rcu);

}



/* Session reference counts. Incremented when code obtains a reference

/* Session reference counts. Incremented when code obtains a reference

 * to a session.

 * to a session.

 */

 */

net/l2tp/l2tp_netlink.c

+4 −2
Original line number Original line Diff line number Diff line
@@ -72,10 +72,12 @@ static struct l2tp_session *l2tp_nl_session_get(struct genl_info *info, 
		   (info->attrs[L2TP_ATTR_CONN_ID])) {

		   (info->attrs[L2TP_ATTR_CONN_ID])) {

		tunnel_id = nla_get_u32(info->attrs[L2TP_ATTR_CONN_ID]);

		tunnel_id = nla_get_u32(info->attrs[L2TP_ATTR_CONN_ID]);

		session_id = nla_get_u32(info->attrs[L2TP_ATTR_SESSION_ID]);

		session_id = nla_get_u32(info->attrs[L2TP_ATTR_SESSION_ID]);

		tunnel = l2tp_tunnel_find(net, tunnel_id);

		tunnel = l2tp_tunnel_get(net, tunnel_id);

		if (tunnel)

		if (tunnel) {

			session = l2tp_session_get(net, tunnel, session_id,

			session = l2tp_session_get(net, tunnel, session_id,

						   do_ref);

						   do_ref);

			l2tp_tunnel_dec_refcount(tunnel);

		}

	}

	}





	return session;

	return session;