[PATCH v5 0/9] nvmem: rework nvmem core and allow unbinding with active consumers

[PATCH v5 0/9] nvmem: rework nvmem core and allow unbinding with active consumers

[Bartosz Golaszewski]

Sashiko pointed out some issues so this iteration fixes them. I'm also
Cc'ing Loic who seems to have encountered the issue of unbinding with
active consumers when working on the block nvmem provider.

Nvmem is one of the subsystems vulnerable to object life-time issues.
The memory nvmem core dereferences is owned by nvmem providers which can
be unbound at any time and even though nvmem devices themselves are
reference-counted, there's no synchronization with the provider modules.

This typically is not a problem because thanks to fw_devlink, consumers
get synchronously unbound before providers but it's enough to pass
fw_devlink=off over the command line, unbind the nvmem controller with
consumers still holding references to it and try to read/write in order
to see fireworks in the kernel log.

User-space can trigger it too if a device (for instance: i2c eeprom on a
cp2112 USB expander) is unplugged halfway through a long read.

This series proposes to use SRCU to protect nvmem against accessing
invalid memory after unbinding with active consumers and also reworks
several places in nvmem core.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Changes in v5:
- Rebase on top of v7.2-rc1
- Drop applied patch from the series
- Link to v4: https://patch.msgid.link/20260521-nvmem-unbind-v4-0-7fa136759491@oss.qualcomm.com

Changes in v4:
- Restore the removed checks for the existence of reg_write/reg_read
  ops in sysfs callbacks as the attributes may be created with only a
  single operation available
- Fix potential use-after-free when decrementing the references to nvmem
  device
- Rename some local variables to better indicate their function
- Initialize the cell list before calling device_initialize() as we
  iterate over it in release path unconditionally
- Restore the nvmem != NULL check in nvmem_unregister() as sashiko
  pointed out there are users who rely on this API contract
- Don't use rcu_dereference() with SRCU as it may trigger a
  false-positive lockdep alert
- Synchronize the removal of nvmem->ops in error path in
  nvmem_register() as it's possible for it to be made available to the
  system before a subsequent failure later in the function
- Link to v3: https://patch.msgid.link/20260429-nvmem-unbind-v3-0-2a694f95395b@oss.qualcomm.com

Changes in v3:
- Add Fixes tag to patch 1
- Don't check the presence of read/write callbacks in sysfs attributes
  as these are not visible without them
- Rework mutex guards and drop unneeded helper variables
- Fix mutex guard conversion: it accidentally converted nvmem_lookup_mutex
  locks to nvmem_mutex
- Extend patch 5 to also rename __nvmem_device_get() to
  nvmem_device_match()
- Call nvmem_sysfs_remove_compat() on unregister, not release
- Split patch 7 into two: one removing the redundant kref and second
  adding SRCU
- Link to v2: https://patch.msgid.link/20260223-nvmem-unbind-v2-0-0df33a933dca@oss.qualcomm.com

Changes in v2:
- add missing SRCU struct cleanup
- improve the teardown path on error in nvmem_register()
- Link to v1: https://lore.kernel.org/r/20260116-nvmem-unbind-v1-0-7bb401ab19a8@oss.qualcomm.com

---
Bartosz Golaszewski (9):
      nvmem: remove unused field from struct nvmem_device
      nvmem: return -EOPNOTSUPP to in-kernel users on missing callbacks
      nvmem: check the return value of gpiod_set_value_cansleep()
      nvmem: simplify locking with guard()
      nvmem: remove unneeded __nvmem_device_put()
      nvmem: split out the reg_read/write() callbacks out of struct nvmem_device
      nvmem: simplify nvmem_sysfs_remove_compat()
      nvmem: remove duplicated reference counting
      nvmem: protect nvmem_device::ops with SRCU

 drivers/nvmem/core.c      | 313 +++++++++++++++++++++++++---------------------
 drivers/nvmem/internals.h |  13 +-
 2 files changed, 176 insertions(+), 150 deletions(-)
---
base-commit: 04e3a00b874c0e91494097bc986ccb74e953e757
change-id: 20260114-nvmem-unbind-673b52fc84a0

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

[PATCH v5 1/9] nvmem: remove unused field from struct nvmem_device

[Bartosz Golaszewski]

The node list_head in struct nvmem_device was added accidentally by
commit ec9c08a1cb8d ("nvmem: Create a header for internal sharing") and
is unused so remove it.

Fixes: ec9c08a1cb8d ("nvmem: Create a header for internal sharing")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/nvmem/internals.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
index 18fed57270e5e3391cf24d5e49836121a53a8cd6..7cbc55f40259fc4315c41979ad8bf75c36bcb056 100644
--- a/drivers/nvmem/internals.h
+++ b/drivers/nvmem/internals.h
@@ -10,7 +10,6 @@
 struct nvmem_device {
 	struct module		*owner;
 	struct device		dev;
-	struct list_head	node;
 	int			stride;
 	int			word_size;
 	int			id;

-- 
2.47.3

[PATCH v5 2/9] nvmem: return -EOPNOTSUPP to in-kernel users on missing callbacks

[Bartosz Golaszewski]

__nvmem_reg_read/write() currently return -EINVAL if the relevant
callback is not present. User-space helpers again check the presence
of the callbacks to see if they should return -EPERM.

Ahead of adding SRCU synchronization: change the error code returned to
in-kernel users to -EOPNOTSUPP which is more indicative of the actual
reason for the failure.

Remove the checks from the sysfs attribute callbacks as these are not
visible without the required callbacks.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/nvmem/core.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index e871181751f3c2739154b3cff27ef9b90032e607..06d7f3d819084579434370cd12c6b263abff196e 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -55,10 +55,10 @@ static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
 static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
 			    void *val, size_t bytes)
 {
-	if (nvmem->reg_read)
-		return nvmem->reg_read(nvmem->priv, offset, val, bytes);
+	if (!nvmem->reg_read)
+		return -EOPNOTSUPP;
 
-	return -EINVAL;
+	return nvmem->reg_read(nvmem->priv, offset, val, bytes);
 }
 
 static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
@@ -66,14 +66,14 @@ static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
 {
 	int ret;
 
-	if (nvmem->reg_write) {
-		gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
-		ret = nvmem->reg_write(nvmem->priv, offset, val, bytes);
-		gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
-		return ret;
-	}
+	if (!nvmem->reg_write)
+		return -EOPNOTSUPP;
+
+	gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
+	ret = nvmem->reg_write(nvmem->priv, offset, val, bytes);
+	gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
 
-	return -EINVAL;
+	return ret;
 }
 
 static int nvmem_access_with_keepouts(struct nvmem_device *nvmem,
@@ -231,13 +231,12 @@ static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj,
 
 	count = round_down(count, nvmem->word_size);
 
-	if (!nvmem->reg_read)
-		return -EPERM;
-
 	rc = nvmem_reg_read(nvmem, pos, buf, count);
-
-	if (rc)
+	if (rc) {
+		if (rc == -EOPNOTSUPP)
+			return -EPERM;
 		return rc;
+	}
 
 	return count;
 }
@@ -264,13 +263,15 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
 
 	count = round_down(count, nvmem->word_size);
 
-	if (!nvmem->reg_write || nvmem->read_only)
+	if (nvmem->read_only)
 		return -EPERM;
 
 	rc = nvmem_reg_write(nvmem, pos, buf, count);
-
-	if (rc)
+	if (rc) {
+		if (rc == -EOPNOTSUPP)
+			return -EPERM;
 		return rc;
+	}
 
 	return count;
 }

-- 
2.47.3

[PATCH v5 3/9] nvmem: check the return value of gpiod_set_value_cansleep()

[Bartosz Golaszewski]

GPIO setters now return integer values and can indicate failures in
lower abstraction layers. Check the return values of
gpiod_set_value_cansleep() calls in nvmem core.

Reviewed-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/nvmem/core.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 06d7f3d819084579434370cd12c6b263abff196e..bf5dde6288cfa75cab4040c6b93e3376e87fc691 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -64,16 +64,22 @@ 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)
 {
-	int ret;
+	int ret, wr_ok;
 
 	if (!nvmem->reg_write)
 		return -EOPNOTSUPP;
 
-	gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
-	ret = nvmem->reg_write(nvmem->priv, offset, val, bytes);
-	gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
+	ret = gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
+	if (ret)
+		return ret;
 
-	return ret;
+	wr_ok = nvmem->reg_write(nvmem->priv, offset, val, bytes);
+
+	ret = gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
+	if (ret)
+		return ret;
+
+	return wr_ok;
 }
 
 static int nvmem_access_with_keepouts(struct nvmem_device *nvmem,

-- 
2.47.3

[PATCH v5 4/9] nvmem: simplify locking with guard()

[Bartosz Golaszewski]

Use lock guards from cleanup.h to simplify locking. While at it: add the
missing mutex.h include.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/nvmem/core.c | 88 ++++++++++++++++++++++------------------------------
 1 file changed, 37 insertions(+), 51 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index bf5dde6288cfa75cab4040c6b93e3376e87fc691..623aeb9533c0124bb893d0c229570e00cfa3c52a 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2013 Maxime Ripard <maxime.ripard@free-electrons.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/export.h>
 #include <linux/fs.h>
@@ -13,6 +14,7 @@
 #include <linux/init.h>
 #include <linux/kref.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/nvmem-provider.h>
 #include <linux/gpio/consumer.h>
@@ -468,27 +470,23 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
 	const struct bin_attribute **pattrs;
 	struct bin_attribute *attrs;
 	unsigned int ncells = 0, i = 0;
-	int ret = 0;
+	int ret;
 
-	mutex_lock(&nvmem_mutex);
+	guard(mutex)(&nvmem_mutex);
 
 	if (list_empty(&nvmem->cells) || nvmem->sysfs_cells_populated)
-		goto unlock_mutex;
+		return 0;
 
 	/* Allocate an array of attributes with a sentinel */
 	ncells = list_count_nodes(&nvmem->cells);
 	pattrs = devm_kcalloc(&nvmem->dev, ncells + 1,
 			      sizeof(struct bin_attribute *), GFP_KERNEL);
-	if (!pattrs) {
-		ret = -ENOMEM;
-		goto unlock_mutex;
-	}
+	if (!pattrs)
+		return -ENOMEM;
 
 	attrs = devm_kcalloc(&nvmem->dev, ncells, sizeof(struct bin_attribute), GFP_KERNEL);
-	if (!attrs) {
-		ret = -ENOMEM;
-		goto unlock_mutex;
-	}
+	if (!attrs)
+		return -ENOMEM;
 
 	/* Initialize each attribute to take the name and size of the cell */
 	list_for_each_entry(entry, &nvmem->cells, node) {
@@ -501,10 +499,8 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
 		attrs[i].size = entry->bytes;
 		attrs[i].read = &nvmem_cell_attr_read;
 		attrs[i].private = entry;
-		if (!attrs[i].attr.name) {
-			ret = -ENOMEM;
-			goto unlock_mutex;
-		}
+		if (!attrs[i].attr.name)
+			return -ENOMEM;
 
 		pattrs[i] = &attrs[i];
 		i++;
@@ -514,13 +510,10 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
 
 	ret = device_add_group(&nvmem->dev, &group);
 	if (ret)
-		goto unlock_mutex;
+		return ret;
 
 	nvmem->sysfs_cells_populated = true;
 
-unlock_mutex:
-	mutex_unlock(&nvmem_mutex);
-
 	return ret;
 }
 
@@ -558,9 +551,8 @@ static const struct bus_type nvmem_bus_type = {
 static void nvmem_cell_entry_drop(struct nvmem_cell_entry *cell)
 {
 	blocking_notifier_call_chain(&nvmem_notifier, NVMEM_CELL_REMOVE, cell);
-	mutex_lock(&nvmem_mutex);
-	list_del(&cell->node);
-	mutex_unlock(&nvmem_mutex);
+	scoped_guard(mutex, &nvmem_mutex)
+		list_del(&cell->node);
 	of_node_put(cell->np);
 	kfree_const(cell->name);
 	kfree(cell);
@@ -576,9 +568,8 @@ static void nvmem_device_remove_all_cells(const struct nvmem_device *nvmem)
 
 static void nvmem_cell_entry_add(struct nvmem_cell_entry *cell)
 {
-	mutex_lock(&nvmem_mutex);
-	list_add_tail(&cell->node, &cell->nvmem->cells);
-	mutex_unlock(&nvmem_mutex);
+	scoped_guard(mutex, &nvmem_mutex)
+		list_add_tail(&cell->node, &cell->nvmem->cells);
 	blocking_notifier_call_chain(&nvmem_notifier, NVMEM_CELL_ADD, cell);
 }
 
@@ -728,14 +719,14 @@ nvmem_find_cell_entry_by_name(struct nvmem_device *nvmem, const char *cell_id)
 {
 	struct nvmem_cell_entry *iter, *cell = NULL;
 
-	mutex_lock(&nvmem_mutex);
+	guard(mutex)(&nvmem_mutex);
+
 	list_for_each_entry(iter, &nvmem->cells, node) {
 		if (strcmp(cell_id, iter->name) == 0) {
 			cell = iter;
 			break;
 		}
 	}
-	mutex_unlock(&nvmem_mutex);
 
 	return cell;
 }
@@ -1121,11 +1112,11 @@ static struct nvmem_device *__nvmem_device_get(void *data,
 	struct nvmem_device *nvmem = NULL;
 	struct device *dev;
 
-	mutex_lock(&nvmem_mutex);
-	dev = bus_find_device(&nvmem_bus_type, NULL, data, match);
-	if (dev)
-		nvmem = to_nvmem_device(dev);
-	mutex_unlock(&nvmem_mutex);
+	scoped_guard(mutex, &nvmem_mutex) {
+		dev = bus_find_device(&nvmem_bus_type, NULL, data, match);
+		if (dev)
+			nvmem = to_nvmem_device(dev);
+	}
 	if (!nvmem)
 		return ERR_PTR(-EPROBE_DEFER);
 
@@ -1335,7 +1326,7 @@ nvmem_cell_get_from_lookup(struct device *dev, const char *con_id)
 
 	dev_id = dev_name(dev);
 
-	mutex_lock(&nvmem_lookup_mutex);
+	guard(mutex)(&nvmem_lookup_mutex);
 
 	list_for_each_entry(lookup, &nvmem_lookup_list, node) {
 		if ((strcmp(lookup->dev_id, dev_id) == 0) &&
@@ -1343,11 +1334,9 @@ nvmem_cell_get_from_lookup(struct device *dev, const char *con_id)
 			/* This is the right entry. */
 			nvmem = __nvmem_device_get((void *)lookup->nvmem_name,
 						   device_match_name);
-			if (IS_ERR(nvmem)) {
+			if (IS_ERR(nvmem))
 				/* Provider may not be registered yet. */
-				cell = ERR_CAST(nvmem);
-				break;
-			}
+				return ERR_CAST(nvmem);
 
 			cell_entry = nvmem_find_cell_entry_by_name(nvmem,
 								   lookup->cell_name);
@@ -1363,7 +1352,6 @@ nvmem_cell_get_from_lookup(struct device *dev, const char *con_id)
 		}
 	}
 
-	mutex_unlock(&nvmem_lookup_mutex);
 	return cell;
 }
 
@@ -1377,18 +1365,16 @@ static void nvmem_layout_module_put(struct nvmem_device *nvmem)
 static struct nvmem_cell_entry *
 nvmem_find_cell_entry_by_node(struct nvmem_device *nvmem, struct device_node *np)
 {
-	struct nvmem_cell_entry *iter, *cell = NULL;
+	struct nvmem_cell_entry *cell;
 
-	mutex_lock(&nvmem_mutex);
-	list_for_each_entry(iter, &nvmem->cells, node) {
-		if (np == iter->np) {
-			cell = iter;
-			break;
-		}
+	guard(mutex)(&nvmem_mutex);
+
+	list_for_each_entry(cell, &nvmem->cells, node) {
+		if (np == cell->np)
+			return cell;
 	}
-	mutex_unlock(&nvmem_mutex);
 
-	return cell;
+	return NULL;
 }
 
 static int nvmem_layout_module_get_optional(struct nvmem_device *nvmem)
@@ -2121,10 +2107,10 @@ void nvmem_add_cell_lookups(struct nvmem_cell_lookup *entries, size_t nentries)
 {
 	int i;
 
-	mutex_lock(&nvmem_lookup_mutex);
+	guard(mutex)(&nvmem_lookup_mutex);
+
 	for (i = 0; i < nentries; i++)
 		list_add_tail(&entries[i].node, &nvmem_lookup_list);
-	mutex_unlock(&nvmem_lookup_mutex);
 }
 EXPORT_SYMBOL_GPL(nvmem_add_cell_lookups);
 
@@ -2139,10 +2125,10 @@ void nvmem_del_cell_lookups(struct nvmem_cell_lookup *entries, size_t nentries)
 {
 	int i;
 
-	mutex_lock(&nvmem_lookup_mutex);
+	guard(mutex)(&nvmem_lookup_mutex);
+
 	for (i = 0; i < nentries; i++)
 		list_del(&entries[i].node);
-	mutex_unlock(&nvmem_lookup_mutex);
 }
 EXPORT_SYMBOL_GPL(nvmem_del_cell_lookups);
 

-- 
2.47.3

[PATCH v5 5/9] nvmem: remove unneeded __nvmem_device_put()

[Bartosz Golaszewski]

__nvmem_device_put() is wrapped by nvmem_device_put() but there's no
extra functionality offered by the latter so just fold one into the
other. There's still the corresponding __nvmem_device_get() so in order
to keep things symmetrical: rename it to nvmem_device_match() to better
reflect its functionality and not confuse users by its presence in the
absence of the similarly prefixed put() counterpart.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/nvmem/core.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 623aeb9533c0124bb893d0c229570e00cfa3c52a..24e079240ebbb0899d6d91f8147210a64d4414cb 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -1106,7 +1106,7 @@ struct nvmem_device *devm_nvmem_register(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_nvmem_register);
 
-static struct nvmem_device *__nvmem_device_get(void *data,
+static struct nvmem_device *nvmem_device_match(void *data,
 			int (*match)(struct device *dev, const void *data))
 {
 	struct nvmem_device *nvmem = NULL;
@@ -1134,13 +1134,6 @@ static struct nvmem_device *__nvmem_device_get(void *data,
 	return nvmem;
 }
 
-static void __nvmem_device_put(struct nvmem_device *nvmem)
-{
-	put_device(&nvmem->dev);
-	module_put(nvmem->owner);
-	kref_put(&nvmem->refcnt, nvmem_device_release);
-}
-
 #if IS_ENABLED(CONFIG_OF)
 /**
  * of_nvmem_device_get() - Get nvmem device from a given id
@@ -1165,7 +1158,7 @@ struct nvmem_device *of_nvmem_device_get(struct device_node *np, const char *id)
 	if (!nvmem_np)
 		return ERR_PTR(-ENOENT);
 
-	nvmem = __nvmem_device_get(nvmem_np, device_match_of_node);
+	nvmem = nvmem_device_match(nvmem_np, device_match_of_node);
 	of_node_put(nvmem_np);
 	return nvmem;
 }
@@ -1193,7 +1186,7 @@ struct nvmem_device *nvmem_device_get(struct device *dev, const char *dev_name)
 
 	}
 
-	return __nvmem_device_get((void *)dev_name, device_match_name);
+	return nvmem_device_match((void *)dev_name, device_match_name);
 }
 EXPORT_SYMBOL_GPL(nvmem_device_get);
 
@@ -1209,7 +1202,7 @@ EXPORT_SYMBOL_GPL(nvmem_device_get);
 struct nvmem_device *nvmem_device_find(void *data,
 			int (*match)(struct device *dev, const void *data))
 {
-	return __nvmem_device_get(data, match);
+	return nvmem_device_match(data, match);
 }
 EXPORT_SYMBOL_GPL(nvmem_device_find);
 
@@ -1253,7 +1246,9 @@ EXPORT_SYMBOL_GPL(devm_nvmem_device_put);
  */
 void nvmem_device_put(struct nvmem_device *nvmem)
 {
-	__nvmem_device_put(nvmem);
+	put_device(&nvmem->dev);
+	module_put(nvmem->owner);
+	kref_put(&nvmem->refcnt, nvmem_device_release);
 }
 EXPORT_SYMBOL_GPL(nvmem_device_put);
 
@@ -1332,7 +1327,7 @@ nvmem_cell_get_from_lookup(struct device *dev, const char *con_id)
 		if ((strcmp(lookup->dev_id, dev_id) == 0) &&
 		    (strcmp(lookup->con_id, con_id) == 0)) {
 			/* This is the right entry. */
-			nvmem = __nvmem_device_get((void *)lookup->nvmem_name,
+			nvmem = nvmem_device_match((void *)lookup->nvmem_name,
 						   device_match_name);
 			if (IS_ERR(nvmem))
 				/* Provider may not be registered yet. */
@@ -1341,12 +1336,12 @@ nvmem_cell_get_from_lookup(struct device *dev, const char *con_id)
 			cell_entry = nvmem_find_cell_entry_by_name(nvmem,
 								   lookup->cell_name);
 			if (!cell_entry) {
-				__nvmem_device_put(nvmem);
+				nvmem_device_put(nvmem);
 				cell = ERR_PTR(-ENOENT);
 			} else {
 				cell = nvmem_create_cell(cell_entry, con_id, 0);
 				if (IS_ERR(cell))
-					__nvmem_device_put(nvmem);
+					nvmem_device_put(nvmem);
 			}
 			break;
 		}
@@ -1444,7 +1439,7 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, const char *id)
 		}
 	}
 
-	nvmem = __nvmem_device_get(nvmem_np, device_match_of_node);
+	nvmem = nvmem_device_match(nvmem_np, device_match_of_node);
 	of_node_put(nvmem_np);
 	if (IS_ERR(nvmem)) {
 		of_node_put(cell_np);
@@ -1454,7 +1449,7 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, const char *id)
 	ret = nvmem_layout_module_get_optional(nvmem);
 	if (ret) {
 		of_node_put(cell_np);
-		__nvmem_device_put(nvmem);
+		nvmem_device_put(nvmem);
 		return ERR_PTR(ret);
 	}
 
@@ -1463,14 +1458,14 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, const char *id)
 	if (!cell_entry) {
 		nvmem_layout_module_put(nvmem);
 		ret = nvmem->layout ? -EPROBE_DEFER : -ENOENT;
-		__nvmem_device_put(nvmem);
+		nvmem_device_put(nvmem);
 		return ERR_PTR(ret);
 	}
 
 	cell = nvmem_create_cell(cell_entry, id, cell_index);
 	if (IS_ERR(cell)) {
 		nvmem_layout_module_put(nvmem);
-		__nvmem_device_put(nvmem);
+		nvmem_device_put(nvmem);
 	}
 
 	return cell;
@@ -1585,7 +1580,7 @@ void nvmem_cell_put(struct nvmem_cell *cell)
 
 	kfree(cell);
 	nvmem_layout_module_put(nvmem);
-	__nvmem_device_put(nvmem);
+	nvmem_device_put(nvmem);
 }
 EXPORT_SYMBOL_GPL(nvmem_cell_put);
 

-- 
2.47.3

[PATCH v5 6/9] nvmem: split out the reg_read/write() callbacks out of struct nvmem_device

[Bartosz Golaszewski]

The reg_read/write() fields of struct nvmem_device point to memory owned
by the nvmem provider. They must not be dereferenced after the provider
is unregistered. Ahead of protecting against accesses to invalid memory
with SRCU, move the callbacks into a separate structure the address of
which is stored in nvmem_device.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/nvmem/core.c      | 37 +++++++++++++++++++++++++++----------
 drivers/nvmem/internals.h |  9 +++++++--
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 24e079240ebbb0899d6d91f8147210a64d4414cb..b6429aae99e0deef502b1929debc47a09986925d 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -57,25 +57,28 @@ static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
 static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
 			    void *val, size_t bytes)
 {
-	if (!nvmem->reg_read)
+	struct nvmem_operations *ops = nvmem->ops;
+
+	if (!ops->reg_read)
 		return -EOPNOTSUPP;
 
-	return nvmem->reg_read(nvmem->priv, offset, val, bytes);
+	return ops->reg_read(nvmem->priv, offset, val, bytes);
 }
 
 static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
 			     void *val, size_t bytes)
 {
+	struct nvmem_operations *ops = nvmem->ops;
 	int ret, wr_ok;
 
-	if (!nvmem->reg_write)
+	if (!ops->reg_write)
 		return -EOPNOTSUPP;
 
 	ret = gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
 	if (ret)
 		return ret;
 
-	wr_ok = nvmem->reg_write(nvmem->priv, offset, val, bytes);
+	wr_ok = ops->reg_write(nvmem->priv, offset, val, bytes);
 
 	ret = gpiod_set_value_cansleep(nvmem->wp_gpio, 1);
 	if (ret)
@@ -286,6 +289,8 @@ 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_operations *ops = nvmem->ops;
+
 	umode_t mode = 0400;
 
 	if (!nvmem->root_only)
@@ -294,10 +299,10 @@ static umode_t nvmem_bin_attr_get_umode(struct nvmem_device *nvmem)
 	if (!nvmem->read_only)
 		mode |= 0200;
 
-	if (!nvmem->reg_write)
+	if (!ops->reg_write)
 		mode &= ~0200;
 
-	if (!nvmem->reg_read)
+	if (!ops->reg_read)
 		mode &= ~0444;
 
 	return mode;
@@ -328,6 +333,7 @@ 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_operations *ops = nvmem->ops;
 
 	/*
 	 * If the device has no .reg_write operation, do not allow
@@ -336,7 +342,7 @@ static umode_t nvmem_attr_is_visible(struct kobject *kobj,
 	 * can be forced into read-write mode using the 'force_ro'
 	 * attribute.
 	 */
-	if (attr == &dev_attr_force_ro.attr && !nvmem->reg_write)
+	if (attr == &dev_attr_force_ro.attr && !ops->reg_write)
 		return 0;	/* Attribute not visible */
 
 	return attr->mode;
@@ -537,6 +543,7 @@ static void nvmem_release(struct device *dev)
 
 	ida_free(&nvmem_ida, nvmem->id);
 	gpiod_put(nvmem->wp_gpio);
+	kfree(nvmem->ops);
 	kfree(nvmem);
 }
 
@@ -897,6 +904,7 @@ EXPORT_SYMBOL_GPL(nvmem_layout_unregister);
 
 struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 {
+	struct nvmem_operations *ops;
 	struct nvmem_device *nvmem;
 	int rval;
 
@@ -910,8 +918,15 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	if (!nvmem)
 		return ERR_PTR(-ENOMEM);
 
+	ops = kzalloc_obj(*ops);
+	if (!ops) {
+		kfree(nvmem);
+		return ERR_PTR(-ENOMEM);
+	}
+
 	rval = ida_alloc(&nvmem_ida, GFP_KERNEL);
 	if (rval < 0) {
+		kfree(ops);
 		kfree(nvmem);
 		return ERR_PTR(rval);
 	}
@@ -921,6 +936,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem->dev.type = &nvmem_provider_type;
 	nvmem->dev.bus = &nvmem_bus_type;
 	nvmem->dev.parent = config->dev;
+	nvmem->ops = ops;
 
 	device_initialize(&nvmem->dev);
 
@@ -937,6 +953,9 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	INIT_LIST_HEAD(&nvmem->cells);
 	nvmem->fixup_dt_cell_info = config->fixup_dt_cell_info;
 
+	ops->reg_read = config->reg_read;
+	ops->reg_write = config->reg_write;
+
 	nvmem->owner = config->owner;
 	if (!nvmem->owner && config->dev->driver)
 		nvmem->owner = config->dev->driver->owner;
@@ -946,8 +965,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem->root_only = config->root_only;
 	nvmem->priv = config->priv;
 	nvmem->type = config->type;
-	nvmem->reg_read = config->reg_read;
-	nvmem->reg_write = config->reg_write;
 	nvmem->keepout = config->keepout;
 	nvmem->nkeepout = config->nkeepout;
 	if (config->of_node)
@@ -973,7 +990,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 		goto err_put_device;
 
 	nvmem->read_only = device_property_present(config->dev, "read-only") ||
-			   config->read_only || !nvmem->reg_write;
+			   config->read_only || !ops->reg_write;
 
 #ifdef CONFIG_NVMEM_SYSFS
 	nvmem->dev.groups = nvmem_dev_groups;
diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
index 7cbc55f40259fc4315c41979ad8bf75c36bcb056..070eef89ffca78fe033aaaada0f91c257fe2f166 100644
--- a/drivers/nvmem/internals.h
+++ b/drivers/nvmem/internals.h
@@ -7,6 +7,12 @@
 #include <linux/nvmem-consumer.h>
 #include <linux/nvmem-provider.h>
 
+/* Hold pointers to callbacks owned by the nvmem provider module. */
+struct nvmem_operations {
+	nvmem_reg_read_t	reg_read;
+	nvmem_reg_write_t	reg_write;
+};
+
 struct nvmem_device {
 	struct module		*owner;
 	struct device		dev;
@@ -26,10 +32,9 @@ struct nvmem_device {
 				   struct nvmem_cell_info *cell);
 	const struct nvmem_keepout *keepout;
 	unsigned int		nkeepout;
-	nvmem_reg_read_t	reg_read;
-	nvmem_reg_write_t	reg_write;
 	struct gpio_desc	*wp_gpio;
 	struct nvmem_layout	*layout;
+	struct nvmem_operations	*ops;
 	void *priv;
 	bool			sysfs_cells_populated;
 };

-- 
2.47.3

[PATCH v5 7/9] nvmem: simplify nvmem_sysfs_remove_compat()

[Bartosz Golaszewski]

There's no need for the config argument in nvmem_sysfs_remove_compat().
Once the compat attribute is registered, that information is carried in
nvmem_device::flags. Rework the code to always query that field and drop
the second argument.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/nvmem/core.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index b6429aae99e0deef502b1929debc47a09986925d..c59c5f0e8a253eaedc0c6b180c7273a6c53ee1ac 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -460,10 +460,9 @@ static int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
 	return 0;
 }
 
-static void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
-			      const struct nvmem_config *config)
+static void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem)
 {
-	if (config->compat)
+	if (nvmem->flags & FLAG_COMPAT)
 		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
 }
 
@@ -530,8 +529,7 @@ static int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
 {
 	return -ENOSYS;
 }
-static void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
-				      const struct nvmem_config *config)
+static void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem)
 {
 }
 
@@ -1052,8 +1050,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	device_del(&nvmem->dev);
 err_remove_cells:
 	nvmem_device_remove_all_cells(nvmem);
-	if (config->compat)
-		nvmem_sysfs_remove_compat(nvmem, config);
+	nvmem_sysfs_remove_compat(nvmem);
 err_put_device:
 	put_device(&nvmem->dev);
 
@@ -1069,8 +1066,7 @@ static void nvmem_device_release(struct kref *kref)
 
 	blocking_notifier_call_chain(&nvmem_notifier, NVMEM_REMOVE, nvmem);
 
-	if (nvmem->flags & FLAG_COMPAT)
-		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
+	nvmem_sysfs_remove_compat(nvmem);
 
 	nvmem_device_remove_all_cells(nvmem);
 	nvmem_destroy_layout(nvmem);

-- 
2.47.3

[PATCH v5 8/9] nvmem: remove duplicated reference counting

[Bartosz Golaszewski]

Commit c1de7f43bd84 ("nvmem: use kref") introduced reference counting
with kref to an already reference counted nvmem_device structure. We
only need one refcount so use the one provded by device's kobject and
drop the kref field from struct nvmem_device.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/nvmem/core.c      | 82 ++++++++++++++++++++---------------------------
 drivers/nvmem/internals.h |  1 -
 2 files changed, 35 insertions(+), 48 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index c59c5f0e8a253eaedc0c6b180c7273a6c53ee1ac..2c6d058a20d97e039f42e45c7b8d4305344c4ba0 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -535,24 +535,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->ops);
-	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);
@@ -571,6 +553,25 @@ 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);
+
+	gpiod_put(nvmem->wp_gpio);
+	nvmem_device_remove_all_cells(nvmem);
+	ida_free(&nvmem_ida, nvmem->id);
+	kfree(nvmem->ops);
+	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)
@@ -934,6 +935,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem->dev.type = &nvmem_provider_type;
 	nvmem->dev.bus = &nvmem_bus_type;
 	nvmem->dev.parent = config->dev;
+	INIT_LIST_HEAD(&nvmem->cells);
 	nvmem->ops = ops;
 
 	device_initialize(&nvmem->dev);
@@ -947,8 +949,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;
 
 	ops->reg_read = config->reg_read;
@@ -1009,24 +1009,24 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	if (config->cells) {
 		rval = nvmem_add_cells(nvmem, config->cells, config->ncells);
 		if (rval)
-			goto err_remove_cells;
+			goto err_remove_compat;
 	}
 
 	if (config->add_legacy_fixed_of_cells) {
 		rval = nvmem_add_cells_from_legacy_of(nvmem);
 		if (rval)
-			goto err_remove_cells;
+			goto err_remove_compat;
 	}
 
 	rval = nvmem_add_cells_from_fixed_layout(nvmem);
 	if (rval)
-		goto err_remove_cells;
+		goto err_remove_compat;
 
 	dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
 
 	rval = device_add(&nvmem->dev);
 	if (rval)
-		goto err_remove_cells;
+		goto err_remove_compat;
 
 	rval = nvmem_populate_layout(nvmem);
 	if (rval)
@@ -1048,8 +1048,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 #endif
 err_remove_dev:
 	device_del(&nvmem->dev);
-err_remove_cells:
-	nvmem_device_remove_all_cells(nvmem);
+err_remove_compat:
 	nvmem_sysfs_remove_compat(nvmem);
 err_put_device:
 	put_device(&nvmem->dev);
@@ -1058,21 +1057,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 }
 EXPORT_SYMBOL_GPL(nvmem_register);
 
-static void nvmem_device_release(struct kref *kref)
-{
-	struct nvmem_device *nvmem;
-
-	nvmem = container_of(kref, struct nvmem_device, refcnt);
-
-	blocking_notifier_call_chain(&nvmem_notifier, NVMEM_REMOVE, nvmem);
-
-	nvmem_sysfs_remove_compat(nvmem);
-
-	nvmem_device_remove_all_cells(nvmem);
-	nvmem_destroy_layout(nvmem);
-	device_unregister(&nvmem->dev);
-}
-
 /**
  * nvmem_unregister() - Unregister previously registered nvmem device
  *
@@ -1080,8 +1064,15 @@ static void nvmem_device_release(struct kref *kref)
  */
 void nvmem_unregister(struct nvmem_device *nvmem)
 {
-	if (nvmem)
-		kref_put(&nvmem->refcnt, nvmem_device_release);
+	if (!nvmem)
+		return;
+
+	blocking_notifier_call_chain(&nvmem_notifier, NVMEM_REMOVE, nvmem);
+
+	nvmem_sysfs_remove_compat(nvmem);
+	nvmem_destroy_layout(nvmem);
+
+	device_unregister(&nvmem->dev);
 }
 EXPORT_SYMBOL_GPL(nvmem_unregister);
 
@@ -1142,8 +1133,6 @@ static struct nvmem_device *nvmem_device_match(void *data,
 		return ERR_PTR(-EINVAL);
 	}
 
-	kref_get(&nvmem->refcnt);
-
 	return nvmem;
 }
 
@@ -1259,9 +1248,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);
 
diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
index 070eef89ffca78fe033aaaada0f91c257fe2f166..60daa79c404dc915872481bd3b02de2258d33406 100644
--- a/drivers/nvmem/internals.h
+++ b/drivers/nvmem/internals.h
@@ -19,7 +19,6 @@ struct nvmem_device {
 	int			stride;
 	int			word_size;
 	int			id;
-	struct kref		refcnt;
 	size_t			size;
 	bool			read_only;
 	bool			root_only;

-- 
2.47.3

[PATCH v5 9/9] nvmem: protect nvmem_device::ops with SRCU

[Bartosz Golaszewski]

With the provider-owned data split out into a separate 'ops' structure,
we can now protect it with SRCU.

Protect all dereferences of nvmem->ops 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.

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@oss.qualcomm.com>
---
 drivers/nvmem/core.c      | 44 ++++++++++++++++++++++++++++++++++++++------
 drivers/nvmem/internals.h |  4 +++-
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 2c6d058a20d97e039f42e45c7b8d4305344c4ba0..2e66d4ae4aedca36662171abacec02fadd5e9039 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -57,7 +57,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_operations *ops = nvmem->ops;
+	struct nvmem_operations *ops;
+
+	guard(srcu)(&nvmem->srcu);
+	ops = srcu_dereference(nvmem->ops, &nvmem->srcu);
+	if (!ops)
+		return -ENODEV;
 
 	if (!ops->reg_read)
 		return -EOPNOTSUPP;
@@ -68,9 +73,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_operations *ops = nvmem->ops;
+	struct nvmem_operations *ops;
 	int ret, wr_ok;
 
+	guard(srcu)(&nvmem->srcu);
+	ops = srcu_dereference(nvmem->ops, &nvmem->srcu);
+	if (!ops)
+		return -ENODEV;
+
 	if (!ops->reg_write)
 		return -EOPNOTSUPP;
 
@@ -289,7 +299,7 @@ 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_operations *ops = nvmem->ops;
+	struct nvmem_operations *ops = rcu_dereference_raw(nvmem->ops);
 
 	umode_t mode = 0400;
 
@@ -333,7 +343,7 @@ 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_operations *ops = nvmem->ops;
+	struct nvmem_operations *ops = rcu_dereference_raw(nvmem->ops);
 
 	/*
 	 * If the device has no .reg_write operation, do not allow
@@ -560,7 +570,7 @@ static void nvmem_release(struct device *dev)
 	gpiod_put(nvmem->wp_gpio);
 	nvmem_device_remove_all_cells(nvmem);
 	ida_free(&nvmem_ida, nvmem->id);
-	kfree(nvmem->ops);
+	cleanup_srcu_struct(&nvmem->srcu);
 	kfree(nvmem);
 }
 
@@ -936,7 +946,20 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem->dev.bus = &nvmem_bus_type;
 	nvmem->dev.parent = config->dev;
 	INIT_LIST_HEAD(&nvmem->cells);
-	nvmem->ops = ops;
+
+	/*
+	 * Must happen before we assign the release() callback in
+	 * device_initialize().
+	 */
+	rval = init_srcu_struct(&nvmem->srcu);
+	if (rval) {
+		ida_free(&nvmem_ida, nvmem->id);
+		kfree(ops);
+		kfree(nvmem);
+		return ERR_PTR(rval);
+	}
+
+	rcu_assign_pointer(nvmem->ops, ops);
 
 	device_initialize(&nvmem->dev);
 
@@ -1051,7 +1074,10 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 err_remove_compat:
 	nvmem_sysfs_remove_compat(nvmem);
 err_put_device:
+	ops = rcu_replace_pointer(nvmem->ops, NULL, true);
+	synchronize_srcu(&nvmem->srcu);
 	put_device(&nvmem->dev);
+	kfree(ops);
 
 	return ERR_PTR(rval);
 }
@@ -1064,13 +1090,19 @@ EXPORT_SYMBOL_GPL(nvmem_register);
  */
 void nvmem_unregister(struct nvmem_device *nvmem)
 {
+	struct nvmem_operations *ops;
+
 	if (!nvmem)
 		return;
 
 	blocking_notifier_call_chain(&nvmem_notifier, NVMEM_REMOVE, nvmem);
 
+	ops = rcu_replace_pointer(nvmem->ops, NULL, true);
+	synchronize_srcu(&nvmem->srcu);
+
 	nvmem_sysfs_remove_compat(nvmem);
 	nvmem_destroy_layout(nvmem);
+	kfree(ops);
 
 	device_unregister(&nvmem->dev);
 }
diff --git a/drivers/nvmem/internals.h b/drivers/nvmem/internals.h
index 60daa79c404dc915872481bd3b02de2258d33406..17418fd0dcc92b101b56082fc15d74042386107d 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>
 
 /* Hold pointers to callbacks owned by the nvmem provider module. */
 struct nvmem_operations {
@@ -16,6 +17,7 @@ struct nvmem_operations {
 struct nvmem_device {
 	struct module		*owner;
 	struct device		dev;
+	struct srcu_struct	srcu;
 	int			stride;
 	int			word_size;
 	int			id;
@@ -33,7 +35,7 @@ struct nvmem_device {
 	unsigned int		nkeepout;
 	struct gpio_desc	*wp_gpio;
 	struct nvmem_layout	*layout;
-	struct nvmem_operations	*ops;
+	struct nvmem_operations __rcu *ops;
 	void *priv;
 	bool			sysfs_cells_populated;
 };

-- 
2.47.3

Re: [PATCH v5 0/9] nvmem: rework nvmem core and allow unbinding with active consumers

[Loic Poulain]

On Mon, Jun 29, 2026 at 11:48 AM Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:
>
> Sashiko pointed out some issues so this iteration fixes them. I'm also
> Cc'ing Loic who seems to have encountered the issue of unbinding with
> active consumers when working on the block nvmem provider.
>
> Nvmem is one of the subsystems vulnerable to object life-time issues.
> The memory nvmem core dereferences is owned by nvmem providers which can
> be unbound at any time and even though nvmem devices themselves are
> reference-counted, there's no synchronization with the provider modules.
>
> This typically is not a problem because thanks to fw_devlink, consumers
> get synchronously unbound before providers but it's enough to pass
> fw_devlink=off over the command line, unbind the nvmem controller with
> consumers still holding references to it and try to read/write in order
> to see fireworks in the kernel log.
>
> User-space can trigger it too if a device (for instance: i2c eeprom on a
> cp2112 USB expander) is unplugged halfway through a long read.
>
> This series proposes to use SRCU to protect nvmem against accessing
> invalid memory after unbinding with active consumers and also reworks
> several places in nvmem core.

Tested the series on Arduino-Q on top of the NVMEM block series + a
couple of binding/unbinding.

Tested-by: Loic Poulain <loic.poulain@oss.qualcomm.com>


>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> Changes in v5:
> - Rebase on top of v7.2-rc1
> - Drop applied patch from the series
> - Link to v4: https://patch.msgid.link/20260521-nvmem-unbind-v4-0-7fa136759491@oss.qualcomm.com
>
> Changes in v4:
> - Restore the removed checks for the existence of reg_write/reg_read
>   ops in sysfs callbacks as the attributes may be created with only a
>   single operation available
> - Fix potential use-after-free when decrementing the references to nvmem
>   device
> - Rename some local variables to better indicate their function
> - Initialize the cell list before calling device_initialize() as we
>   iterate over it in release path unconditionally
> - Restore the nvmem != NULL check in nvmem_unregister() as sashiko
>   pointed out there are users who rely on this API contract
> - Don't use rcu_dereference() with SRCU as it may trigger a
>   false-positive lockdep alert
> - Synchronize the removal of nvmem->ops in error path in
>   nvmem_register() as it's possible for it to be made available to the
>   system before a subsequent failure later in the function
> - Link to v3: https://patch.msgid.link/20260429-nvmem-unbind-v3-0-2a694f95395b@oss.qualcomm.com
>
> Changes in v3:
> - Add Fixes tag to patch 1
> - Don't check the presence of read/write callbacks in sysfs attributes
>   as these are not visible without them
> - Rework mutex guards and drop unneeded helper variables
> - Fix mutex guard conversion: it accidentally converted nvmem_lookup_mutex
>   locks to nvmem_mutex
> - Extend patch 5 to also rename __nvmem_device_get() to
>   nvmem_device_match()
> - Call nvmem_sysfs_remove_compat() on unregister, not release
> - Split patch 7 into two: one removing the redundant kref and second
>   adding SRCU
> - Link to v2: https://patch.msgid.link/20260223-nvmem-unbind-v2-0-0df33a933dca@oss.qualcomm.com
>
> Changes in v2:
> - add missing SRCU struct cleanup
> - improve the teardown path on error in nvmem_register()
> - Link to v1: https://lore.kernel.org/r/20260116-nvmem-unbind-v1-0-7bb401ab19a8@oss.qualcomm.com
>
> ---
> Bartosz Golaszewski (9):
>       nvmem: remove unused field from struct nvmem_device
>       nvmem: return -EOPNOTSUPP to in-kernel users on missing callbacks
>       nvmem: check the return value of gpiod_set_value_cansleep()
>       nvmem: simplify locking with guard()
>       nvmem: remove unneeded __nvmem_device_put()
>       nvmem: split out the reg_read/write() callbacks out of struct nvmem_device
>       nvmem: simplify nvmem_sysfs_remove_compat()
>       nvmem: remove duplicated reference counting
>       nvmem: protect nvmem_device::ops with SRCU
>
>  drivers/nvmem/core.c      | 313 +++++++++++++++++++++++++---------------------
>  drivers/nvmem/internals.h |  13 +-
>  2 files changed, 176 insertions(+), 150 deletions(-)
> ---
> base-commit: 04e3a00b874c0e91494097bc986ccb74e953e757
> change-id: 20260114-nvmem-unbind-673b52fc84a0
>
> Best regards,
> --
> Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>

Re: [PATCH v5 0/9] nvmem: rework nvmem core and allow unbinding with active consumers

[Srinivas Kandagatla]

On Mon, 29 Jun 2026 11:47:45 +0200, Bartosz Golaszewski wrote:
> Sashiko pointed out some issues so this iteration fixes them. I'm also
> Cc'ing Loic who seems to have encountered the issue of unbinding with
> active consumers when working on the block nvmem provider.
> 
> Nvmem is one of the subsystems vulnerable to object life-time issues.
> The memory nvmem core dereferences is owned by nvmem providers which can
> be unbound at any time and even though nvmem devices themselves are
> reference-counted, there's no synchronization with the provider modules.
> 
> [...]

Applied, thanks!

[1/9] nvmem: remove unused field from struct nvmem_device
      commit: 5409a299432b54f33fc47cb182df5e8b8166031c
[2/9] nvmem: return -EOPNOTSUPP to in-kernel users on missing callbacks
      commit: c6c4594fc8c139a533a0e0b05727009911b06157
[3/9] nvmem: check the return value of gpiod_set_value_cansleep()
      commit: b25b0a566a91d2a3a38a27329b7d6034a08ad9df
[4/9] nvmem: simplify locking with guard()
      commit: 376bc6403ac53e833014b4e834465ea5aec15de0
[5/9] nvmem: remove unneeded __nvmem_device_put()
      commit: fb564b9150b4271b6de24a7b38c320c538c7c9f5
[6/9] nvmem: split out the reg_read/write() callbacks out of struct nvmem_device
      commit: 15dcb6cc98a5c5011124b0dc62869ed6609381b8
[7/9] nvmem: simplify nvmem_sysfs_remove_compat()
      commit: 9e81282a54bbc091c9e98d7eb3b1b5c247295dc0
[8/9] nvmem: remove duplicated reference counting
      commit: 0c0e7756ec1b5a1b76c9b196508b421881b77c3f
[9/9] nvmem: protect nvmem_device::ops with SRCU
      commit: ea87821719eea4cf729e237e5cc417172f8b58e3

Best regards,
-- 
Srinivas Kandagatla <srini@kernel.org>