From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, greg@kroah.com
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Guillaume Nault <g.nault@alphalink.fr>,
"David S. Miller" <davem@davemloft.net>,
Giuliano Procida <gprocida@google.com>
Subject: [PATCH 4.4 49/65] l2tp: hold tunnel while looking up sessions in l2tp_netlink
Date: Tue, 26 May 2020 20:53:08 +0200 [thread overview]
Message-ID: <20200526183922.958551997@linuxfoundation.org> (raw)
In-Reply-To: <20200526183905.988782958@linuxfoundation.org>
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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
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(-)
--- 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_pern
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
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.
*/
@@ -1351,17 +1338,6 @@ static void l2tp_udp_encap_destroy(struc
}
}
-/* 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)
{
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -234,6 +234,8 @@ out:
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);
@@ -272,6 +274,17 @@ int l2tp_nl_register_ops(enum l2tp_pwtyp
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.
*/
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -72,10 +72,12 @@ static struct l2tp_session *l2tp_nl_sess
(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;
next prev parent reply other threads:[~2020-05-26 19:32 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-26 18:52 [PATCH 4.4 00/65] 4.4.225-rc1 review Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 01/65] igb: use igb_adapter->io_addr instead of e1000_hw->hw_addr Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 02/65] padata: Remove unused but set variables Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 03/65] padata: get_next is never NULL Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 04/65] padata: ensure the reorder timer callback runs on the correct CPU Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 05/65] padata: ensure padata_do_serial() " Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 06/65] evm: Check also if *tfm is an error pointer in init_desc() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 07/65] fix multiplication overflow in copy_fdtable() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 08/65] HID: multitouch: add eGalaxTouch P80H84 support Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 09/65] ceph: fix double unlock in handle_cap_export() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 10/65] USB: core: Fix misleading driver bug report Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 11/65] platform/x86: asus-nb-wmi: Do not load on Asus T100TA and T200TA Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 12/65] ARM: futex: Address build warning Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 13/65] media: Fix media_open() to clear filp->private_data in error leg Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 14/65] drivers/media/media-devnode: clear private_data before put_device() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 15/65] media-devnode: add missing mutex lock in error handler Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 16/65] media-devnode: fix namespace mess Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 17/65] media-device: dynamically allocate struct media_devnode Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 18/65] media: fix use-after-free in cdev_put() when app exits after driver unbind Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 19/65] media: fix media devnode ioctl/syscall and unregister race Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 20/65] i2c: dev: switch from register_chrdev to cdev API Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 21/65] i2c: dev: dont start function name with return Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 22/65] i2c: dev: use after free in detach Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 23/65] i2c-dev: dont get i2c adapter via i2c_dev Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 24/65] i2c: dev: Fix the race between the release of i2c_dev and cdev Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 25/65] padata: set cpu_index of unused CPUs to -1 Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 26/65] sched/fair, cpumask: Export for_each_cpu_wrap() Greg Kroah-Hartman
2020-05-27 7:50 ` nobuhiro1.iwamatsu
2020-05-27 8:09 ` Greg KH
2020-05-27 14:03 ` Daniel Jordan
2020-05-26 18:52 ` [PATCH 4.4 27/65] padata: Replace delayed timer with immediate workqueue in padata_reorder Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 28/65] padata: initialize pd->cpu with effective cpumask Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 29/65] padata: purge get_cpu and reorder_via_wq from padata_do_serial Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 30/65] ALSA: pcm: fix incorrect hw_base increase Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 31/65] ext4: lock the xattr block before checksuming it Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 32/65] platform/x86: alienware-wmi: fix kfree on potentially uninitialized pointer Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 33/65] libnvdimm/btt: Remove unnecessary code in btt_freelist_init Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 34/65] l2tp: lock socket before checking flags in connect() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 35/65] l2tp: fix racy socket lookup in l2tp_ip and l2tp_ip6 bind() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 36/65] l2tp: hold session while sending creation notifications Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 37/65] l2tp: take a reference on sessions used in genetlink handlers Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 38/65] l2tp: dont use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6 Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 39/65] net: l2tp: export debug flags to UAPI Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 40/65] net: l2tp: deprecate PPPOL2TP_MSG_* in favour of L2TP_MSG_* Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 41/65] net: l2tp: ppp: change PPPOL2TP_MSG_* => L2TP_MSG_* Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 42/65] New kernel function to get IP overhead on a socket Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 43/65] L2TP:Adjust intf MTU, add underlay L3, L2 hdrs Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 44/65] l2tp: remove useless duplicate session detection in l2tp_netlink Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 45/65] l2tp: remove l2tp_session_find() Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 46/65] l2tp: define parameters of l2tp_session_get*() as "const" Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 47/65] l2tp: define parameters of l2tp_tunnel_find*() " Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 48/65] l2tp: initialise sessions refcount before making it reachable Greg Kroah-Hartman
2020-05-26 18:53 ` Greg Kroah-Hartman [this message]
2020-05-26 18:53 ` [PATCH 4.4 50/65] l2tp: hold tunnel while processing genl delete command Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 51/65] l2tp: hold tunnel while handling genl tunnel updates Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 52/65] l2tp: hold tunnel while handling genl TUNNEL_GET commands Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 53/65] l2tp: hold tunnel used while creating sessions with netlink Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 54/65] l2tp: prevent creation of sessions on terminated tunnels Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 55/65] l2tp: pass tunnel pointer to ->session_create() Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 56/65] l2tp: fix l2tp_eth module loading Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 57/65] l2tp: dont register sessions in l2tp_session_create() Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 58/65] l2tp: initialise l2tp_eth sessions before registering them Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 59/65] l2tp: protect sock pointer of struct pppol2tp_session with RCU Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 60/65] l2tp: initialise PPP sessions before registering them Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 61/65] Revert "gfs2: Dont demote a glock until its revokes are written" Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 62/65] staging: iio: ad2s1210: Fix SPI reading Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 63/65] mei: release me_cl object reference Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 64/65] iio: sca3000: Remove an erroneous get_device() Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 65/65] l2tp: device MTU setup, tunnel socket needs a lock Greg Kroah-Hartman
[not found] ` <20200526183905.988782958-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
2020-05-27 8:32 ` [PATCH 4.4 00/65] 4.4.225-rc1 review Jon Hunter
2020-05-27 8:32 ` Jon Hunter
2020-05-27 8:52 ` Naresh Kamboju
2020-05-27 10:30 ` Chris Paterson
2020-05-27 13:50 ` Guenter Roeck
2020-05-27 17:16 ` shuah
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200526183922.958551997@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=davem@davemloft.net \
--cc=g.nault@alphalink.fr \
--cc=gprocida@google.com \
--cc=greg@kroah.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.