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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 30 Oct 2021 16:12:51 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     leon@...nel.org, idosch@...sch.org
Cc:     edwin.peer@...adcom.com, jiri@...nulli.us, netdev@...r.kernel.org,
        Jakub Kicinski <kuba@...nel.org>
Subject: [RFC 2/5] devlink: add API for explicit locking

Expose devlink instance lock and create the registration
helpers as needed. The new unregistration helper does
not wait for all refs to be gone, and free just gives
up the reference instead of actually freeing the memory.

Signed-off-by: Jakub Kicinski <kuba@...nel.org>
---
 include/net/devlink.h |  16 ++++++
 net/core/devlink.c    | 122 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 130 insertions(+), 8 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index d8e4274e2af4..66c1951a6f0e 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1191,6 +1191,7 @@ enum {
 };
 
 struct devlink_ops {
+	struct module *owner;
 	/**
 	 * @supported_flash_update_params:
 	 * mask of parameters supported by the driver's .flash_update
@@ -1490,6 +1491,8 @@ void *devlink_priv(struct devlink *devlink);
 struct devlink *priv_to_devlink(void *priv);
 struct device *devlink_to_dev(const struct devlink *devlink);
 
+int lockdep_devlink_is_held(struct devlink *devlink);
+
 struct ib_device;
 
 struct net *devlink_net(const struct devlink *devlink);
@@ -1507,10 +1510,23 @@ static inline struct devlink *devlink_alloc(const struct devlink_ops *ops,
 {
 	return devlink_alloc_ns(ops, priv_size, &init_net, dev);
 }
+
+void devlink_lock(struct devlink *devlink);
+void devlink_unlock(struct devlink *devlink);
+
+void devlink_lock_reg(struct devlink *devlink);
+void devlink_unlock_reg(struct devlink *devlink);
+
+void devlink_get(struct devlink *devlink);
+bool devlink_is_alive(const struct devlink *devlink);
+
 void devlink_set_features(struct devlink *devlink, u64 features);
 void devlink_register(struct devlink *devlink);
+void __devlink_register(struct devlink *devlink);
 void devlink_unregister(struct devlink *devlink);
+void __devlink_unregister(struct devlink *devlink);
 void devlink_free(struct devlink *devlink);
+void __devlink_free(struct devlink *devlink);
 int devlink_port_register(struct devlink *devlink,
 			  struct devlink_port *devlink_port,
 			  unsigned int port_index);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 9ea0c0bbc796..6783b066f9a7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -182,11 +182,24 @@ struct net *devlink_net(const struct devlink *devlink)
 }
 EXPORT_SYMBOL_GPL(devlink_net);
 
+void devlink_free(struct devlink *devlink);
+
 void devlink_put(struct devlink *devlink)
 {
-	if (refcount_dec_and_test(&devlink->refcount))
-		complete(&devlink->comp);
+	if (refcount_dec_and_test(&devlink->refcount)) {
+		if (devlink->ops->lock_flags & DEVLINK_LOCK_REF_MODE)
+			devlink_free(devlink);
+		else
+			complete(&devlink->comp);
+	}
 }
+EXPORT_SYMBOL_GPL(devlink_put);
+
+void devlink_get(struct devlink *devlink)
+{
+	refcount_inc(&devlink->refcount);
+}
+EXPORT_SYMBOL_GPL(devlink_get);
 
 struct devlink *__must_check devlink_try_get(struct devlink *devlink)
 {
@@ -195,6 +208,60 @@ struct devlink *__must_check devlink_try_get(struct devlink *devlink)
 	return NULL;
 }
 
+bool devlink_is_alive(const struct devlink *devlink)
+{
+	return xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
+}
+EXPORT_SYMBOL_GPL(devlink_is_alive);
+
+/**
+ * devlink_lock_reg() - take devlink registration lock as well as instance lock
+ * @devlink: instance to lock
+ */
+void devlink_lock_reg(struct devlink *devlink)
+{
+	mutex_lock(&devlink_mutex);
+	mutex_lock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_lock_reg);
+
+/**
+ * devlink_unlock_reg()
+ * @devlink: instance to lock
+ */
+void devlink_unlock_reg(struct devlink *devlink)
+{
+	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink_mutex);
+}
+EXPORT_SYMBOL_GPL(devlink_unlock_reg);
+
+/**
+ * devlink_lock() - lock devlink instance
+ * @devlink: instance to lock
+ */
+void devlink_lock(struct devlink *devlink)
+{
+	mutex_lock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_lock);
+
+/**
+ * devlink_unlock() - release devlink instance lock
+ * @devlink: instance to unlock
+ */
+void devlink_unlock(struct devlink *devlink)
+{
+	mutex_unlock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_unlock);
+
+int lockdep_devlink_is_held(struct devlink *devlink)
+{
+	return lockdep_is_held(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(lockdep_devlink_is_held);
+
 static struct devlink *devlink_get_from_attrs(struct net *net,
 					      struct nlattr **attrs)
 {
@@ -9016,6 +9083,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
 	mutex_init(&devlink->reporters_lock);
 	refcount_set(&devlink->refcount, 1);
 	init_completion(&devlink->comp);
+	__module_get(devlink->ops->owner);
 
 	return devlink;
 }
@@ -9105,6 +9173,18 @@ static void devlink_notify_unregister(struct devlink *devlink)
 	devlink_notify(devlink, DEVLINK_CMD_DEL);
 }
 
+void __devlink_register(struct devlink *devlink)
+{
+	ASSERT_DEVLINK_NOT_REGISTERED(devlink);
+	lockdep_assert_held(&devlink->lock);
+	lockdep_assert_held(&devlink_mutex);
+	/* Make sure that we are in .probe() routine */
+
+	xa_set_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
+	devlink_notify_register(devlink);
+}
+EXPORT_SYMBOL_GPL(__devlink_register);
+
 /**
  *	devlink_register - Register devlink instance
  *
@@ -9112,16 +9192,27 @@ static void devlink_notify_unregister(struct devlink *devlink)
  */
 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);
+	__devlink_register(devlink);
 	mutex_unlock(&devlink_mutex);
 }
 EXPORT_SYMBOL_GPL(devlink_register);
 
+/* Note: this is NOT an equivalent to devlink_unregister() sans locking
+ * it also accounts for the ability to take references on the instance.
+ */
+void __devlink_unregister(struct devlink *devlink)
+{
+	ASSERT_DEVLINK_REGISTERED(devlink);
+	/* Make sure that we are in .remove() routine */
+	WARN_ON(!(devlink->ops->lock_flags & DEVLINK_LOCK_REF_MODE));
+	lockdep_assert_held(&devlink->lock);
+
+	devlink_notify_unregister(devlink);
+	xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
+}
+EXPORT_SYMBOL_GPL(__devlink_unregister);
+
 /**
  *	devlink_unregister - Unregister devlink instance
  *
@@ -9131,6 +9222,7 @@ void devlink_unregister(struct devlink *devlink)
 {
 	ASSERT_DEVLINK_REGISTERED(devlink);
 	/* Make sure that we are in .remove() routine */
+	WARN_ON(devlink->ops->lock_flags & DEVLINK_LOCK_REF_MODE);
 
 	devlink_put(devlink);
 	wait_for_completion(&devlink->comp);
@@ -9142,6 +9234,12 @@ void devlink_unregister(struct devlink *devlink)
 }
 EXPORT_SYMBOL_GPL(devlink_unregister);
 
+void __devlink_free(struct devlink *devlink)
+{
+	devlink_put(devlink);
+}
+EXPORT_SYMBOL_GPL(__devlink_free);
+
 /**
  *	devlink_free - Free devlink instance resources
  *
@@ -9168,6 +9266,8 @@ void devlink_free(struct devlink *devlink)
 	xa_destroy(&devlink->snapshot_ids);
 	xa_erase(&devlinks, devlink->index);
 
+	module_put(devlink->ops->owner);
+
 	kfree(devlink);
 }
 EXPORT_SYMBOL_GPL(devlink_free);
@@ -11502,7 +11602,8 @@ void devlink_compat_running_version(struct devlink *devlink,
 		return;
 
 	mutex_lock(&devlink->lock);
-	__devlink_compat_running_version(devlink, buf, len);
+	if (devlink_is_alive(devlink))
+		__devlink_compat_running_version(devlink, buf, len);
 	mutex_unlock(&devlink->lock);
 }
 
@@ -11519,9 +11620,14 @@ int devlink_compat_flash_update(struct devlink *devlink, const char *file_name)
 		return ret;
 
 	mutex_lock(&devlink->lock);
+	if (!devlink_is_alive(devlink)) {
+		ret = -ENODEV;
+		goto exit_unlock;
+	}
 	devlink_flash_update_begin_notify(devlink);
 	ret = devlink->ops->flash_update(devlink, &params, NULL);
 	devlink_flash_update_end_notify(devlink);
+exit_unlock:
 	mutex_unlock(&devlink->lock);
 
 	release_firmware(params.fw);
-- 
2.31.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ