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>] [day] [month] [year] [list]
Message-Id: <20241214215759.60811-1-gerhard@engleder-embedded.com>
Date: Sat, 14 Dec 2024 22:57:59 +0100
From: Gerhard Engleder <gerhard@...leder-embedded.com>
To: linux-kernel@...r.kernel.org
Cc: arnd@...db.de,
	gregkh@...uxfoundation.org,
	Gerhard Engleder <gerhard@...leder-embedded.com>,
	Gerhard Engleder <eg@...a.com>
Subject: [PATCH] misc: keba: Fix kernfs warning on module unload

Unloading the cp500 module leads to the following warning:

kernfs: can not remove 'eeprom', no directory
WARNING: CPU: 1 PID: 1610 at fs/kernfs/dir.c:1683 kernfs_remove_by_name_ns+0xb1/0xc0

The parent I2C device of the nvmem devices is freed before the nvmem
devices. The reference to the nvmem devices is put by devm after
cp500_remove(), but at this time the parent I2C device does not exist
anymore as the I2C controller and its devices have already been freed in
cp500_remove(). Thus, nvmem tries to remove an entry from an already
deleted directory.

Free nvmem devices before I2C controller auxiliary device.

Signed-off-by: Gerhard Engleder <eg@...a.com>
---
 drivers/misc/keba/cp500.c | 69 +++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 24 deletions(-)

diff --git a/drivers/misc/keba/cp500.c b/drivers/misc/keba/cp500.c
index 255d3022dae8..d0c6113dcff3 100644
--- a/drivers/misc/keba/cp500.c
+++ b/drivers/misc/keba/cp500.c
@@ -126,8 +126,9 @@ static struct cp500_devs cp520_devices = {
 };
 
 struct cp500_nvmem {
-	struct nvmem_device *nvmem;
+	struct nvmem_device *base_nvmem;
 	unsigned int offset;
+	struct nvmem_device *nvmem;
 };
 
 struct cp500 {
@@ -581,8 +582,8 @@ static int cp500_nvmem_read(void *priv, unsigned int offset, void *val,
 	struct cp500_nvmem *nvmem = priv;
 	int ret;
 
-	ret = nvmem_device_read(nvmem->nvmem, nvmem->offset + offset, bytes,
-				val);
+	ret = nvmem_device_read(nvmem->base_nvmem, nvmem->offset + offset,
+				bytes, val);
 	if (ret != bytes)
 		return ret;
 
@@ -595,15 +596,16 @@ static int cp500_nvmem_write(void *priv, unsigned int offset, void *val,
 	struct cp500_nvmem *nvmem = priv;
 	int ret;
 
-	ret = nvmem_device_write(nvmem->nvmem, nvmem->offset + offset, bytes,
-				 val);
+	ret = nvmem_device_write(nvmem->base_nvmem, nvmem->offset + offset,
+				 bytes, val);
 	if (ret != bytes)
 		return ret;
 
 	return 0;
 }
 
-static int cp500_nvmem_register(struct cp500 *cp500, struct nvmem_device *nvmem)
+static int cp500_nvmem_register(struct cp500 *cp500,
+				struct nvmem_device *base_nvmem)
 {
 	struct device *dev = &cp500->pci_dev->dev;
 	struct nvmem_config nvmem_config = {};
@@ -625,27 +627,52 @@ static int cp500_nvmem_register(struct cp500 *cp500, struct nvmem_device *nvmem)
 	nvmem_config.reg_read = cp500_nvmem_read;
 	nvmem_config.reg_write = cp500_nvmem_write;
 
-	cp500->nvmem_cpu.nvmem = nvmem;
+	cp500->nvmem_cpu.base_nvmem = base_nvmem;
 	cp500->nvmem_cpu.offset = CP500_EEPROM_CPU_OFFSET;
 	nvmem_config.name = CP500_EEPROM_CPU_NAME;
 	nvmem_config.size = CP500_EEPROM_CPU_SIZE;
 	nvmem_config.priv = &cp500->nvmem_cpu;
-	tmp = devm_nvmem_register(dev, &nvmem_config);
+	tmp = nvmem_register(&nvmem_config);
 	if (IS_ERR(tmp))
 		return PTR_ERR(tmp);
+	cp500->nvmem_cpu.nvmem = tmp;
 
-	cp500->nvmem_user.nvmem = nvmem;
+	cp500->nvmem_user.base_nvmem = base_nvmem;
 	cp500->nvmem_user.offset = CP500_EEPROM_USER_OFFSET;
 	nvmem_config.name = CP500_EEPROM_USER_NAME;
 	nvmem_config.size = CP500_EEPROM_USER_SIZE;
 	nvmem_config.priv = &cp500->nvmem_user;
-	tmp = devm_nvmem_register(dev, &nvmem_config);
-	if (IS_ERR(tmp))
+	tmp = nvmem_register(&nvmem_config);
+	if (IS_ERR(tmp)) {
+		nvmem_unregister(cp500->nvmem_cpu.nvmem);
+		cp500->nvmem_cpu.nvmem = NULL;
+
 		return PTR_ERR(tmp);
+	}
+	cp500->nvmem_user.nvmem = tmp;
 
 	return 0;
 }
 
+static void cp500_nvmem_unregister(struct cp500 *cp500)
+{
+	int notified;
+
+	if (cp500->nvmem_user.nvmem) {
+		nvmem_unregister(cp500->nvmem_user.nvmem);
+		cp500->nvmem_user.nvmem = NULL;
+	}
+	if (cp500->nvmem_cpu.nvmem) {
+		nvmem_unregister(cp500->nvmem_cpu.nvmem);
+		cp500->nvmem_cpu.nvmem = NULL;
+	}
+
+	/* CPU and user nvmem use the same base_nvmem, put only once */
+	notified = atomic_read(&cp500->nvmem_notified);
+	if (notified)
+		nvmem_device_put(cp500->nvmem_cpu.base_nvmem);
+}
+
 static int cp500_nvmem_match(struct device *dev, const void *data)
 {
 	const struct cp500 *cp500 = data;
@@ -663,13 +690,6 @@ static int cp500_nvmem_match(struct device *dev, const void *data)
 	return 0;
 }
 
-static void cp500_devm_nvmem_put(void *data)
-{
-	struct nvmem_device *nvmem = data;
-
-	nvmem_device_put(nvmem);
-}
-
 static int cp500_nvmem(struct notifier_block *nb, unsigned long action,
 		       void *data)
 {
@@ -698,10 +718,6 @@ static int cp500_nvmem(struct notifier_block *nb, unsigned long action,
 		return NOTIFY_DONE;
 	}
 
-	ret = devm_add_action_or_reset(dev, cp500_devm_nvmem_put, nvmem);
-	if (ret)
-		return ret;
-
 	ret = cp500_nvmem_register(cp500, nvmem);
 	if (ret)
 		return ret;
@@ -932,12 +948,17 @@ static void cp500_remove(struct pci_dev *pci_dev)
 {
 	struct cp500 *cp500 = pci_get_drvdata(pci_dev);
 
+	/*
+	 * unregister CPU and user nvmem and put base_nvmem before parent
+	 * auxiliary device of base_nvmem is unregistered
+	 */
+	nvmem_unregister_notifier(&cp500->nvmem_notifier);
+	cp500_nvmem_unregister(cp500);
+
 	cp500_unregister_auxiliary_devs(cp500);
 
 	cp500_disable(cp500);
 
-	nvmem_unregister_notifier(&cp500->nvmem_notifier);
-
 	pci_set_drvdata(pci_dev, 0);
 
 	pci_free_irq_vectors(pci_dev);
-- 
2.39.2


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ