All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] platform/x86: dell-smbios-wmi: Fixes and cleanups
@ 2026-07-16  5:33 Armin Wolf
  2026-07-16  5:33 ` [PATCH 1/3] platform/x86: dell-smbios-wmi: Fix chardev resource management Armin Wolf
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Armin Wolf @ 2026-07-16  5:33 UTC (permalink / raw)
  To: ilpo.jarvinen, hansg
  Cc: Dell.Client.Kernel, shuangpeng.kernel, platform-driver-x86,
	linux-kernel, gregkh, arnd

This patch series primarily deals with a potential UAF inside
the dell-smbios-wmi driver reported by a user. The first patch
fixes this issue, with the remaining two patches further
cleaning up the resource management.

All patches have been tested on a Dell Inspiron 3505 and appear
to work. I also verified that the UAF issue does not occur anymore
after applying the first patch.

Armin Wolf (3):
  platform/x86: dell-smbios-wmi: Fix chardev resource management
  platform/x86: dell-smbios: Pass device to callbacks
  platform/x86: dell-smbios-wmi: Replace global list with single item

 drivers/platform/x86/dell/dell-smbios-base.c |   6 +-
 drivers/platform/x86/dell/dell-smbios-smm.c  |   8 +-
 drivers/platform/x86/dell/dell-smbios-wmi.c  | 101 +++++++++----------
 drivers/platform/x86/dell/dell-smbios.h      |   4 +-
 4 files changed, 55 insertions(+), 64 deletions(-)

-- 
2.39.5


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

* [PATCH 1/3] platform/x86: dell-smbios-wmi: Fix chardev resource management
  2026-07-16  5:33 [PATCH 0/3] platform/x86: dell-smbios-wmi: Fixes and cleanups Armin Wolf
@ 2026-07-16  5:33 ` Armin Wolf
  2026-07-20 10:08   ` Ilpo Järvinen
  2026-07-16  5:33 ` [PATCH 2/3] platform/x86: dell-smbios: Pass device to callbacks Armin Wolf
  2026-07-16  5:33 ` [PATCH 3/3] platform/x86: dell-smbios-wmi: Replace global list with single item Armin Wolf
  2 siblings, 1 reply; 6+ messages in thread
From: Armin Wolf @ 2026-07-16  5:33 UTC (permalink / raw)
  To: ilpo.jarvinen, hansg
  Cc: Dell.Client.Kernel, shuangpeng.kernel, platform-driver-x86,
	linux-kernel, gregkh, arnd

When unbinding the WMI driver while a userspace application has
an open file descriptor for the character device, a UAF occurs:

KASAN: slab-use-after-free in _copy_to_user from platform/x86/dell-smbios-wmi

The reason for this is that even after calling misc_deregister(),
userspace appications can still call read() and/or ioctl() on open
file descriptors associated with the already unregistered character
device. This causes a UAF by attempting to access the already freed
state container of the WMI driver.

Fix this by no longer storing the state container inside
filp->private_data. Instead retrieve the state container using
get_first_smbios_priv() and return -ENODEV if the state container
does not exist anymore.

Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/178144969601.60470.13396800403157907003@gmail.com/
Fixes: 93885e85a77f ("platform/x86: dell-smbios-wmi: Stop using WMI chardev")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-smbios-wmi.c | 66 ++++++++++-----------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
index 64d0871b706e..c8a6e711283d 100644
--- a/drivers/platform/x86/dell/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
@@ -6,6 +6,7 @@
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
 #include <linux/fs.h>
@@ -13,14 +14,14 @@
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/rwsem.h>
 #include <linux/uaccess.h>
 #include <linux/wmi.h>
 #include <uapi/linux/wmi.h>
 #include "dell-smbios.h"
 #include "dell-wmi-descriptor.h"
 
-static DEFINE_MUTEX(call_mutex);
-static DEFINE_MUTEX(list_mutex);
+static DECLARE_RWSEM(list_lock);
 static int wmi_supported;
 
 struct misc_bios_flags_structure {
@@ -32,6 +33,7 @@ struct misc_bios_flags_structure {
 #define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
 
 struct wmi_smbios_priv {
+	struct mutex call_lock; /* Protects the content of the SMBIOS buffer */
 	struct dell_wmi_smbios_buffer *buf;
 	struct list_head list;
 	struct wmi_device *wdev;
@@ -87,40 +89,35 @@ static int dell_smbios_wmi_call(struct calling_interface_buffer *buffer)
 	size_t size;
 	int ret;
 
-	mutex_lock(&call_mutex);
+	guard(rwsem_read)(&list_lock);
+
 	priv = get_first_smbios_priv();
-	if (!priv) {
-		ret = -ENODEV;
-		goto out_wmi_call;
-	}
+	if (!priv)
+		return -ENODEV;
 
 	size = sizeof(struct calling_interface_buffer);
 	difference = priv->req_buf_size - sizeof(u64) - size;
 
+	guard(mutex)(&priv->call_lock);
+
 	memset(&priv->buf->ext, 0, difference);
 	memcpy(&priv->buf->std, buffer, size);
 	ret = run_smbios_call(priv->wdev);
 	memcpy(buffer, &priv->buf->std, size);
-out_wmi_call:
-	mutex_unlock(&call_mutex);
 
 	return ret;
 }
 
-static int dell_smbios_wmi_open(struct inode *inode, struct file *filp)
+static ssize_t dell_smbios_wmi_read(struct file *filp, char __user *buffer, size_t length,
+				    loff_t *offset)
 {
 	struct wmi_smbios_priv *priv;
 
-	priv = container_of(filp->private_data, struct wmi_smbios_priv, char_dev);
-	filp->private_data = priv;
+	guard(rwsem_read)(&list_lock);
 
-	return nonseekable_open(inode, filp);
-}
-
-static ssize_t dell_smbios_wmi_read(struct file *filp, char __user *buffer, size_t length,
-				    loff_t *offset)
-{
-	struct wmi_smbios_priv *priv = filp->private_data;
+	priv = get_first_smbios_priv();
+	if (!priv)
+		return -ENODEV;
 
 	return simple_read_from_buffer(buffer, length, offset, &priv->req_buf_size,
 				       sizeof(priv->req_buf_size));
@@ -167,22 +164,24 @@ static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
 static long dell_smbios_wmi_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct dell_wmi_smbios_buffer __user *input = (struct dell_wmi_smbios_buffer __user *)arg;
-	struct wmi_smbios_priv *priv = filp->private_data;
-	long ret;
+	struct wmi_smbios_priv *priv;
 
 	if (cmd != DELL_WMI_SMBIOS_CMD)
 		return -ENOIOCTLCMD;
 
-	mutex_lock(&call_mutex);
-	ret = dell_smbios_wmi_do_ioctl(priv, input);
-	mutex_unlock(&call_mutex);
+	guard(rwsem_read)(&list_lock);
 
-	return ret;
+	priv = get_first_smbios_priv();
+	if (!priv)
+		return -ENODEV;
+
+	guard(mutex)(&priv->call_lock);
+
+	return dell_smbios_wmi_do_ioctl(priv, input);
 }
 
 static const struct file_operations dell_smbios_wmi_fops = {
 	.owner		= THIS_MODULE,
-	.open		= dell_smbios_wmi_open,
 	.read		= dell_smbios_wmi_read,
 	.unlocked_ioctl	= dell_smbios_wmi_ioctl,
 	.compat_ioctl	= compat_ptr_ioctl,
@@ -254,6 +253,10 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
 	if (!priv->buf)
 		return -ENOMEM;
 
+	ret = devm_mutex_init(&wdev->dev, &priv->call_lock);
+	if (ret)
+		return ret;
+
 	ret = dell_smbios_wmi_register_chardev(priv);
 	if (ret)
 		return ret;
@@ -262,9 +265,8 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
 	if (ret)
 		return ret;
 
-	mutex_lock(&list_mutex);
+	guard(rwsem_write)(&list_lock);
 	list_add_tail(&priv->list, &wmi_list);
-	mutex_unlock(&list_mutex);
 
 	return 0;
 }
@@ -273,12 +275,10 @@ static void dell_smbios_wmi_remove(struct wmi_device *wdev)
 {
 	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
 
-	mutex_lock(&call_mutex);
-	mutex_lock(&list_mutex);
-	list_del(&priv->list);
-	mutex_unlock(&list_mutex);
 	dell_smbios_unregister_device(&wdev->dev);
-	mutex_unlock(&call_mutex);
+
+	guard(rwsem_write)(&list_lock);
+	list_del(&priv->list);
 }
 
 static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
-- 
2.39.5


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

* [PATCH 2/3] platform/x86: dell-smbios: Pass device to callbacks
  2026-07-16  5:33 [PATCH 0/3] platform/x86: dell-smbios-wmi: Fixes and cleanups Armin Wolf
  2026-07-16  5:33 ` [PATCH 1/3] platform/x86: dell-smbios-wmi: Fix chardev resource management Armin Wolf
@ 2026-07-16  5:33 ` Armin Wolf
  2026-07-16  5:33 ` [PATCH 3/3] platform/x86: dell-smbios-wmi: Replace global list with single item Armin Wolf
  2 siblings, 0 replies; 6+ messages in thread
From: Armin Wolf @ 2026-07-16  5:33 UTC (permalink / raw)
  To: ilpo.jarvinen, hansg
  Cc: Dell.Client.Kernel, shuangpeng.kernel, platform-driver-x86,
	linux-kernel, gregkh, arnd

The WMI SMBIOS backend needs to access the its driver state container
when performing SMBIOS calls. Pass the device associated with a given
backend to the callback function to allow the WMI backend to retrieve
said state container in a more straightforward manner.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-smbios-base.c |  6 +++---
 drivers/platform/x86/dell/dell-smbios-smm.c  |  8 ++++----
 drivers/platform/x86/dell/dell-smbios-wmi.c  | 10 ++--------
 drivers/platform/x86/dell/dell-smbios.h      |  4 +++-
 4 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c
index 4b7c54b76228..d8c55ec6acd4 100644
--- a/drivers/platform/x86/dell/dell-smbios-base.c
+++ b/drivers/platform/x86/dell/dell-smbios-base.c
@@ -40,7 +40,7 @@ struct smbios_device {
 	struct list_head list;
 	struct device *device;
 	int priority;
-	int (*call_fn)(struct calling_interface_buffer *arg);
+	smbios_callback_fn_t call_fn;
 };
 
 struct smbios_call {
@@ -146,7 +146,7 @@ int dell_smbios_error(int value)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_error);
 
-int dell_smbios_register_device(struct device *d, int priority, void *call_fn)
+int dell_smbios_register_device(struct device *d, int priority, smbios_callback_fn_t call_fn)
 {
 	struct smbios_device *priv;
 
@@ -312,7 +312,7 @@ int dell_smbios_call(struct calling_interface_buffer *buffer)
 		goto out_smbios_call;
 	}
 
-	ret = selected->call_fn(buffer);
+	ret = selected->call_fn(selected->device, buffer);
 
 out_smbios_call:
 	mutex_unlock(&smbios_mutex);
diff --git a/drivers/platform/x86/dell/dell-smbios-smm.c b/drivers/platform/x86/dell/dell-smbios-smm.c
index 7055e2c40f34..fc4a674c692e 100644
--- a/drivers/platform/x86/dell/dell-smbios-smm.c
+++ b/drivers/platform/x86/dell/dell-smbios-smm.c
@@ -49,7 +49,7 @@ static void find_cmd_address(const struct dmi_header *dm, void *dummy)
 	}
 }
 
-static int dell_smbios_smm_call(struct calling_interface_buffer *input)
+static int dell_smbios_smm_call(struct device *dev, struct calling_interface_buffer *input)
 {
 	struct smi_cmd command;
 	size_t size;
@@ -70,7 +70,7 @@ static int dell_smbios_smm_call(struct calling_interface_buffer *input)
 }
 
 /* When enabled this indicates that SMM won't work */
-static bool test_wsmt_enabled(void)
+static bool test_wsmt_enabled(struct device *dev)
 {
 	struct calling_interface_token *wsmt;
 
@@ -88,7 +88,7 @@ static bool test_wsmt_enabled(void)
 	memset(buffer, 0, sizeof(struct calling_interface_buffer));
 	buffer->input[0] = wsmt->location;
 	buffer->output[0] = 99;
-	dell_smbios_smm_call(buffer);
+	dell_smbios_smm_call(dev, buffer);
 	if (buffer->output[0] == 99)
 		return true;
 
@@ -109,7 +109,7 @@ int init_dell_smbios_smm(void)
 
 	dmi_walk(find_cmd_address, NULL);
 
-	if (test_wsmt_enabled()) {
+	if (test_wsmt_enabled(&platform_device->dev)) {
 		pr_debug("Disabling due to WSMT enabled\n");
 		ret = -ENODEV;
 		goto fail_wsmt;
diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
index c8a6e711283d..552b80429688 100644
--- a/drivers/platform/x86/dell/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
@@ -82,19 +82,13 @@ static int run_smbios_call(struct wmi_device *wdev)
 	return 0;
 }
 
-static int dell_smbios_wmi_call(struct calling_interface_buffer *buffer)
+static int dell_smbios_wmi_call(struct device *dev, struct calling_interface_buffer *buffer)
 {
-	struct wmi_smbios_priv *priv;
+	struct wmi_smbios_priv *priv = dev_get_drvdata(dev);
 	size_t difference;
 	size_t size;
 	int ret;
 
-	guard(rwsem_read)(&list_lock);
-
-	priv = get_first_smbios_priv();
-	if (!priv)
-		return -ENODEV;
-
 	size = sizeof(struct calling_interface_buffer);
 	difference = priv->req_buf_size - sizeof(u64) - size;
 
diff --git a/drivers/platform/x86/dell/dell-smbios.h b/drivers/platform/x86/dell/dell-smbios.h
index f421b8533a9e..1e64803bc00d 100644
--- a/drivers/platform/x86/dell/dell-smbios.h
+++ b/drivers/platform/x86/dell/dell-smbios.h
@@ -47,6 +47,8 @@
 
 struct notifier_block;
 
+typedef int (*smbios_callback_fn_t)(struct device *dev, struct calling_interface_buffer *buffer);
+
 struct calling_interface_token {
 	u16 tokenID;
 	u16 location;
@@ -64,7 +66,7 @@ struct calling_interface_structure {
 	struct calling_interface_token tokens[];
 } __packed;
 
-int dell_smbios_register_device(struct device *d, int priority, void *call_fn);
+int dell_smbios_register_device(struct device *d, int priority, smbios_callback_fn_t call_fn);
 void dell_smbios_unregister_device(struct device *d);
 
 int dell_smbios_error(int value);
-- 
2.39.5


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

* [PATCH 3/3] platform/x86: dell-smbios-wmi: Replace global list with single item
  2026-07-16  5:33 [PATCH 0/3] platform/x86: dell-smbios-wmi: Fixes and cleanups Armin Wolf
  2026-07-16  5:33 ` [PATCH 1/3] platform/x86: dell-smbios-wmi: Fix chardev resource management Armin Wolf
  2026-07-16  5:33 ` [PATCH 2/3] platform/x86: dell-smbios: Pass device to callbacks Armin Wolf
@ 2026-07-16  5:33 ` Armin Wolf
  2026-07-20 10:18   ` Ilpo Järvinen
  2 siblings, 1 reply; 6+ messages in thread
From: Armin Wolf @ 2026-07-16  5:33 UTC (permalink / raw)
  To: ilpo.jarvinen, hansg
  Cc: Dell.Client.Kernel, shuangpeng.kernel, platform-driver-x86,
	linux-kernel, gregkh, arnd

There can only exist a single instance of the dell-smbios-wmi driver
at the same time because of naming conflicts with the character
device ("wmi/dell-smbios"). Having a global list for all instances
thus makes no sense.

Replace the global list with a single item used by the character
device. This simplifies the driver and allows us to mark it as
being multi-instance safe.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-smbios-wmi.c | 67 ++++++++++-----------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
index 552b80429688..f9443ddfff55 100644
--- a/drivers/platform/x86/dell/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
@@ -10,7 +10,6 @@
 #include <linux/device.h>
 #include <linux/dmi.h>
 #include <linux/fs.h>
-#include <linux/list.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -21,7 +20,6 @@
 #include "dell-smbios.h"
 #include "dell-wmi-descriptor.h"
 
-static DECLARE_RWSEM(list_lock);
 static int wmi_supported;
 
 struct misc_bios_flags_structure {
@@ -35,20 +33,14 @@ struct misc_bios_flags_structure {
 struct wmi_smbios_priv {
 	struct mutex call_lock; /* Protects the content of the SMBIOS buffer */
 	struct dell_wmi_smbios_buffer *buf;
-	struct list_head list;
 	struct wmi_device *wdev;
 	struct device *child;
 	u64 req_buf_size;
 	struct miscdevice char_dev;
 };
-static LIST_HEAD(wmi_list);
 
-static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
-{
-	return list_first_entry_or_null(&wmi_list,
-					struct wmi_smbios_priv,
-					list);
-}
+static DECLARE_RWSEM(chardev_lock);	/* Protects chardev_priv */
+static struct wmi_smbios_priv *chardev_priv;
 
 static int run_smbios_call(struct wmi_device *wdev)
 {
@@ -105,16 +97,13 @@ static int dell_smbios_wmi_call(struct device *dev, struct calling_interface_buf
 static ssize_t dell_smbios_wmi_read(struct file *filp, char __user *buffer, size_t length,
 				    loff_t *offset)
 {
-	struct wmi_smbios_priv *priv;
-
-	guard(rwsem_read)(&list_lock);
+	guard(rwsem_read)(&chardev_lock);
 
-	priv = get_first_smbios_priv();
-	if (!priv)
+	if (!chardev_priv)
 		return -ENODEV;
 
-	return simple_read_from_buffer(buffer, length, offset, &priv->req_buf_size,
-				       sizeof(priv->req_buf_size));
+	return simple_read_from_buffer(buffer, length, offset, &chardev_priv->req_buf_size,
+				       sizeof(chardev_priv->req_buf_size));
 }
 
 static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
@@ -158,20 +147,18 @@ static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
 static long dell_smbios_wmi_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct dell_wmi_smbios_buffer __user *input = (struct dell_wmi_smbios_buffer __user *)arg;
-	struct wmi_smbios_priv *priv;
 
 	if (cmd != DELL_WMI_SMBIOS_CMD)
 		return -ENOIOCTLCMD;
 
-	guard(rwsem_read)(&list_lock);
+	guard(rwsem_read)(&chardev_lock);
 
-	priv = get_first_smbios_priv();
-	if (!priv)
+	if (!chardev_priv)
 		return -ENODEV;
 
-	guard(mutex)(&priv->call_lock);
+	guard(mutex)(&chardev_priv->call_lock);
 
-	return dell_smbios_wmi_do_ioctl(priv, input);
+	return dell_smbios_wmi_do_ioctl(chardev_priv, input);
 }
 
 static const struct file_operations dell_smbios_wmi_fops = {
@@ -188,10 +175,29 @@ static void dell_smbios_wmi_unregister_chardev(void *data)
 	misc_deregister(char_dev);
 }
 
+static void dell_smbios_wmi_clear_chardev(void *data)
+{
+	guard(rwsem_write)(&chardev_lock);
+
+	chardev_priv = NULL;
+}
+
 static int dell_smbios_wmi_register_chardev(struct wmi_smbios_priv *priv)
 {
 	int ret;
 
+	scoped_guard(rwsem_write, &chardev_lock) {
+		/* We can only have a single chardev at a time */
+		if (chardev_priv)
+			return -EBUSY;
+
+		chardev_priv = priv;
+	}
+
+	ret = devm_add_action_or_reset(&priv->wdev->dev, dell_smbios_wmi_clear_chardev, NULL);
+	if (ret < 0)
+		return ret;
+
 	priv->char_dev.minor = MISC_DYNAMIC_MINOR;
 	priv->char_dev.name = "wmi/dell-smbios";
 	priv->char_dev.fops = &dell_smbios_wmi_fops;
@@ -255,24 +261,12 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
 	if (ret)
 		return ret;
 
-	ret = dell_smbios_register_device(&wdev->dev, 1, &dell_smbios_wmi_call);
-	if (ret)
-		return ret;
-
-	guard(rwsem_write)(&list_lock);
-	list_add_tail(&priv->list, &wmi_list);
-
-	return 0;
+	return dell_smbios_register_device(&wdev->dev, 1, &dell_smbios_wmi_call);
 }
 
 static void dell_smbios_wmi_remove(struct wmi_device *wdev)
 {
-	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
-
 	dell_smbios_unregister_device(&wdev->dev);
-
-	guard(rwsem_write)(&list_lock);
-	list_del(&priv->list);
 }
 
 static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
@@ -310,6 +304,7 @@ static struct wmi_driver dell_smbios_wmi_driver = {
 	.probe = dell_smbios_wmi_probe,
 	.remove = dell_smbios_wmi_remove,
 	.id_table = dell_smbios_wmi_id_table,
+	.no_singleton = true,
 };
 
 int init_dell_smbios_wmi(void)
-- 
2.39.5


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

* Re: [PATCH 1/3] platform/x86: dell-smbios-wmi: Fix chardev resource management
  2026-07-16  5:33 ` [PATCH 1/3] platform/x86: dell-smbios-wmi: Fix chardev resource management Armin Wolf
@ 2026-07-20 10:08   ` Ilpo Järvinen
  0 siblings, 0 replies; 6+ messages in thread
From: Ilpo Järvinen @ 2026-07-20 10:08 UTC (permalink / raw)
  To: Armin Wolf
  Cc: Hans de Goede, Dell.Client.Kernel, shuangpeng.kernel,
	platform-driver-x86, LKML, Greg Kroah-Hartman, arnd

On Thu, 16 Jul 2026, Armin Wolf wrote:

> When unbinding the WMI driver while a userspace application has
> an open file descriptor for the character device, a UAF occurs:
> 
> KASAN: slab-use-after-free in _copy_to_user from platform/x86/dell-smbios-wmi
> 
> The reason for this is that even after calling misc_deregister(),
> userspace appications can still call read() and/or ioctl() on open
> file descriptors associated with the already unregistered character
> device. This causes a UAF by attempting to access the already freed
> state container of the WMI driver.
> 
> Fix this by no longer storing the state container inside
> filp->private_data. Instead retrieve the state container using
> get_first_smbios_priv() and return -ENODEV if the state container
> does not exist anymore.
> 
> Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
> Closes: https://lore.kernel.org/platform-driver-x86/178144969601.60470.13396800403157907003@gmail.com/
> Fixes: 93885e85a77f ("platform/x86: dell-smbios-wmi: Stop using WMI chardev")
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/dell/dell-smbios-wmi.c | 66 ++++++++++-----------
>  1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
> index 64d0871b706e..c8a6e711283d 100644
> --- a/drivers/platform/x86/dell/dell-smbios-wmi.c
> +++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
> @@ -6,6 +6,7 @@
>   */
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include <linux/cleanup.h>
>  #include <linux/device.h>
>  #include <linux/dmi.h>
>  #include <linux/fs.h>
> @@ -13,14 +14,14 @@
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/rwsem.h>
>  #include <linux/uaccess.h>
>  #include <linux/wmi.h>
>  #include <uapi/linux/wmi.h>
>  #include "dell-smbios.h"
>  #include "dell-wmi-descriptor.h"
>  
> -static DEFINE_MUTEX(call_mutex);
> -static DEFINE_MUTEX(list_mutex);
> +static DECLARE_RWSEM(list_lock);

Please add a comment about what this protects.

Maybe move wmi_list next to it so the connection between them is more 
obvious?

Should get_first_smbios_priv() use lockdep_assert_held() to check it's 
called from a safe context?

-- 
 i.

>  static int wmi_supported;
>  
>  struct misc_bios_flags_structure {
> @@ -32,6 +33,7 @@ struct misc_bios_flags_structure {
>  #define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
>  
>  struct wmi_smbios_priv {
> +	struct mutex call_lock; /* Protects the content of the SMBIOS buffer */
>  	struct dell_wmi_smbios_buffer *buf;
>  	struct list_head list;
>  	struct wmi_device *wdev;
> @@ -87,40 +89,35 @@ static int dell_smbios_wmi_call(struct calling_interface_buffer *buffer)
>  	size_t size;
>  	int ret;
>  
> -	mutex_lock(&call_mutex);
> +	guard(rwsem_read)(&list_lock);
> +
>  	priv = get_first_smbios_priv();
> -	if (!priv) {
> -		ret = -ENODEV;
> -		goto out_wmi_call;
> -	}
> +	if (!priv)
> +		return -ENODEV;
>  
>  	size = sizeof(struct calling_interface_buffer);
>  	difference = priv->req_buf_size - sizeof(u64) - size;
>  
> +	guard(mutex)(&priv->call_lock);
> +
>  	memset(&priv->buf->ext, 0, difference);
>  	memcpy(&priv->buf->std, buffer, size);
>  	ret = run_smbios_call(priv->wdev);
>  	memcpy(buffer, &priv->buf->std, size);
> -out_wmi_call:
> -	mutex_unlock(&call_mutex);
>  
>  	return ret;
>  }
>  
> -static int dell_smbios_wmi_open(struct inode *inode, struct file *filp)
> +static ssize_t dell_smbios_wmi_read(struct file *filp, char __user *buffer, size_t length,
> +				    loff_t *offset)
>  {
>  	struct wmi_smbios_priv *priv;
>  
> -	priv = container_of(filp->private_data, struct wmi_smbios_priv, char_dev);
> -	filp->private_data = priv;
> +	guard(rwsem_read)(&list_lock);
>  
> -	return nonseekable_open(inode, filp);
> -}
> -
> -static ssize_t dell_smbios_wmi_read(struct file *filp, char __user *buffer, size_t length,
> -				    loff_t *offset)
> -{
> -	struct wmi_smbios_priv *priv = filp->private_data;
> +	priv = get_first_smbios_priv();
> +	if (!priv)
> +		return -ENODEV;
>  
>  	return simple_read_from_buffer(buffer, length, offset, &priv->req_buf_size,
>  				       sizeof(priv->req_buf_size));
> @@ -167,22 +164,24 @@ static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
>  static long dell_smbios_wmi_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
>  	struct dell_wmi_smbios_buffer __user *input = (struct dell_wmi_smbios_buffer __user *)arg;
> -	struct wmi_smbios_priv *priv = filp->private_data;
> -	long ret;
> +	struct wmi_smbios_priv *priv;
>  
>  	if (cmd != DELL_WMI_SMBIOS_CMD)
>  		return -ENOIOCTLCMD;
>  
> -	mutex_lock(&call_mutex);
> -	ret = dell_smbios_wmi_do_ioctl(priv, input);
> -	mutex_unlock(&call_mutex);
> +	guard(rwsem_read)(&list_lock);
>  
> -	return ret;
> +	priv = get_first_smbios_priv();
> +	if (!priv)
> +		return -ENODEV;
> +
> +	guard(mutex)(&priv->call_lock);
> +
> +	return dell_smbios_wmi_do_ioctl(priv, input);
>  }
>  
>  static const struct file_operations dell_smbios_wmi_fops = {
>  	.owner		= THIS_MODULE,
> -	.open		= dell_smbios_wmi_open,
>  	.read		= dell_smbios_wmi_read,
>  	.unlocked_ioctl	= dell_smbios_wmi_ioctl,
>  	.compat_ioctl	= compat_ptr_ioctl,
> @@ -254,6 +253,10 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
>  	if (!priv->buf)
>  		return -ENOMEM;
>  
> +	ret = devm_mutex_init(&wdev->dev, &priv->call_lock);
> +	if (ret)
> +		return ret;
> +
>  	ret = dell_smbios_wmi_register_chardev(priv);
>  	if (ret)
>  		return ret;
> @@ -262,9 +265,8 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
>  	if (ret)
>  		return ret;
>  
> -	mutex_lock(&list_mutex);
> +	guard(rwsem_write)(&list_lock);
>  	list_add_tail(&priv->list, &wmi_list);
> -	mutex_unlock(&list_mutex);
>  
>  	return 0;
>  }
> @@ -273,12 +275,10 @@ static void dell_smbios_wmi_remove(struct wmi_device *wdev)
>  {
>  	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
>  
> -	mutex_lock(&call_mutex);
> -	mutex_lock(&list_mutex);
> -	list_del(&priv->list);
> -	mutex_unlock(&list_mutex);
>  	dell_smbios_unregister_device(&wdev->dev);
> -	mutex_unlock(&call_mutex);
> +
> +	guard(rwsem_write)(&list_lock);
> +	list_del(&priv->list);
>  }
>  
>  static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
> 

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

* Re: [PATCH 3/3] platform/x86: dell-smbios-wmi: Replace global list with single item
  2026-07-16  5:33 ` [PATCH 3/3] platform/x86: dell-smbios-wmi: Replace global list with single item Armin Wolf
@ 2026-07-20 10:18   ` Ilpo Järvinen
  0 siblings, 0 replies; 6+ messages in thread
From: Ilpo Järvinen @ 2026-07-20 10:18 UTC (permalink / raw)
  To: Armin Wolf
  Cc: Hans de Goede, Dell.Client.Kernel, shuangpeng.kernel,
	platform-driver-x86, LKML, Greg Kroah-Hartman, arnd

On Thu, 16 Jul 2026, Armin Wolf wrote:

> There can only exist a single instance of the dell-smbios-wmi driver
> at the same time because of naming conflicts with the character
> device ("wmi/dell-smbios"). Having a global list for all instances
> thus makes no sense.
> 
> Replace the global list with a single item used by the character
> device. This simplifies the driver and allows us to mark it as
> being multi-instance safe.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/dell/dell-smbios-wmi.c | 67 ++++++++++-----------
>  1 file changed, 31 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
> index 552b80429688..f9443ddfff55 100644
> --- a/drivers/platform/x86/dell/dell-smbios-wmi.c
> +++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
> @@ -10,7 +10,6 @@
>  #include <linux/device.h>
>  #include <linux/dmi.h>
>  #include <linux/fs.h>
> -#include <linux/list.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> @@ -21,7 +20,6 @@
>  #include "dell-smbios.h"
>  #include "dell-wmi-descriptor.h"
>  
> -static DECLARE_RWSEM(list_lock);
>  static int wmi_supported;
>  
>  struct misc_bios_flags_structure {
> @@ -35,20 +33,14 @@ struct misc_bios_flags_structure {
>  struct wmi_smbios_priv {
>  	struct mutex call_lock; /* Protects the content of the SMBIOS buffer */
>  	struct dell_wmi_smbios_buffer *buf;
> -	struct list_head list;
>  	struct wmi_device *wdev;
>  	struct device *child;
>  	u64 req_buf_size;
>  	struct miscdevice char_dev;
>  };
> -static LIST_HEAD(wmi_list);

Okay, nevermind the comments on patch 1.

--
 i.

>  
> -static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
> -{
> -	return list_first_entry_or_null(&wmi_list,
> -					struct wmi_smbios_priv,
> -					list);
> -}
> +static DECLARE_RWSEM(chardev_lock);	/* Protects chardev_priv */
> +static struct wmi_smbios_priv *chardev_priv;
>  
>  static int run_smbios_call(struct wmi_device *wdev)
>  {
> @@ -105,16 +97,13 @@ static int dell_smbios_wmi_call(struct device *dev, struct calling_interface_buf
>  static ssize_t dell_smbios_wmi_read(struct file *filp, char __user *buffer, size_t length,
>  				    loff_t *offset)
>  {
> -	struct wmi_smbios_priv *priv;
> -
> -	guard(rwsem_read)(&list_lock);
> +	guard(rwsem_read)(&chardev_lock);
>  
> -	priv = get_first_smbios_priv();
> -	if (!priv)
> +	if (!chardev_priv)
>  		return -ENODEV;
>  
> -	return simple_read_from_buffer(buffer, length, offset, &priv->req_buf_size,
> -				       sizeof(priv->req_buf_size));
> +	return simple_read_from_buffer(buffer, length, offset, &chardev_priv->req_buf_size,
> +				       sizeof(chardev_priv->req_buf_size));
>  }
>  
>  static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
> @@ -158,20 +147,18 @@ static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
>  static long dell_smbios_wmi_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
>  	struct dell_wmi_smbios_buffer __user *input = (struct dell_wmi_smbios_buffer __user *)arg;
> -	struct wmi_smbios_priv *priv;
>  
>  	if (cmd != DELL_WMI_SMBIOS_CMD)
>  		return -ENOIOCTLCMD;
>  
> -	guard(rwsem_read)(&list_lock);
> +	guard(rwsem_read)(&chardev_lock);
>  
> -	priv = get_first_smbios_priv();
> -	if (!priv)
> +	if (!chardev_priv)
>  		return -ENODEV;
>  
> -	guard(mutex)(&priv->call_lock);
> +	guard(mutex)(&chardev_priv->call_lock);
>  
> -	return dell_smbios_wmi_do_ioctl(priv, input);
> +	return dell_smbios_wmi_do_ioctl(chardev_priv, input);
>  }
>  
>  static const struct file_operations dell_smbios_wmi_fops = {
> @@ -188,10 +175,29 @@ static void dell_smbios_wmi_unregister_chardev(void *data)
>  	misc_deregister(char_dev);
>  }
>  
> +static void dell_smbios_wmi_clear_chardev(void *data)
> +{
> +	guard(rwsem_write)(&chardev_lock);
> +
> +	chardev_priv = NULL;
> +}
> +
>  static int dell_smbios_wmi_register_chardev(struct wmi_smbios_priv *priv)
>  {
>  	int ret;
>  
> +	scoped_guard(rwsem_write, &chardev_lock) {
> +		/* We can only have a single chardev at a time */
> +		if (chardev_priv)
> +			return -EBUSY;
> +
> +		chardev_priv = priv;
> +	}
> +
> +	ret = devm_add_action_or_reset(&priv->wdev->dev, dell_smbios_wmi_clear_chardev, NULL);
> +	if (ret < 0)
> +		return ret;
> +
>  	priv->char_dev.minor = MISC_DYNAMIC_MINOR;
>  	priv->char_dev.name = "wmi/dell-smbios";
>  	priv->char_dev.fops = &dell_smbios_wmi_fops;
> @@ -255,24 +261,12 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
>  	if (ret)
>  		return ret;
>  
> -	ret = dell_smbios_register_device(&wdev->dev, 1, &dell_smbios_wmi_call);
> -	if (ret)
> -		return ret;
> -
> -	guard(rwsem_write)(&list_lock);
> -	list_add_tail(&priv->list, &wmi_list);
> -
> -	return 0;
> +	return dell_smbios_register_device(&wdev->dev, 1, &dell_smbios_wmi_call);
>  }
>  
>  static void dell_smbios_wmi_remove(struct wmi_device *wdev)
>  {
> -	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
> -
>  	dell_smbios_unregister_device(&wdev->dev);
> -
> -	guard(rwsem_write)(&list_lock);
> -	list_del(&priv->list);
>  }
>  
>  static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
> @@ -310,6 +304,7 @@ static struct wmi_driver dell_smbios_wmi_driver = {
>  	.probe = dell_smbios_wmi_probe,
>  	.remove = dell_smbios_wmi_remove,
>  	.id_table = dell_smbios_wmi_id_table,
> +	.no_singleton = true,
>  };
>  
>  int init_dell_smbios_wmi(void)
> 

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

end of thread, other threads:[~2026-07-20 10:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16  5:33 [PATCH 0/3] platform/x86: dell-smbios-wmi: Fixes and cleanups Armin Wolf
2026-07-16  5:33 ` [PATCH 1/3] platform/x86: dell-smbios-wmi: Fix chardev resource management Armin Wolf
2026-07-20 10:08   ` Ilpo Järvinen
2026-07-16  5:33 ` [PATCH 2/3] platform/x86: dell-smbios: Pass device to callbacks Armin Wolf
2026-07-16  5:33 ` [PATCH 3/3] platform/x86: dell-smbios-wmi: Replace global list with single item Armin Wolf
2026-07-20 10:18   ` Ilpo Järvinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.