[PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers

mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed

* [PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers
@ 2026-09-25  9:52 Bartosz Golaszewski
  2026-09-25  9:52 ` [PATCH v6 1/2] nvmem: remove duplicated reference counting Bartosz Golaszewski
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Bartosz Golaszewski @ 2026-09-25  9:52 UTC (permalink / raw)
  To: Srinivas Kandagatla, Bartosz Golaszewski, Johan Hovold, Loic Poulain
  Cc: linux-kernel, brgl, Bartosz Golaszewski

This series was queued by Srini for v7.3[1], sent out to Greg[2] but
last two patches didn't apply[3] and subsequently slipped through the
cracks and never got into mainline. This is just a resend of the two
patches missing from mainline rebased on top of current next.

--

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.

[1] https://lore.kernel.org/all/178354240050.448408.12632228727228648876.b4-ty@kernel.org/
[2] https://lore.kernel.org/all/20260729094647.111468-1-srini@kernel.org/
[3] https://lore.kernel.org/all/2026073105-replace-depose-42d3@gregkh/

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Changes in v6:
- Drop patches applied upstream
- Rebase on top of current linux-next
- Link to v5: https://patch.msgid.link/20260629-nvmem-unbind-v5-0-233212f241c4@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 (2):
      nvmem: remove duplicated reference counting
      nvmem: protect nvmem_device::ops with SRCU

 drivers/nvmem/core.c      | 120 +++++++++++++++++++++++++++-------------------
 drivers/nvmem/internals.h |   5 +-
 2 files changed, 73 insertions(+), 52 deletions(-)
---
base-commit: 9578843eb42e311b93e3ef35e732f237c8cc2592
change-id: 20260114-nvmem-unbind-673b52fc84a0

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v6 1/2] nvmem: remove duplicated reference counting
  2026-09-25  9:52 [PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers Bartosz Golaszewski
@ 2026-09-25  9:52 ` Bartosz Golaszewski
  2026-09-25  9:52 ` [PATCH v6 2/2] nvmem: protect nvmem_device::ops with SRCU Bartosz Golaszewski
  2026-09-25 10:52 ` [PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers Srinivas Kandagatla
  2 siblings, 0 replies; 4+ messages in thread
From: Bartosz Golaszewski @ 2026-09-25  9:52 UTC (permalink / raw)
  To: Srinivas Kandagatla, Bartosz Golaszewski, Johan Hovold, Loic Poulain
  Cc: linux-kernel, brgl, 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      | 80 ++++++++++++++++++++---------------------------
 drivers/nvmem/internals.h |  1 -
 2 files changed, 34 insertions(+), 47 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index f3024e5a5c5ffae08d34c1155f7e2e31fa8a6fdb..2398d33bdd12eb7d6cac185a49fa6a79da13b1c5 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)
@@ -918,6 +919,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);
@@ -931,8 +933,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;
@@ -993,20 +993,20 @@ 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;
 	}
 
 	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)
@@ -1032,8 +1032,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);
@@ -1042,21 +1041,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
  *
@@ -1064,8 +1048,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);
 
@@ -1126,8 +1117,6 @@ static struct nvmem_device *nvmem_device_match(void *data,
 		return ERR_PTR(-EINVAL);
 	}
 
-	kref_get(&nvmem->refcnt);
-
 	return nvmem;
 }
 
@@ -1243,9 +1232,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 4e610deeaa7b3ca41404d52b893b1d27b962e985..2c3645a2727270298139dd7b49b81fa0bda58ad5 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v6 2/2] nvmem: protect nvmem_device::ops with SRCU
  2026-09-25  9:52 [PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers Bartosz Golaszewski
  2026-09-25  9:52 ` [PATCH v6 1/2] nvmem: remove duplicated reference counting Bartosz Golaszewski
@ 2026-09-25  9:52 ` Bartosz Golaszewski
  2026-09-25 10:52 ` [PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers Srinivas Kandagatla
  2 siblings, 0 replies; 4+ messages in thread
From: Bartosz Golaszewski @ 2026-09-25  9:52 UTC (permalink / raw)
  To: Srinivas Kandagatla, Bartosz Golaszewski, Johan Hovold, Loic Poulain
  Cc: linux-kernel, brgl, 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 2398d33bdd12eb7d6cac185a49fa6a79da13b1c5..7d4cce8e27424a032fff59a1189847adb1898168 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);
 }
 
@@ -920,7 +930,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);
 
@@ -1035,7 +1058,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);
 }
@@ -1048,13 +1074,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 2c3645a2727270298139dd7b49b81fa0bda58ad5..bc7a99f5aefbffced60d275bec888ac867fd9032 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers
  2026-09-25  9:52 [PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers Bartosz Golaszewski
  2026-09-25  9:52 ` [PATCH v6 1/2] nvmem: remove duplicated reference counting Bartosz Golaszewski
  2026-09-25  9:52 ` [PATCH v6 2/2] nvmem: protect nvmem_device::ops with SRCU Bartosz Golaszewski
@ 2026-09-25 10:52 ` Srinivas Kandagatla
  2 siblings, 0 replies; 4+ messages in thread
From: Srinivas Kandagatla @ 2026-09-25 10:52 UTC (permalink / raw)
  To: Bartosz Golaszewski, Johan Hovold, Loic Poulain, Bartosz Golaszewski
  Cc: linux-kernel, brgl


On Fri, 25 Sep 2026 11:52:15 +0200, Bartosz Golaszewski wrote:
> This series was queued by Srini for v7.3[1], sent out to Greg[2] but
> last two patches didn't apply[3] and subsequently slipped through the
> cracks and never got into mainline. This is just a resend of the two
> patches missing from mainline rebased on top of current next.
> 
> --
> 
> [...]

Applied, thanks!

[1/2] nvmem: remove duplicated reference counting
      commit: f2928a7103008f27fff6340c161817faffa7d390
[2/2] nvmem: protect nvmem_device::ops with SRCU
      commit: d30804cbeea7ebb6277badc8d6210af5383ac1a4

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-25 10:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-25  9:52 [PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers Bartosz Golaszewski
2026-09-25  9:52 ` [PATCH v6 1/2] nvmem: remove duplicated reference counting Bartosz Golaszewski
2026-09-25  9:52 ` [PATCH v6 2/2] nvmem: protect nvmem_device::ops with SRCU Bartosz Golaszewski
2026-09-25 10:52 ` [PATCH v6 0/2] nvmem: rework nvmem core and allow unbinding with active consumers Srinivas Kandagatla

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®