| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20220627135501.713980-2-jiri@resnulli.us>
Date: Mon, 27 Jun 2022 15:55:00 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net, kuba@...nel.org, idosch@...dia.com,
petrm@...dia.com, pabeni@...hat.com, edumazet@...gle.com,
mlxsw@...dia.com, saeedm@...dia.com
Subject: [patch net-next RFC 1/2] net: devlink: make sure that devlink_try_get() works with valid pointer during xarray iteration
From: Jiri Pirko <jiri@...dia.com>
Remove dependency on devlink_mutex during devlinks xarray iteration.
The devlinks xarray consistency is ensured by internally by xarray.
There is a reference taken when working with devlink using
devlink_try_get(). But there is no guarantee that devlink pointer
picked during xarray iteration is not freed before devlink_try_get()
is called.
Make sure that devlink_try_get() works with valid pointer.
Achieve it by:
1) Splitting devlink_put() so the completion is sent only
after grace period. Completion unblocks the devlink_unregister()
routine, which is followed-up by devlink_free()
2) Iterate the devlink xarray holding RCU read lock.
Signed-off-by: Jiri Pirko <jiri@...dia.com>
---
net/core/devlink.c | 113 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 95 insertions(+), 18 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index db61f3a341cb..9e34b4b9b19b 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -69,6 +69,7 @@ struct devlink {
u8 reload_failed:1;
refcount_t refcount;
struct completion comp;
+ struct rcu_head rcu;
char priv[] __aligned(NETDEV_ALIGN);
};
@@ -220,8 +221,6 @@ static DEFINE_XARRAY_FLAGS(devlinks, XA_FLAGS_ALLOC);
/* devlink_mutex
*
* An overall lock guarding every operation coming from userspace.
- * It also guards devlink devices list and it is taken when
- * driver registers/unregisters it.
*/
static DEFINE_MUTEX(devlink_mutex);
@@ -231,10 +230,21 @@ struct net *devlink_net(const struct devlink *devlink)
}
EXPORT_SYMBOL_GPL(devlink_net);
+static void __devlink_put_rcu(struct rcu_head *head)
+{
+ struct devlink *devlink = container_of(head, struct devlink, rcu);
+
+ complete(&devlink->comp);
+}
+
void devlink_put(struct devlink *devlink)
{
if (refcount_dec_and_test(&devlink->refcount))
- complete(&devlink->comp);
+ /* Make sure unregister operation that may await the completion
+ * is unblocked only after all users are after the enf of
+ * RCU grace period.
+ */
+ call_rcu(&devlink->rcu, __devlink_put_rcu);
}
struct devlink *__must_check devlink_try_get(struct devlink *devlink)
@@ -288,6 +298,7 @@ static struct devlink *devlink_get_from_attrs(struct net *net,
lockdep_assert_held(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (strcmp(devlink->dev->bus->name, busname) == 0 &&
strcmp(dev_name(devlink->dev), devname) == 0 &&
@@ -299,6 +310,7 @@ static struct devlink *devlink_get_from_attrs(struct net *net,
if (!found || !devlink_try_get(devlink))
devlink = ERR_PTR(-ENODEV);
+ rcu_read_unlock();
return devlink;
}
@@ -1322,9 +1334,11 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
int err = 0;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -1351,7 +1365,9 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
@@ -1425,29 +1441,32 @@ static int devlink_nl_cmd_get_dumpit(struct sk_buff *msg,
int err;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
- if (!net_eq(devlink_net(devlink), sock_net(msg->sk))) {
- devlink_put(devlink);
- continue;
- }
+ if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
+ goto retry;
- if (idx < start) {
- idx++;
- devlink_put(devlink);
- continue;
- }
+ if (idx < start)
+ goto inc;
err = devlink_nl_fill(msg, devlink, DEVLINK_CMD_NEW,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, NLM_F_MULTI);
- devlink_put(devlink);
- if (err)
+ if (err) {
+ devlink_put(devlink);
goto out;
+ }
+inc:
idx++;
+retry:
+ devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -1488,9 +1507,11 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
int err;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -1516,7 +1537,9 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -2173,9 +2196,11 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg,
int err;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -2204,7 +2229,9 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->linecards_lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -2445,9 +2472,11 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
int err;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -2473,7 +2502,9 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -2597,9 +2628,11 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
int err = 0;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
!devlink->ops->sb_pool_get)
@@ -2622,7 +2655,9 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -2818,9 +2853,11 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
int err = 0;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
!devlink->ops->sb_port_pool_get)
@@ -2843,7 +2880,9 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -3067,9 +3106,11 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
int err = 0;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
!devlink->ops->sb_tc_pool_bind_get)
@@ -3093,7 +3134,9 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -5154,9 +5197,11 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
int err = 0;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -5184,7 +5229,9 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -5389,9 +5436,11 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
int err = 0;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -5424,7 +5473,9 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -5973,9 +6024,11 @@ static int devlink_nl_cmd_region_get_dumpit(struct sk_buff *msg,
int err = 0;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -5986,7 +6039,9 @@ static int devlink_nl_cmd_region_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
if (err)
goto out;
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
@@ -6507,9 +6562,11 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
int err = 0;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -6533,7 +6590,9 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
idx++;
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
@@ -7687,9 +7746,11 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
int err;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry_rep;
@@ -7715,11 +7776,13 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->reporters_lock);
retry_rep:
devlink_put(devlink);
+ rcu_read_lock();
}
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry_port;
@@ -7750,7 +7813,9 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry_port:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -8287,9 +8352,11 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
int err;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -8315,7 +8382,9 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -8514,9 +8583,11 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
int err;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -8543,7 +8614,9 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -8828,9 +8901,11 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
int err;
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -8857,7 +8932,9 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
out:
mutex_unlock(&devlink_mutex);
@@ -9585,10 +9662,8 @@ void devlink_register(struct devlink *devlink)
ASSERT_DEVLINK_NOT_REGISTERED(devlink);
/* Make sure that we are in .probe() routine */
- mutex_lock(&devlink_mutex);
xa_set_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
devlink_notify_register(devlink);
- mutex_unlock(&devlink_mutex);
}
EXPORT_SYMBOL_GPL(devlink_register);
@@ -9605,10 +9680,8 @@ void devlink_unregister(struct devlink *devlink)
devlink_put(devlink);
wait_for_completion(&devlink->comp);
- mutex_lock(&devlink_mutex);
devlink_notify_unregister(devlink);
xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
- mutex_unlock(&devlink_mutex);
}
EXPORT_SYMBOL_GPL(devlink_unregister);
@@ -12118,9 +12191,11 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
* all devlink instances from this namespace into init_net.
*/
mutex_lock(&devlink_mutex);
+ rcu_read_lock();
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ rcu_read_unlock();
if (!net_eq(devlink_net(devlink), net))
goto retry;
@@ -12134,7 +12209,9 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
pr_warn("Failed to reload devlink instance into init_net\n");
retry:
devlink_put(devlink);
+ rcu_read_lock();
}
+ rcu_read_unlock();
mutex_unlock(&devlink_mutex);
}
--
2.35.3
Powered by blists - more mailing lists