| 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: <20260116-nvmem-unbind-v1-7-7bb401ab19a8@oss.qualcomm.com>
Date: Fri, 16 Jan 2026 12:01:14 +0100
From: Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>
To: Srinivas Kandagatla <srini@...nel.org>,
Bartosz Golaszewski <brgl@...nel.org>
Cc: linux-kernel@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>
Subject: [PATCH 7/7] nvmem: synchronize nvmem device unregistering with
SRCU
With the provider-owned data split out into a separate structure, we can
now protect it with SRCU.
Protect all dereferences of nvmem->impl with an SRCU read lock.
Synchronize SRCU in nvmem_unregister() after setting the implementation
pointer to NULL. This has the effect of numbing down the device after
nvmem_unregister() returns - it will no longer accept any consumer calls
and return -ENODEV. The actual device will live on for as long as there
are references to it but we will no longer reach into the consumer's
memory which may be gone by this time.
The change has the added benefit of dropping the - now redundant -
reference counting with kref. We are left with a single release()
function depending on the kobject reference counting provided by struct
device.
Nvmem cell entries are destroyed in .release() now as they may be still
dereferenced via the nvmem_cell handles after nvmem_release(). The
actual calls will still go through SRCU and fail with -ENODEV if the
provider is gone.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>
---
drivers/nvmem/core.c | 115 +++++++++++++++++++++++++++-------------------
drivers/nvmem/internals.h | 5 +-
2 files changed, 72 insertions(+), 48 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index f6892af8ace52d033846f4052722289c2aa826df..1acc65026eb5094658e5523104c649a280bfc471 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -12,7 +12,6 @@
#include <linux/fs.h>
#include <linux/idr.h>
#include <linux/init.h>
-#include <linux/kref.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/nvmem-consumer.h>
@@ -57,7 +56,12 @@ static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
void *val, size_t bytes)
{
- struct nvmem_impl *impl = nvmem->impl;
+ struct nvmem_impl *impl;
+
+ guard(srcu)(&nvmem->srcu);
+ impl = rcu_dereference(nvmem->impl);
+ if (!impl)
+ return -ENODEV;
if (!impl->reg_read)
return -EOPNOTSUPP;
@@ -68,9 +72,14 @@ static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
void *val, size_t bytes)
{
- struct nvmem_impl *impl = nvmem->impl;
+ struct nvmem_impl *impl;
int ret, written;
+ guard(srcu)(&nvmem->srcu);
+ impl = rcu_dereference(nvmem->impl);
+ if (!impl)
+ return -ENODEV;
+
if (!impl->reg_write)
return -EOPNOTSUPP;
@@ -289,10 +298,14 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
static umode_t nvmem_bin_attr_get_umode(struct nvmem_device *nvmem)
{
- struct nvmem_impl *impl = nvmem->impl;
-
+ struct nvmem_impl *impl;
umode_t mode = 0400;
+ guard(srcu)(&nvmem->srcu);
+ impl = rcu_dereference(nvmem->impl);
+ if (!impl)
+ return 0;
+
if (!nvmem->root_only)
mode |= 0044;
@@ -333,7 +346,12 @@ static umode_t nvmem_attr_is_visible(struct kobject *kobj,
{
struct device *dev = kobj_to_dev(kobj);
struct nvmem_device *nvmem = to_nvmem_device(dev);
- struct nvmem_impl *impl = nvmem->impl;
+ struct nvmem_impl *impl;
+
+ guard(srcu)(&nvmem->srcu);
+ impl = rcu_dereference(nvmem->impl);
+ if (!impl)
+ return 0;
/*
* If the device has no .reg_write operation, do not allow
@@ -537,24 +555,6 @@ static void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
#endif /* CONFIG_NVMEM_SYSFS */
-static void nvmem_release(struct device *dev)
-{
- struct nvmem_device *nvmem = to_nvmem_device(dev);
-
- ida_free(&nvmem_ida, nvmem->id);
- gpiod_put(nvmem->wp_gpio);
- kfree(nvmem->impl);
- kfree(nvmem);
-}
-
-static const struct device_type nvmem_provider_type = {
- .release = nvmem_release,
-};
-
-static const struct bus_type nvmem_bus_type = {
- .name = "nvmem",
-};
-
static void nvmem_cell_entry_drop(struct nvmem_cell_entry *cell)
{
blocking_notifier_call_chain(&nvmem_notifier, NVMEM_CELL_REMOVE, cell);
@@ -573,6 +573,23 @@ static void nvmem_device_remove_all_cells(const struct nvmem_device *nvmem)
nvmem_cell_entry_drop(cell);
}
+static void nvmem_release(struct device *dev)
+{
+ struct nvmem_device *nvmem = to_nvmem_device(dev);
+
+ nvmem_device_remove_all_cells(nvmem);
+ ida_free(&nvmem_ida, nvmem->id);
+ kfree(nvmem);
+}
+
+static const struct device_type nvmem_provider_type = {
+ .release = nvmem_release,
+};
+
+static const struct bus_type nvmem_bus_type = {
+ .name = "nvmem",
+};
+
static void nvmem_cell_entry_add(struct nvmem_cell_entry *cell)
{
scoped_guard(mutex, &nvmem_mutex)
@@ -951,7 +968,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
goto err_put_device;
}
- kref_init(&nvmem->refcnt);
INIT_LIST_HEAD(&nvmem->cells);
nvmem->fixup_dt_cell_info = config->fixup_dt_cell_info;
@@ -959,7 +975,12 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
impl->reg_read = config->reg_read;
impl->reg_write = config->reg_write;
- nvmem->impl = impl;
+ rval = init_srcu_struct(&nvmem->srcu);
+ if (rval)
+ goto err_put_device;
+
+ rcu_assign_pointer(nvmem->impl, impl);
+
nvmem->owner = config->owner;
if (!nvmem->owner && config->dev->driver)
nvmem->owner = config->dev->driver->owner;
@@ -1064,32 +1085,28 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
}
EXPORT_SYMBOL_GPL(nvmem_register);
-static void nvmem_device_release(struct kref *kref)
+/**
+ * nvmem_unregister() - Unregister previously registered nvmem device
+ *
+ * @nvmem: Pointer to previously registered nvmem device.
+ */
+void nvmem_unregister(struct nvmem_device *nvmem)
{
- struct nvmem_device *nvmem;
-
- nvmem = container_of(kref, struct nvmem_device, refcnt);
+ struct nvmem_impl *impl;
blocking_notifier_call_chain(&nvmem_notifier, NVMEM_REMOVE, nvmem);
if (nvmem->flags & FLAG_COMPAT)
device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
- nvmem_device_remove_all_cells(nvmem);
+ impl = rcu_replace_pointer(nvmem->impl, NULL, true);
+ synchronize_srcu(&nvmem->srcu);
+
nvmem_destroy_layout(nvmem);
+ kfree(impl);
+ gpiod_put(nvmem->wp_gpio);
device_unregister(&nvmem->dev);
}
-
-/**
- * nvmem_unregister() - Unregister previously registered nvmem device
- *
- * @nvmem: Pointer to previously registered nvmem device.
- */
-void nvmem_unregister(struct nvmem_device *nvmem)
-{
- if (nvmem)
- kref_put(&nvmem->refcnt, nvmem_device_release);
-}
EXPORT_SYMBOL_GPL(nvmem_unregister);
static void devm_nvmem_unregister(void *nvmem)
@@ -1149,8 +1166,6 @@ static struct nvmem_device *__nvmem_device_get(void *data,
return ERR_PTR(-EINVAL);
}
- kref_get(&nvmem->refcnt);
-
return nvmem;
}
@@ -1266,9 +1281,8 @@ EXPORT_SYMBOL_GPL(devm_nvmem_device_put);
*/
void nvmem_device_put(struct nvmem_device *nvmem)
{
- put_device(&nvmem->dev);
module_put(nvmem->owner);
- kref_put(&nvmem->refcnt, nvmem_device_release);
+ put_device(&nvmem->dev);
}
EXPORT_SYMBOL_GPL(nvmem_device_put);
@@ -1655,6 +1669,15 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem,
{
int rc;
+ /*
+ * Take the SRCU read lock earlier. It will be taken again in
+ * nvmem_reg_read() but that's alright, they can be nested. If
+ * nvmem_reg_read() returns -ENODEV, we'll return right way. If it
+ * succeeds, we need to stay within the SRCU read-critical section
+ * until we're done calling cell->read_post_process().
+ */
+ guard(srcu)(&nvmem->srcu);
+
rc = nvmem_reg_read(nvmem, cell->offset, buf, cell->raw_len);
if (rc)
diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
index 05197074799ff3e2a6720f6552878a9e1354a5c3..5afb1297a93a38e399085391130c4df99f64af16 100644
--- a/drivers/nvmem/internals.h
+++ b/drivers/nvmem/internals.h
@@ -6,6 +6,7 @@
#include <linux/device.h>
#include <linux/nvmem-consumer.h>
#include <linux/nvmem-provider.h>
+#include <linux/srcu.h>
/*
* Holds data owned by the provider of the nvmem implementation. This goes
@@ -20,11 +21,11 @@ struct nvmem_impl {
struct nvmem_device {
struct module *owner;
struct device dev;
- struct nvmem_impl *impl;
+ struct nvmem_impl __rcu *impl;
+ struct srcu_struct srcu;
int stride;
int word_size;
int id;
- struct kref refcnt;
size_t size;
bool read_only;
bool root_only;
--
2.47.3
Powered by blists - more mailing lists