From: Asbjørn Sloth Tønnesen asbjorn@asbjorn.st

commit 41c43fbee68f4f9a2a9675d83bca91c77862d7f0 upstream.

Move the L2TP_MSG_* definitions to UAPI, as it is part of the netlink API.

Signed-off-by: Asbjoern Sloth Toennesen asbjorn@asbjorn.st Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- include/uapi/linux/l2tp.h | 17 ++++++++++++++++- net/l2tp/l2tp_core.h | 10 ---------- 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/include/uapi/linux/l2tp.h b/include/uapi/linux/l2tp.h index 4bd27d0270a2..bb2d62037037 100644 --- a/include/uapi/linux/l2tp.h +++ b/include/uapi/linux/l2tp.h @@ -108,7 +108,7 @@ enum { L2TP_ATTR_VLAN_ID, /* u16 */ L2TP_ATTR_COOKIE, /* 0, 4 or 8 bytes */ L2TP_ATTR_PEER_COOKIE, /* 0, 4 or 8 bytes */ - L2TP_ATTR_DEBUG, /* u32 */ + L2TP_ATTR_DEBUG, /* u32, enum l2tp_debug_flags */ L2TP_ATTR_RECV_SEQ, /* u8 */ L2TP_ATTR_SEND_SEQ, /* u8 */ L2TP_ATTR_LNS_MODE, /* u8 */ @@ -175,6 +175,21 @@ enum l2tp_seqmode { L2TP_SEQ_ALL = 2, };

+/** + * enum l2tp_debug_flags - debug message categories for L2TP tunnels/sessions + * + * @L2TP_MSG_DEBUG: verbose debug (if compiled in) + * @L2TP_MSG_CONTROL: userspace - kernel interface + * @L2TP_MSG_SEQ: sequence numbers + * @L2TP_MSG_DATA: data packets + */ +enum l2tp_debug_flags { + L2TP_MSG_DEBUG = (1 << 0), + L2TP_MSG_CONTROL = (1 << 1), + L2TP_MSG_SEQ = (1 << 2), + L2TP_MSG_DATA = (1 << 3), +}; + /* * NETLINK_GENERIC related info */ diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 7c2037184b6c..092698a8f74b 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -23,16 +23,6 @@ #define L2TP_HASH_BITS_2 8 #define L2TP_HASH_SIZE_2 (1 << L2TP_HASH_BITS_2)

-/* Debug message categories for the DEBUG socket option */ -enum { - L2TP_MSG_DEBUG = (1 << 0), /* verbose debug (if - * compiled in) */ - L2TP_MSG_CONTROL = (1 << 1), /* userspace - kernel - * interface */ - L2TP_MSG_SEQ = (1 << 2), /* sequence numbers */ - L2TP_MSG_DATA = (1 << 3), /* data packets */ -}; - struct sk_buff;

struct l2tp_stats {

From: Asbjørn Sloth Tønnesen asbjorn@asbjorn.st

commit fba40c632c6473fa89660e870a6042c0fe733f8c upstream.

Signed-off-by: Asbjoern Sloth Toennesen asbjorn@asbjorn.st Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- net/l2tp/l2tp_ppp.c | 54 ++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index d919b3e6b548..809606f2d54a 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -231,14 +231,14 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int if (sk->sk_state & PPPOX_BOUND) { struct pppox_sock *po;

- l2tp_dbg(session, PPPOL2TP_MSG_DATA, + l2tp_dbg(session, L2TP_MSG_DATA, "%s: recv %d byte data frame, passing to ppp\n", session->name, data_len);

po = pppox_sk(sk); ppp_input(&po->chan, skb); } else { - l2tp_dbg(session, PPPOL2TP_MSG_DATA, + l2tp_dbg(session, L2TP_MSG_DATA, "%s: recv %d byte data frame, passing to L2TP socket\n", session->name, data_len);

@@ -251,7 +251,7 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int return;

no_sock: - l2tp_info(session, PPPOL2TP_MSG_DATA, "%s: no socket\n", session->name); + l2tp_info(session, L2TP_MSG_DATA, "%s: no socket\n", session->name); kfree_skb(skb); }

@@ -782,7 +782,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, /* This is how we get the session context from the socket. */ sk->sk_user_data = session; sk->sk_state = PPPOX_CONNECTED; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: created\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: created\n", session->name);

end: @@ -833,7 +833,7 @@ static int pppol2tp_session_create(struct net *net, u32 tunnel_id, u32 session_i ps = l2tp_session_priv(session); ps->tunnel_sock = tunnel->sock;

- l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: created\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: created\n", session->name);

error = 0; @@ -995,7 +995,7 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session, struct l2tp_tunnel *tunnel = session->tunnel; struct pppol2tp_ioc_stats stats;

- l2tp_dbg(session, PPPOL2TP_MSG_CONTROL, + l2tp_dbg(session, L2TP_MSG_CONTROL, "%s: pppol2tp_session_ioctl(cmd=%#x, arg=%#lx)\n", session->name, cmd, arg);

@@ -1018,7 +1018,7 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session, if (copy_to_user((void __user *) arg, &ifr, sizeof(struct ifreq))) break;

- l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get mtu=%d\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: get mtu=%d\n", session->name, session->mtu); err = 0; break; @@ -1034,7 +1034,7 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,

session->mtu = ifr.ifr_mtu;

- l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: set mtu=%d\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: set mtu=%d\n", session->name, session->mtu); err = 0; break; @@ -1048,7 +1048,7 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session, if (put_user(session->mru, (int __user *) arg)) break;

- l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get mru=%d\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: get mru=%d\n", session->name, session->mru); err = 0; break; @@ -1063,7 +1063,7 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session, break;

session->mru = val; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: set mru=%d\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: set mru=%d\n", session->name, session->mru); err = 0; break; @@ -1073,7 +1073,7 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session, if (put_user(ps->flags, (int __user *) arg)) break;

- l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get flags=%d\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: get flags=%d\n", session->name, ps->flags); err = 0; break; @@ -1083,7 +1083,7 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session, if (get_user(val, (int __user *) arg)) break; ps->flags = val; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: set flags=%d\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: set flags=%d\n", session->name, ps->flags); err = 0; break; @@ -1100,7 +1100,7 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session, if (copy_to_user((void __user *) arg, &stats, sizeof(stats))) break; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get L2TP stats\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: get L2TP stats\n", session->name); err = 0; break; @@ -1128,7 +1128,7 @@ static int pppol2tp_tunnel_ioctl(struct l2tp_tunnel *tunnel, struct sock *sk; struct pppol2tp_ioc_stats stats;

- l2tp_dbg(tunnel, PPPOL2TP_MSG_CONTROL, + l2tp_dbg(tunnel, L2TP_MSG_CONTROL, "%s: pppol2tp_tunnel_ioctl(cmd=%#x, arg=%#lx)\n", tunnel->name, cmd, arg);

@@ -1171,7 +1171,7 @@ static int pppol2tp_tunnel_ioctl(struct l2tp_tunnel *tunnel, err = -EFAULT; break; } - l2tp_info(tunnel, PPPOL2TP_MSG_CONTROL, "%s: get L2TP stats\n", + l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: get L2TP stats\n", tunnel->name); err = 0; break; @@ -1261,7 +1261,7 @@ static int pppol2tp_tunnel_setsockopt(struct sock *sk, switch (optname) { case PPPOL2TP_SO_DEBUG: tunnel->debug = val; - l2tp_info(tunnel, PPPOL2TP_MSG_CONTROL, "%s: set debug=%x\n", + l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: set debug=%x\n", tunnel->name, tunnel->debug); break;

@@ -1289,7 +1289,7 @@ static int pppol2tp_session_setsockopt(struct sock *sk, break; } session->recv_seq = val ? -1 : 0; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, + l2tp_info(session, L2TP_MSG_CONTROL, "%s: set recv_seq=%d\n", session->name, session->recv_seq); break; @@ -1307,7 +1307,7 @@ static int pppol2tp_session_setsockopt(struct sock *sk, PPPOL2TP_L2TP_HDR_SIZE_NOSEQ; } l2tp_session_set_header_len(session, session->tunnel->version); - l2tp_info(session, PPPOL2TP_MSG_CONTROL, + l2tp_info(session, L2TP_MSG_CONTROL, "%s: set send_seq=%d\n", session->name, session->send_seq); break; @@ -1318,20 +1318,20 @@ static int pppol2tp_session_setsockopt(struct sock *sk, break; } session->lns_mode = val ? -1 : 0; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, + l2tp_info(session, L2TP_MSG_CONTROL, "%s: set lns_mode=%d\n", session->name, session->lns_mode); break;

case PPPOL2TP_SO_DEBUG: session->debug = val; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: set debug=%x\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: set debug=%x\n", session->name, session->debug); break;

case PPPOL2TP_SO_REORDERTO: session->reorder_timeout = msecs_to_jiffies(val); - l2tp_info(session, PPPOL2TP_MSG_CONTROL, + l2tp_info(session, L2TP_MSG_CONTROL, "%s: set reorder_timeout=%d\n", session->name, session->reorder_timeout); break; @@ -1412,7 +1412,7 @@ static int pppol2tp_tunnel_getsockopt(struct sock *sk, switch (optname) { case PPPOL2TP_SO_DEBUG: *val = tunnel->debug; - l2tp_info(tunnel, PPPOL2TP_MSG_CONTROL, "%s: get debug=%x\n", + l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: get debug=%x\n", tunnel->name, tunnel->debug); break;

@@ -1435,31 +1435,31 @@ static int pppol2tp_session_getsockopt(struct sock *sk, switch (optname) { case PPPOL2TP_SO_RECVSEQ: *val = session->recv_seq; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, + l2tp_info(session, L2TP_MSG_CONTROL, "%s: get recv_seq=%d\n", session->name, *val); break;

case PPPOL2TP_SO_SENDSEQ: *val = session->send_seq; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, + l2tp_info(session, L2TP_MSG_CONTROL, "%s: get send_seq=%d\n", session->name, *val); break;

case PPPOL2TP_SO_LNSMODE: *val = session->lns_mode; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, + l2tp_info(session, L2TP_MSG_CONTROL, "%s: get lns_mode=%d\n", session->name, *val); break;

case PPPOL2TP_SO_DEBUG: *val = session->debug; - l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get debug=%d\n", + l2tp_info(session, L2TP_MSG_CONTROL, "%s: get debug=%d\n", session->name, *val); break;

case PPPOL2TP_SO_REORDERTO: *val = (int) jiffies_to_msecs(session->reorder_timeout); - l2tp_info(session, PPPOL2TP_MSG_CONTROL, + l2tp_info(session, L2TP_MSG_CONTROL, "%s: get reorder_timeout=%d\n", session->name, *val); break;

From: "R. Parameswaran" parameswaran.r7@gmail.com

commit b784e7ebfce8cfb16c6f95e14e8532d0768ab7ff upstream.

Existing L2TP kernel code does not derive the optimal MTU for Ethernet pseudowires and instead leaves this to a userspace L2TP daemon or operator. If an MTU is not specified, the existing kernel code chooses an MTU that does not take account of all tunnel header overheads, which can lead to unwanted IP fragmentation. When L2TP is used without a control plane (userspace daemon), we would prefer that the kernel does a better job of choosing a default pseudowire MTU, taking account of all tunnel header overheads, including IP header options, if any. This patch addresses this.

Change-set here uses the new kernel function, kernel_sock_ip_overhead(), to factor the outer IP overhead on the L2TP tunnel socket (including IP Options, if any) when calculating the default MTU for an Ethernet pseudowire, along with consideration of the inner Ethernet header.

Signed-off-by: R. Parameswaran rparames@brocade.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- net/l2tp/l2tp_eth.c | 55 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 4 deletions(-)

diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c index eecc64e138de..f0efbf1e9a49 100644 --- a/net/l2tp/l2tp_eth.c +++ b/net/l2tp/l2tp_eth.c @@ -30,6 +30,9 @@ #include <net/xfrm.h> #include <net/net_namespace.h> #include <net/netns/generic.h> +#include <linux/ip.h> +#include <linux/ipv6.h> +#include <linux/udp.h>

#include "l2tp_core.h"

@@ -206,6 +209,53 @@ static void l2tp_eth_show(struct seq_file *m, void *arg) } #endif

+static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel, + struct l2tp_session *session, + struct net_device *dev) +{ + unsigned int overhead = 0; + struct dst_entry *dst; + u32 l3_overhead = 0; + + /* if the encap is UDP, account for UDP header size */ + if (tunnel->encap == L2TP_ENCAPTYPE_UDP) { + overhead += sizeof(struct udphdr); + dev->needed_headroom += sizeof(struct udphdr); + } + if (session->mtu != 0) { + dev->mtu = session->mtu; + dev->needed_headroom += session->hdr_len; + return; + } + l3_overhead = kernel_sock_ip_overhead(tunnel->sock); + if (l3_overhead == 0) { + /* L3 Overhead couldn't be identified, this could be + * because tunnel->sock was NULL or the socket's + * address family was not IPv4 or IPv6, + * dev mtu stays at 1500. + */ + return; + } + /* Adjust MTU, factor overhead - underlay L3, overlay L2 hdr + * UDP overhead, if any, was already factored in above. + */ + overhead += session->hdr_len + ETH_HLEN + l3_overhead; + + /* If PMTU discovery was enabled, use discovered MTU on L2TP device */ + dst = sk_dst_get(tunnel->sock); + if (dst) { + /* dst_mtu will use PMTU if found, else fallback to intf MTU */ + u32 pmtu = dst_mtu(dst); + + if (pmtu != 0) + dev->mtu = pmtu; + dst_release(dst); + } + session->mtu = dev->mtu - overhead; + dev->mtu = session->mtu; + dev->needed_headroom += session->hdr_len; +} + static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg) { struct net_device *dev; @@ -249,10 +299,7 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p }

dev_net_set(dev, net); - if (session->mtu == 0) - session->mtu = dev->mtu - session->hdr_len; - dev->mtu = session->mtu; - dev->needed_headroom += session->hdr_len; + l2tp_eth_adjust_mtu(tunnel, session, dev);

priv = netdev_priv(dev); priv->dev = dev;

From: Guillaume Nault g.nault@alphalink.fr

commit 55a3ce3b9d98f752df9e2cfb1cba7e715522428a upstream.

This function isn't used anymore.

Signed-off-by: Guillaume Nault g.nault@alphalink.fr Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- net/l2tp/l2tp_core.c | 51 +------------------------------------------- net/l2tp/l2tp_core.h | 3 --- 2 files changed, 1 insertion(+), 53 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 7c3da29fad8e..440065462a69 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -216,27 +216,6 @@ static void l2tp_tunnel_sock_put(struct sock *sk) sock_put(sk); }

-/* Lookup a session by id in the global session list - */ -static struct l2tp_session *l2tp_session_find_2(struct net *net, u32 session_id) -{ - struct l2tp_net *pn = l2tp_pernet(net); - struct hlist_head *session_list = - l2tp_session_id_hash_2(pn, session_id); - struct l2tp_session *session; - - rcu_read_lock_bh(); - hlist_for_each_entry_rcu(session, session_list, global_hlist) { - if (session->session_id == session_id) { - rcu_read_unlock_bh(); - return session; - } - } - rcu_read_unlock_bh(); - - return NULL; -} - /* Session hash list. * The session_id SHOULD be random according to RFC2661, but several * L2TP implementations (Cisco and Microsoft) use incrementing @@ -249,35 +228,7 @@ l2tp_session_id_hash(struct l2tp_tunnel *tunnel, u32 session_id) return &tunnel->session_hlist[hash_32(session_id, L2TP_HASH_BITS)]; }

-/* Lookup a session by id - */ -struct l2tp_session *l2tp_session_find(struct net *net, struct l2tp_tunnel *tunnel, u32 session_id) -{ - struct hlist_head *session_list; - struct l2tp_session *session; - - /* In L2TPv3, session_ids are unique over all tunnels and we - * sometimes need to look them up before we know the - * tunnel. - */ - if (tunnel == NULL) - return l2tp_session_find_2(net, session_id); - - session_list = l2tp_session_id_hash(tunnel, session_id); - read_lock_bh(&tunnel->hlist_lock); - hlist_for_each_entry(session, session_list, hlist) { - if (session->session_id == session_id) { - read_unlock_bh(&tunnel->hlist_lock); - return session; - } - } - read_unlock_bh(&tunnel->hlist_lock); - - return NULL; -} -EXPORT_SYMBOL_GPL(l2tp_session_find); - -/* Like l2tp_session_find() but takes a reference 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. */ struct l2tp_session *l2tp_session_get(struct net *net, diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 092698a8f74b..e38db6a807f5 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -234,9 +234,6 @@ static inline struct l2tp_tunnel *l2tp_sock_to_tunnel(struct sock *sk) struct l2tp_session *l2tp_session_get(struct net *net, struct l2tp_tunnel *tunnel, u32 session_id, bool do_ref); -struct l2tp_session *l2tp_session_find(struct net *net, - struct l2tp_tunnel *tunnel, - u32 session_id); struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth, bool do_ref); struct l2tp_session *l2tp_session_get_by_ifname(struct net *net, char *ifname,

From: Guillaume Nault g.nault@alphalink.fr

commit 2f858b928bf5a8174911aaec76b8b72a9ca0533d upstream.

l2tp_tunnel_find() and l2tp_tunnel_find_nth() don't modify "net".

Signed-off-by: Guillaume Nault g.nault@alphalink.fr Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- net/l2tp/l2tp_core.c | 4 ++-- net/l2tp/l2tp_core.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index be8d7b2b8790..a9d4d42e2ef6 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -378,7 +378,7 @@ static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel,

/* Lookup a tunnel by id */ -struct l2tp_tunnel *l2tp_tunnel_find(struct net *net, u32 tunnel_id) +struct l2tp_tunnel *l2tp_tunnel_find(const struct net *net, u32 tunnel_id) { struct l2tp_tunnel *tunnel; struct l2tp_net *pn = l2tp_pernet(net); @@ -396,7 +396,7 @@ struct l2tp_tunnel *l2tp_tunnel_find(struct net *net, u32 tunnel_id) } EXPORT_SYMBOL_GPL(l2tp_tunnel_find);

-struct l2tp_tunnel *l2tp_tunnel_find_nth(struct net *net, int nth) +struct l2tp_tunnel *l2tp_tunnel_find_nth(const struct net *net, int nth) { struct l2tp_net *pn = l2tp_pernet(net); struct l2tp_tunnel *tunnel; diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 3a3d96df6071..2f9a09097e30 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -239,8 +239,8 @@ struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth, struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, const char *ifname, bool do_ref); -struct l2tp_tunnel *l2tp_tunnel_find(struct net *net, u32 tunnel_id); -struct l2tp_tunnel *l2tp_tunnel_find_nth(struct net *net, int nth); +struct l2tp_tunnel *l2tp_tunnel_find(const struct net *net, u32 tunnel_id); +struct l2tp_tunnel *l2tp_tunnel_find_nth(const struct net *net, int nth);

int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg,

From: Guillaume Nault g.nault@alphalink.fr

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 a4ca44fa578c ("net: l2tp: Standardize logging styles") and fixed more than two years later by commit 29abe2fda54f ("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: 309795f4bec2 ("l2tp: Add netlink control API for L2TP") Signed-off-by: Guillaume Nault g.nault@alphalink.fr Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- net/l2tp/l2tp_core.c | 66 +++++++++++++---------------------------- net/l2tp/l2tp_core.h | 13 ++++++++ net/l2tp/l2tp_netlink.c | 6 ++-- 3 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 7f72957405b8..5d1eb253a0b1 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -112,7 +112,6 @@ struct l2tp_net { 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) { @@ -126,39 +125,6 @@ static inline struct l2tp_net *l2tp_pernet(const struct net *net) 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. * The session_id SHOULD be random according to RFC3931, but several * 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)]; }

+/* 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. * 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 */ static void l2tp_tunnel_del_work(struct work_struct *work) { diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 2f9a09097e30..f747deaf6e09 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -231,6 +231,8 @@ static inline struct l2tp_tunnel *l2tp_sock_to_tunnel(struct sock *sk) 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_tunnel *tunnel, 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); 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 * to a session. */ diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c index 36651b60d776..0a27f7e976f3 100644 --- a/net/l2tp/l2tp_netlink.c +++ b/net/l2tp/l2tp_netlink.c @@ -72,10 +72,12 @@ static struct l2tp_session *l2tp_nl_session_get(struct genl_info *info, (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]); - tunnel = l2tp_tunnel_find(net, tunnel_id); - if (tunnel) + tunnel = l2tp_tunnel_get(net, tunnel_id); + if (tunnel) { session = l2tp_session_get(net, tunnel, session_id, do_ref); + l2tp_tunnel_dec_refcount(tunnel); + } }

return session;

From: Guillaume Nault g.nault@alphalink.fr

commit 8c0e421525c9eb50d68e8f633f703ca31680b746 upstream.

We need to make sure the tunnel is not going to be destroyed by l2tp_tunnel_destruct() concurrently.

Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP") Signed-off-by: Guillaume Nault g.nault@alphalink.fr Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- net/l2tp/l2tp_netlink.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c index 93148a215e7c..e454f23f31fb 100644 --- a/net/l2tp/l2tp_netlink.c +++ b/net/l2tp/l2tp_netlink.c @@ -310,8 +310,8 @@ static int l2tp_nl_cmd_tunnel_modify(struct sk_buff *skb, struct genl_info *info } tunnel_id = nla_get_u32(info->attrs[L2TP_ATTR_CONN_ID]);

- tunnel = l2tp_tunnel_find(net, tunnel_id); - if (tunnel == NULL) { + tunnel = l2tp_tunnel_get(net, tunnel_id); + if (!tunnel) { ret = -ENODEV; goto out; } @@ -322,6 +322,8 @@ static int l2tp_nl_cmd_tunnel_modify(struct sk_buff *skb, struct genl_info *info ret = l2tp_tunnel_notify(&l2tp_nl_family, info, tunnel, L2TP_CMD_TUNNEL_MODIFY);

+ l2tp_tunnel_dec_refcount(tunnel); + out: return ret; }

From: Guillaume Nault g.nault@alphalink.fr

commit e702c1204eb57788ef189c839c8c779368267d70 upstream.

Use l2tp_tunnel_get() to retrieve tunnel, so that it can't go away on us. Otherwise l2tp_tunnel_destruct() might release the last reference count concurrently, thus freeing the tunnel while we're using it.

Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP") Signed-off-by: Guillaume Nault g.nault@alphalink.fr Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- net/l2tp/l2tp_netlink.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c index 8f39086de144..5ea5d3ffa309 100644 --- a/net/l2tp/l2tp_netlink.c +++ b/net/l2tp/l2tp_netlink.c @@ -510,8 +510,9 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf ret = -EINVAL; goto out; } + tunnel_id = nla_get_u32(info->attrs[L2TP_ATTR_CONN_ID]); - tunnel = l2tp_tunnel_find(net, tunnel_id); + tunnel = l2tp_tunnel_get(net, tunnel_id); if (!tunnel) { ret = -ENODEV; goto out; @@ -519,24 +520,24 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf

if (!info->attrs[L2TP_ATTR_SESSION_ID]) { ret = -EINVAL; - goto out; + goto out_tunnel; } session_id = nla_get_u32(info->attrs[L2TP_ATTR_SESSION_ID]);

if (!info->attrs[L2TP_ATTR_PEER_SESSION_ID]) { ret = -EINVAL; - goto out; + goto out_tunnel; } peer_session_id = nla_get_u32(info->attrs[L2TP_ATTR_PEER_SESSION_ID]);

if (!info->attrs[L2TP_ATTR_PW_TYPE]) { ret = -EINVAL; - goto out; + goto out_tunnel; } cfg.pw_type = nla_get_u16(info->attrs[L2TP_ATTR_PW_TYPE]); if (cfg.pw_type >= __L2TP_PWTYPE_MAX) { ret = -EINVAL; - goto out; + goto out_tunnel; }

if (tunnel->version > 2) { @@ -555,7 +556,7 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf u16 len = nla_len(info->attrs[L2TP_ATTR_COOKIE]); if (len > 8) { ret = -EINVAL; - goto out; + goto out_tunnel; } cfg.cookie_len = len; memcpy(&cfg.cookie[0], nla_data(info->attrs[L2TP_ATTR_COOKIE]), len); @@ -564,7 +565,7 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf u16 len = nla_len(info->attrs[L2TP_ATTR_PEER_COOKIE]); if (len > 8) { ret = -EINVAL; - goto out; + goto out_tunnel; } cfg.peer_cookie_len = len; memcpy(&cfg.peer_cookie[0], nla_data(info->attrs[L2TP_ATTR_PEER_COOKIE]), len); @@ -607,7 +608,7 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf if ((l2tp_nl_cmd_ops[cfg.pw_type] == NULL) || (l2tp_nl_cmd_ops[cfg.pw_type]->session_create == NULL)) { ret = -EPROTONOSUPPORT; - goto out; + goto out_tunnel; }

/* Check that pseudowire-specific params are present */ @@ -617,7 +618,7 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf case L2TP_PWTYPE_ETH_VLAN: if (!info->attrs[L2TP_ATTR_VLAN_ID]) { ret = -EINVAL; - goto out; + goto out_tunnel; } break; case L2TP_PWTYPE_ETH: @@ -645,6 +646,8 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf } }

+out_tunnel: + l2tp_tunnel_dec_refcount(tunnel); out: return ret; }

From: Guillaume Nault g.nault@alphalink.fr

commit f026bc29a8e093edfbb2a77700454b285c97e8ad upstream.

Using l2tp_tunnel_find() in pppol2tp_session_create() and l2tp_eth_create() is racy, because no reference is held on the returned session. These functions are only used to implement the ->session_create callback which is run by l2tp_nl_cmd_session_create(). Therefore searching for the parent tunnel isn't necessary because l2tp_nl_cmd_session_create() already has a pointer to it and holds a reference.

This patch modifies ->session_create()'s prototype to directly pass the the parent tunnel as parameter, thus avoiding searching for it in pppol2tp_session_create() and l2tp_eth_create().

Since we have to touch the ->session_create() call in l2tp_nl_cmd_session_create(), let's also remove the useless conditional: we know that ->session_create isn't NULL at this point because it's already been checked earlier in this same function.

Finally, one might be tempted to think that the removed l2tp_tunnel_find() calls were harmless because they would return the same tunnel as the one held by l2tp_nl_cmd_session_create() anyway. But that tunnel might be removed and a new one created with same tunnel Id before the l2tp_tunnel_find() call. In this case l2tp_tunnel_find() would return the new tunnel which wouldn't be protected by the reference held by l2tp_nl_cmd_session_create().

Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP") Fixes: d9e31d17ceba ("l2tp: Add L2TP ethernet pseudowire support") Signed-off-by: Guillaume Nault g.nault@alphalink.fr Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- net/l2tp/l2tp_core.h | 4 +++- net/l2tp/l2tp_eth.c | 11 +++-------- net/l2tp/l2tp_netlink.c | 8 ++++---- net/l2tp/l2tp_ppp.c | 19 +++++++------------ 4 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 39a952962593..fac92fda574d 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -201,7 +201,9 @@ struct l2tp_tunnel { };

struct l2tp_nl_cmd_ops { - int (*session_create)(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg); + int (*session_create)(struct net *net, struct l2tp_tunnel *tunnel, + u32 session_id, u32 peer_session_id, + struct l2tp_session_cfg *cfg); int (*session_delete)(struct l2tp_session *session); };

diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c index f0efbf1e9a49..4c122494f022 100644 --- a/net/l2tp/l2tp_eth.c +++ b/net/l2tp/l2tp_eth.c @@ -256,23 +256,18 @@ static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel, dev->needed_headroom += session->hdr_len; }

-static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg) +static int l2tp_eth_create(struct net *net, struct l2tp_tunnel *tunnel, + u32 session_id, u32 peer_session_id, + struct l2tp_session_cfg *cfg) { struct net_device *dev; char name[IFNAMSIZ]; - struct l2tp_tunnel *tunnel; struct l2tp_session *session; struct l2tp_eth *priv; struct l2tp_eth_sess *spriv; int rc; struct l2tp_eth_net *pn;

- tunnel = l2tp_tunnel_find(net, tunnel_id); - if (!tunnel) { - rc = -ENODEV; - goto out; - } - if (cfg->ifname) { dev = dev_get_by_name(net, cfg->ifname); if (dev) { diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c index 5ea5d3ffa309..47d7bdff8be8 100644 --- a/net/l2tp/l2tp_netlink.c +++ b/net/l2tp/l2tp_netlink.c @@ -632,10 +632,10 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf break; }

- ret = -EPROTONOSUPPORT; - if (l2tp_nl_cmd_ops[cfg.pw_type]->session_create) - ret = (*l2tp_nl_cmd_ops[cfg.pw_type]->session_create)(net, tunnel_id, - session_id, peer_session_id, &cfg); + ret = l2tp_nl_cmd_ops[cfg.pw_type]->session_create(net, tunnel, + session_id, + peer_session_id, + &cfg);

if (ret >= 0) { session = l2tp_session_get(net, tunnel, session_id, false); diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 809606f2d54a..c8f877bfb00f 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -795,25 +795,20 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,

#ifdef CONFIG_L2TP_V3

-/* Called when creating sessions via the netlink interface. - */ -static int pppol2tp_session_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg) +/* Called when creating sessions via the netlink interface. */ +static int pppol2tp_session_create(struct net *net, struct l2tp_tunnel *tunnel, + u32 session_id, u32 peer_session_id, + struct l2tp_session_cfg *cfg) { int error; - struct l2tp_tunnel *tunnel; struct l2tp_session *session; struct pppol2tp_session *ps;

- tunnel = l2tp_tunnel_find(net, tunnel_id); - - /* Error if we can't find the tunnel */ - error = -ENOENT; - if (tunnel == NULL) - goto out; - /* Error if tunnel socket is not prepped */ - if (tunnel->sock == NULL) + if (!tunnel->sock) { + error = -ENOENT; goto out; + }

/* Default MTU values. */ if (cfg->mtu == 0)

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 Signed-off-by: Giuliano Procida gprocida@google.com --- 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(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 3a7031426b46..36c7f616294a 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -321,8 +321,8 @@ struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, } 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; @@ -370,6 +370,10 @@ static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel, 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: @@ -379,6 +383,7 @@ static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel,

return err; } +EXPORT_SYMBOL_GPL(l2tp_session_register);

/* Lookup a tunnel by id */ @@ -1788,7 +1793,6 @@ 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 *session; - int err;

session = kzalloc(sizeof(struct l2tp_session) + priv_size, GFP_KERNEL); if (session != NULL) { @@ -1845,17 +1849,6 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn

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; }

diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index fac92fda574d..2b9b6fb67ae9 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -259,6 +259,9 @@ 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); + void __l2tp_session_unhash(struct l2tp_session *session); int l2tp_session_delete(struct l2tp_session *session); void l2tp_session_free(struct l2tp_session *session); diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c index d22a39c0c486..5902d088b44f 100644 --- a/net/l2tp/l2tp_eth.c +++ b/net/l2tp/l2tp_eth.c @@ -267,6 +267,13 @@ static int l2tp_eth_create(struct net *net, struct l2tp_tunnel *tunnel, 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, NET_NAME_UNKNOWN, l2tp_eth_dev_setup); if (!dev) { @@ -298,6 +305,7 @@ static int l2tp_eth_create(struct net *net, struct l2tp_tunnel *tunnel, __module_get(THIS_MODULE); /* Must be done after register_netdev() */ strlcpy(session->ifname, dev->name, IFNAMSIZ); + l2tp_session_dec_refcount(session);

dev_hold(dev);

@@ -308,6 +316,7 @@ static int l2tp_eth_create(struct net *net, struct l2tp_tunnel *tunnel, spriv->dev = NULL; out_del_session: l2tp_session_delete(session); + l2tp_session_dec_refcount(session); out: return rc; } diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index c8f877bfb00f..e617993939d4 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -722,6 +722,14 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, 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 */ @@ -807,7 +815,7 @@ static int pppol2tp_session_create(struct net *net, struct l2tp_tunnel *tunnel, /* Error if tunnel socket is not prepped */ if (!tunnel->sock) { error = -ENOENT; - goto out; + goto err; }

/* Default MTU values. */ @@ -822,18 +830,21 @@ static int pppol2tp_session_create(struct net *net, struct l2tp_tunnel *tunnel, 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;

- l2tp_info(session, L2TP_MSG_CONTROL, "%s: created\n", - session->name); + error = l2tp_session_register(session, tunnel); + if (error < 0) + goto err_sess;

- error = 0; + return 0;

-out: +err_sess: + kfree(session); +err: return error; }

From: Guillaume Nault g.nault@alphalink.fr

commit ee40fb2e1eb5bc0ddd3f2f83c6e39a454ef5a741 upstream.

pppol2tp_session_create() registers sessions that can't have their corresponding socket initialised. This socket has to be created by userspace, then connected to the session by pppol2tp_connect(). Therefore, we need to protect the pppol2tp socket pointer of L2TP sessions, so that it can safely be updated when userspace is connecting or closing the socket. This will eventually allow pppol2tp_connect() to avoid generating transient states while initialising its parts of the session.

To this end, this patch protects the pppol2tp socket pointer using RCU.

The pppol2tp socket pointer is still set in pppol2tp_connect(), but only once we know the function isn't going to fail. It's eventually reset by pppol2tp_release(), which now has to wait for a grace period to elapse before it can drop the last reference on the socket. This ensures that pppol2tp_session_get_sock() can safely grab a reference on the socket, even after ps->sk is reset to NULL but before this operation actually gets visible from pppol2tp_session_get_sock().

The rest is standard RCU conversion: pppol2tp_recv(), which already runs in atomic context, is simply enclosed by rcu_read_lock() and rcu_read_unlock(), while other functions are converted to use pppol2tp_session_get_sock() followed by sock_put(). pppol2tp_session_setsockopt() is a special case. It used to retrieve the pppol2tp socket from the L2TP session, which itself was retrieved from the pppol2tp socket. Therefore we can just avoid dereferencing ps->sk and directly use the original socket pointer instead.

With all users of ps->sk now handling NULL and concurrent updates, the L2TP ->ref() and ->deref() callbacks aren't needed anymore. Therefore, rather than converting pppol2tp_session_sock_hold() and pppol2tp_session_sock_put(), we can just drop them.

Signed-off-by: Guillaume Nault g.nault@alphalink.fr Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Giuliano Procida gprocida@google.com --- net/l2tp/l2tp_ppp.c | 154 +++++++++++++++++++++++++++++--------------- 1 file changed, 101 insertions(+), 53 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index e617993939d4..9eb07c1a993e 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -122,8 +122,11 @@ struct pppol2tp_session { int owner; /* pid that opened the socket */

- struct sock *sock; /* Pointer to the session + struct mutex sk_lock; /* Protects .sk */ + struct sock __rcu *sk; /* Pointer to the session * PPPoX socket */ + struct sock *__sk; /* Copy of .sk, for cleanup */ + struct rcu_head rcu; /* For asynchronous release */ struct sock *tunnel_sock; /* Pointer to the tunnel UDP * socket */ int flags; /* accessed by PPPIOCGFLAGS. @@ -138,6 +141,24 @@ static const struct ppp_channel_ops pppol2tp_chan_ops = {

static const struct proto_ops pppol2tp_ops;

+/* Retrieves the pppol2tp socket associated to a session. + * A reference is held on the returned socket, so this function must be paired + * with sock_put(). + */ +static struct sock *pppol2tp_session_get_sock(struct l2tp_session *session) +{ + struct pppol2tp_session *ps = l2tp_session_priv(session); + struct sock *sk; + + rcu_read_lock(); + sk = rcu_dereference(ps->sk); + if (sk) + sock_hold(sk); + rcu_read_unlock(); + + return sk; +} + /* Helpers to obtain tunnel/session contexts from sockets. */ static inline struct l2tp_session *pppol2tp_sock_to_session(struct sock *sk) @@ -224,7 +245,8 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int /* If the socket is bound, send it in to PPP's input queue. Otherwise * queue it on the session socket. */ - sk = ps->sock; + rcu_read_lock(); + sk = rcu_dereference(ps->sk); if (sk == NULL) goto no_sock;

@@ -247,30 +269,16 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int kfree_skb(skb); } } + rcu_read_unlock();

return;

no_sock: + rcu_read_unlock(); l2tp_info(session, L2TP_MSG_DATA, "%s: no socket\n", session->name); kfree_skb(skb); }

-static void pppol2tp_session_sock_hold(struct l2tp_session *session) -{ - struct pppol2tp_session *ps = l2tp_session_priv(session); - - if (ps->sock) - sock_hold(ps->sock); -} - -static void pppol2tp_session_sock_put(struct l2tp_session *session) -{ - struct pppol2tp_session *ps = l2tp_session_priv(session); - - if (ps->sock) - sock_put(ps->sock); -} - /************************************************************************ * Transmit handling ***********************************************************************/ @@ -431,14 +439,16 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb) */ static void pppol2tp_session_close(struct l2tp_session *session) { - struct pppol2tp_session *ps = l2tp_session_priv(session); - struct sock *sk = ps->sock; - struct socket *sock = sk->sk_socket; + struct sock *sk;

BUG_ON(session->magic != L2TP_SESSION_MAGIC);

- if (sock) - inet_shutdown(sock, SEND_SHUTDOWN); + sk = pppol2tp_session_get_sock(session); + if (sk) { + if (sk->sk_socket) + inet_shutdown(sk->sk_socket, SEND_SHUTDOWN); + sock_put(sk); + }

/* Don't let the session go away before our socket does */ l2tp_session_inc_refcount(session); @@ -461,6 +471,14 @@ static void pppol2tp_session_destruct(struct sock *sk) } }

+static void pppol2tp_put_sk(struct rcu_head *head) +{ + struct pppol2tp_session *ps; + + ps = container_of(head, typeof(*ps), rcu); + sock_put(ps->__sk); +} + /* Called when the PPPoX socket (session) is closed. */ static int pppol2tp_release(struct socket *sock) @@ -486,11 +504,24 @@ static int pppol2tp_release(struct socket *sock)

session = pppol2tp_sock_to_session(sk);

- /* Purge any queued data */ if (session != NULL) { + struct pppol2tp_session *ps; + __l2tp_session_unhash(session); l2tp_session_queue_purge(session); - sock_put(sk); + + ps = l2tp_session_priv(session); + mutex_lock(&ps->sk_lock); + ps->__sk = rcu_dereference_protected(ps->sk, + lockdep_is_held(&ps->sk_lock)); + RCU_INIT_POINTER(ps->sk, NULL); + mutex_unlock(&ps->sk_lock); + call_rcu(&ps->rcu, pppol2tp_put_sk); + + /* Rely on the sock_put() call at the end of the function for + * dropping the reference held by pppol2tp_sock_to_session(). + * The last reference will be dropped by pppol2tp_put_sk(). + */ } release_sock(sk);

@@ -557,12 +588,14 @@ static int pppol2tp_create(struct net *net, struct socket *sock, int kern) static void pppol2tp_show(struct seq_file *m, void *arg) { struct l2tp_session *session = arg; - struct pppol2tp_session *ps = l2tp_session_priv(session); + struct sock *sk; + + sk = pppol2tp_session_get_sock(session); + if (sk) { + struct pppox_sock *po = pppox_sk(sk);

- if (ps) { - struct pppox_sock *po = pppox_sk(ps->sock); - if (po) - seq_printf(m, " interface %s\n", ppp_dev_name(&po->chan)); + seq_printf(m, " interface %s\n", ppp_dev_name(&po->chan)); + sock_put(sk); } } #endif @@ -700,13 +733,17 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, /* Using a pre-existing session is fine as long as it hasn't * been connected yet. */ - if (ps->sock) { + mutex_lock(&ps->sk_lock); + if (rcu_dereference_protected(ps->sk, + lockdep_is_held(&ps->sk_lock))) { + mutex_unlock(&ps->sk_lock); error = -EEXIST; goto end; }

/* consistency checks */ if (ps->tunnel_sock != tunnel->sock) { + mutex_unlock(&ps->sk_lock); error = -EEXIST; goto end; } @@ -723,19 +760,21 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, goto end; }

+ ps = l2tp_session_priv(session); + mutex_init(&ps->sk_lock); l2tp_session_inc_refcount(session); + + mutex_lock(&ps->sk_lock); error = l2tp_session_register(session, tunnel); if (error < 0) { + mutex_unlock(&ps->sk_lock); kfree(session); goto end; } drop_refcnt = true; }

- /* Associate session with its PPPoL2TP socket */ - ps = l2tp_session_priv(session); ps->owner = current->pid; - ps->sock = sk; ps->tunnel_sock = tunnel->sock;

session->recv_skb = pppol2tp_recv; @@ -744,12 +783,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, session->show = pppol2tp_show; #endif

- /* We need to know each time a skb is dropped from the reorder - * queue. - */ - session->ref = pppol2tp_session_sock_hold; - session->deref = pppol2tp_session_sock_put; - /* If PMTU discovery was enabled, use the MTU that was discovered */ dst = sk_dst_get(tunnel->sock); if (dst != NULL) { @@ -783,12 +816,17 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, po->chan.mtu = session->mtu;

error = ppp_register_net_channel(sock_net(sk), &po->chan); - if (error) + if (error) { + mutex_unlock(&ps->sk_lock); goto end; + }

out_no_ppp: /* This is how we get the session context from the socket. */ sk->sk_user_data = session; + rcu_assign_pointer(ps->sk, sk); + mutex_unlock(&ps->sk_lock); + sk->sk_state = PPPOX_CONNECTED; l2tp_info(session, L2TP_MSG_CONTROL, "%s: created\n", session->name); @@ -834,6 +872,7 @@ static int pppol2tp_session_create(struct net *net, struct l2tp_tunnel *tunnel, }

ps = l2tp_session_priv(session); + mutex_init(&ps->sk_lock); ps->tunnel_sock = tunnel->sock;

error = l2tp_session_register(session, tunnel); @@ -1005,12 +1044,10 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session, "%s: pppol2tp_session_ioctl(cmd=%#x, arg=%#lx)\n", session->name, cmd, arg);

- sk = ps->sock; + sk = pppol2tp_session_get_sock(session); if (!sk) return -EBADR;

- sock_hold(sk); - switch (cmd) { case SIOCGIFMTU: err = -ENXIO; @@ -1286,7 +1323,6 @@ static int pppol2tp_session_setsockopt(struct sock *sk, int optname, int val) { int err = 0; - struct pppol2tp_session *ps = l2tp_session_priv(session);

switch (optname) { case PPPOL2TP_SO_RECVSEQ: @@ -1307,8 +1343,8 @@ static int pppol2tp_session_setsockopt(struct sock *sk, } session->send_seq = val ? -1 : 0; { - struct sock *ssk = ps->sock; - struct pppox_sock *po = pppox_sk(ssk); + struct pppox_sock *po = pppox_sk(sk); + po->chan.hdrlen = val ? PPPOL2TP_L2TP_HDR_SIZE_SEQ : PPPOL2TP_L2TP_HDR_SIZE_NOSEQ; } @@ -1644,8 +1680,9 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v) { struct l2tp_session *session = v; struct l2tp_tunnel *tunnel = session->tunnel; - struct pppol2tp_session *ps = l2tp_session_priv(session); - struct pppox_sock *po = pppox_sk(ps->sock); + unsigned char state; + char user_data_ok; + struct sock *sk; u32 ip = 0; u16 port = 0;

@@ -1655,6 +1692,15 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v) port = ntohs(inet->inet_sport); }

+ sk = pppol2tp_session_get_sock(session); + if (sk) { + state = sk->sk_state; + user_data_ok = (session == sk->sk_user_data) ? 'Y' : 'N'; + } else { + state = 0; + user_data_ok = 'N'; + } + seq_printf(m, " SESSION '%s' %08X/%d %04X/%04X -> " "%04X/%04X %d %c\n", session->name, ip, port, @@ -1662,9 +1708,7 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v) session->session_id, tunnel->peer_tunnel_id, session->peer_session_id, - ps->sock->sk_state, - (session == ps->sock->sk_user_data) ? - 'Y' : 'N'); + state, user_data_ok); seq_printf(m, " %d/%d/%c/%c/%s %08x %u\n", session->mtu, session->mru, session->recv_seq ? 'R' : '-', @@ -1681,8 +1725,12 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v) atomic_long_read(&session->stats.rx_bytes), atomic_long_read(&session->stats.rx_errors));

- if (po) + if (sk) { + struct pppox_sock *po = pppox_sk(sk); + seq_printf(m, " interface %s\n", ppp_dev_name(&po->chan)); + sock_put(sk); + } }

static int pppol2tp_seq_show(struct seq_file *m, void *v)