[PATCH] platform/x86: dell-smo8800: keep the state until the last file is closed

[PATCH] platform/x86: dell-smo8800: keep the state until the last file is closed

[Chad Aldreda]

struct smo8800_device is allocated with devm_kzalloc(), so it is freed
when the driver is unbound, but a /dev/freefall file opened before the
unbind still reaches it through file->private_data. release() then
clears misc_opened in freed memory, and a read() blocked in
wait_event_interruptible() is not woken by remove(). It sleeps on a wait
queue inside the freed structure until a signal arrives, and then uses
it. Unbinding the driver needs root. With KASAN, closing the file after
an unbind gives a slab-use-after-free in smo8800_misc_release(), and a
blocked reader interrupted after the unbind gives further reports in
smo8800_misc_read() and prepare_to_wait_event().

Allocate the structure with kzalloc() and give it a kref. open() takes a
reference and release() drops it. The device's own reference is dropped
by a devm action, which runs after smo8800_remove() on unbind and when
probe fails. misc_open() calls ->open with misc_mtx held and
misc_deregister() takes misc_mtx, so the device reference is held
whenever ->open runs. Once the misc device is deregistered, by remove()
or by the probe error path, a removed flag is set and the wait queue
woken. A read() blocked at that point returns -ENODEV unless an event
arrived first, and a later read() returns -ENODEV.

The problem was found with a Coccinelle script that reports device nodes
in devm memory [1]. The fix, the reproducer and this message were
written with an AI assistant (Claude) and checked by hand. The fix was
tested in QEMU, with KASAN enabled, using a test-only module that
registers an "smo8800" platform device with a simulated interrupt, not
on Dell hardware.

Link: https://lore.kernel.org/r/20261106193517.3267013-2-ca@modelmiser.com [1]
Fixes: 4738d8aabe89 ("platform: x86: dell-smo8800: Dell Latitude freefall driver (ACPI SMO8800/SMO8810)")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Chad Aldreda <ca@modelmiser.com>
---
The Coccinelle script reports a devm_k*() allocation when it, or an
address inside it, is passed to misc_register(), cdev_add(),
cdev_device_add(), video_register_device() or v4l2_device_register()
in the same function, or when the allocated struct embeds one of those
objects. It reports dell-smo8800 for the misc_register() of
&smo8800->miscdev.

Reproduced in QEMU 10.2.4 (q35, KASAN, booted with kasan_multi_shot). A
test-only module registers a platform device named "smo8800" with an
interrupt from an irq_sim domain, so the driver binds without the ACPI
device, and a module parameter raises the interrupt. A process opens
/dev/freefall and the driver is unbound through sysfs. Without this
patch:

  close after the unbind:                  1 KASAN report (release)
  read() blocked across the unbind:        not woken; after a signal,
                                           7 reports (read, wait queue,
                                           release)
  unbind, bind, then close the old file:   1 report
  close before the unbind (control):       0

With it there are none: the blocked read() returns -ENODEV when the
driver is unbound, a bind after the unbind works, and an interrupt
still wakes a reader with a count. Not tested on Dell hardware.

An interrupt that arrives while remove() runs, between the reader's
count check and its removed check, is dropped and the read returns
-ENODEV. remove() frees the IRQ before setting removed, so no event can
be lost after that.

kzalloc() rather than kzalloc_obj(), which first appears in v7.0: the
patch then applies as is from v6.14. On v6.6 to v6.13 only the include
hunk needs adjusting (those trees have no dell-smo8800-ids.h); v5.15 and
v6.1 need a manual backport (remove() returns int there), and before
v5.15 the driver was an ACPI driver.

 drivers/platform/x86/dell/dell-smo8800.c | 35 ++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-smo8800.c b/drivers/platform/x86/dell/dell-smo8800.c
index 8872f9b57fce..8880bf39f895 100644
--- a/drivers/platform/x86/dell/dell-smo8800.c
+++ b/drivers/platform/x86/dell/dell-smo8800.c
@@ -13,9 +13,11 @@
 #include <linux/fs.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
+#include <linux/kref.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/slab.h>
 #include <linux/uaccess.h>
 #include "dell-smo8800-ids.h"
 
@@ -26,8 +28,22 @@ struct smo8800_device {
 	unsigned long misc_opened;   /* whether the device is open */
 	wait_queue_head_t misc_wait; /* Wait queue for the misc dev */
 	struct device *dev;          /* acpi device */
+	struct kref kref;            /* device + open file */
+	bool removed;                /* the device has been removed */
 };
 
+static void smo8800_free(struct kref *kref)
+{
+	kfree(container_of(kref, struct smo8800_device, kref));
+}
+
+static void smo8800_put(void *data)
+{
+	struct smo8800_device *smo8800 = data;
+
+	kref_put(&smo8800->kref, smo8800_free);
+}
+
 static irqreturn_t smo8800_interrupt_quick(int irq, void *data)
 {
 	struct smo8800_device *smo8800 = data;
@@ -60,11 +76,15 @@ static ssize_t smo8800_misc_read(struct file *file, char __user *buf,
 
 	atomic_set(&smo8800->counter, 0);
 	retval = wait_event_interruptible(smo8800->misc_wait,
-				(data = atomic_xchg(&smo8800->counter, 0)));
+				(data = atomic_xchg(&smo8800->counter, 0)) ||
+				READ_ONCE(smo8800->removed));
 
 	if (retval)
 		return retval;
 
+	if (!data)
+		return -ENODEV;
+
 	retval = 1;
 
 	byte_data = min_t(u32, data, 255);
@@ -84,6 +104,7 @@ static int smo8800_misc_open(struct inode *inode, struct file *file)
 		return -EBUSY; /* already open */
 
 	atomic_set(&smo8800->counter, 0);
+	kref_get(&smo8800->kref);
 	return 0;
 }
 
@@ -93,6 +114,7 @@ static int smo8800_misc_release(struct inode *inode, struct file *file)
 					 struct smo8800_device, miscdev);
 
 	clear_bit(0, &smo8800->misc_opened); /* release the device */
+	smo8800_put(smo8800);
 	return 0;
 }
 
@@ -108,11 +130,16 @@ static int smo8800_probe(struct platform_device *device)
 	int err;
 	struct smo8800_device *smo8800;
 
-	smo8800 = devm_kzalloc(&device->dev, sizeof(*smo8800), GFP_KERNEL);
+	/* An open file can outlive the device, so it holds a reference. */
+	smo8800 = kzalloc(sizeof(*smo8800), GFP_KERNEL);
 	if (!smo8800) {
 		dev_err(&device->dev, "failed to allocate device data\n");
 		return -ENOMEM;
 	}
+	kref_init(&smo8800->kref);
+	err = devm_add_action_or_reset(&device->dev, smo8800_put, smo8800);
+	if (err)
+		return err;
 
 	smo8800->dev = &device->dev;
 	smo8800->miscdev.minor = MISC_DYNAMIC_MINOR;
@@ -151,6 +178,8 @@ static int smo8800_probe(struct platform_device *device)
 
 error:
 	misc_deregister(&smo8800->miscdev);
+	WRITE_ONCE(smo8800->removed, true);
+	wake_up_interruptible(&smo8800->misc_wait);
 	return err;
 }
 
@@ -160,6 +189,8 @@ static void smo8800_remove(struct platform_device *device)
 
 	free_irq(smo8800->irq, smo8800);
 	misc_deregister(&smo8800->miscdev);
+	WRITE_ONCE(smo8800->removed, true);
+	wake_up_interruptible(&smo8800->misc_wait);
 	dev_dbg(&device->dev, "device /dev/freefall unregistered\n");
 }
 
-- 
2.43.0

Re: [PATCH] platform/x86: dell-smo8800: keep the state until the last file is closed

[Chad Aldreda]

Shuangpeng Bai reported this bug in June, with a reproducer, and I missed
the report when I sent this patch. It is the same use-after-free after an
unbind with the file still open.

Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Closes: https://lore.kernel.org/all/178144969601.60470.2802762361760676957@gmail.com/

Chad

Re: [PATCH] platform/x86: dell-smo8800: keep the state until the last file is closed

[Pali Rohár]

Hello, I have not checked this patch too deeply, but I have feeling that
something is probably wrong if we need to add a new manual refcounter
around a whole device. Anyway, this freefall logic was originally copied
from the lis3lv02d.c driver, which now lives in the file:
linux/drivers/misc/lis3lv02d/lis3lv02d.c
So if in the freefall device of dell-smo8800.c is some error then
probably the same issue would be in the freefall device of lis3lv02d.c.

On Saturday 07 November 2026 20:49:58 Chad Aldreda wrote:
> struct smo8800_device is allocated with devm_kzalloc(), so it is freed
> when the driver is unbound, but a /dev/freefall file opened before the
> unbind still reaches it through file->private_data. release() then
> clears misc_opened in freed memory, and a read() blocked in
> wait_event_interruptible() is not woken by remove(). It sleeps on a wait
> queue inside the freed structure until a signal arrives, and then uses
> it. Unbinding the driver needs root. With KASAN, closing the file after
> an unbind gives a slab-use-after-free in smo8800_misc_release(), and a
> blocked reader interrupted after the unbind gives further reports in
> smo8800_misc_read() and prepare_to_wait_event().
> 
> Allocate the structure with kzalloc() and give it a kref. open() takes a
> reference and release() drops it. The device's own reference is dropped
> by a devm action, which runs after smo8800_remove() on unbind and when
> probe fails. misc_open() calls ->open with misc_mtx held and
> misc_deregister() takes misc_mtx, so the device reference is held
> whenever ->open runs. Once the misc device is deregistered, by remove()
> or by the probe error path, a removed flag is set and the wait queue
> woken. A read() blocked at that point returns -ENODEV unless an event
> arrived first, and a later read() returns -ENODEV.
> 
> The problem was found with a Coccinelle script that reports device nodes
> in devm memory [1]. The fix, the reproducer and this message were
> written with an AI assistant (Claude) and checked by hand. The fix was
> tested in QEMU, with KASAN enabled, using a test-only module that
> registers an "smo8800" platform device with a simulated interrupt, not
> on Dell hardware.
> 
> Link: https://lore.kernel.org/r/20261106193517.3267013-2-ca@modelmiser.com [1]
> Fixes: 4738d8aabe89 ("platform: x86: dell-smo8800: Dell Latitude freefall driver (ACPI SMO8800/SMO8810)")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Chad Aldreda <ca@modelmiser.com>
> ---
> The Coccinelle script reports a devm_k*() allocation when it, or an
> address inside it, is passed to misc_register(), cdev_add(),
> cdev_device_add(), video_register_device() or v4l2_device_register()
> in the same function, or when the allocated struct embeds one of those
> objects. It reports dell-smo8800 for the misc_register() of
> &smo8800->miscdev.
> 
> Reproduced in QEMU 10.2.4 (q35, KASAN, booted with kasan_multi_shot). A
> test-only module registers a platform device named "smo8800" with an
> interrupt from an irq_sim domain, so the driver binds without the ACPI
> device, and a module parameter raises the interrupt. A process opens
> /dev/freefall and the driver is unbound through sysfs. Without this
> patch:
> 
>   close after the unbind:                  1 KASAN report (release)
>   read() blocked across the unbind:        not woken; after a signal,
>                                            7 reports (read, wait queue,
>                                            release)
>   unbind, bind, then close the old file:   1 report
>   close before the unbind (control):       0
> 
> With it there are none: the blocked read() returns -ENODEV when the
> driver is unbound, a bind after the unbind works, and an interrupt
> still wakes a reader with a count. Not tested on Dell hardware.
> 
> An interrupt that arrives while remove() runs, between the reader's
> count check and its removed check, is dropped and the read returns
> -ENODEV. remove() frees the IRQ before setting removed, so no event can
> be lost after that.
> 
> kzalloc() rather than kzalloc_obj(), which first appears in v7.0: the
> patch then applies as is from v6.14. On v6.6 to v6.13 only the include
> hunk needs adjusting (those trees have no dell-smo8800-ids.h); v5.15 and
> v6.1 need a manual backport (remove() returns int there), and before
> v5.15 the driver was an ACPI driver.
> 
>  drivers/platform/x86/dell/dell-smo8800.c | 35 ++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-smo8800.c b/drivers/platform/x86/dell/dell-smo8800.c
> index 8872f9b57fce..8880bf39f895 100644
> --- a/drivers/platform/x86/dell/dell-smo8800.c
> +++ b/drivers/platform/x86/dell/dell-smo8800.c
> @@ -13,9 +13,11 @@
>  #include <linux/fs.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> +#include <linux/kref.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include "dell-smo8800-ids.h"
>  
> @@ -26,8 +28,22 @@ struct smo8800_device {
>  	unsigned long misc_opened;   /* whether the device is open */
>  	wait_queue_head_t misc_wait; /* Wait queue for the misc dev */
>  	struct device *dev;          /* acpi device */
> +	struct kref kref;            /* device + open file */
> +	bool removed;                /* the device has been removed */
>  };
>  
> +static void smo8800_free(struct kref *kref)
> +{
> +	kfree(container_of(kref, struct smo8800_device, kref));
> +}
> +
> +static void smo8800_put(void *data)
> +{
> +	struct smo8800_device *smo8800 = data;
> +
> +	kref_put(&smo8800->kref, smo8800_free);
> +}
> +
>  static irqreturn_t smo8800_interrupt_quick(int irq, void *data)
>  {
>  	struct smo8800_device *smo8800 = data;
> @@ -60,11 +76,15 @@ static ssize_t smo8800_misc_read(struct file *file, char __user *buf,
>  
>  	atomic_set(&smo8800->counter, 0);
>  	retval = wait_event_interruptible(smo8800->misc_wait,
> -				(data = atomic_xchg(&smo8800->counter, 0)));
> +				(data = atomic_xchg(&smo8800->counter, 0)) ||
> +				READ_ONCE(smo8800->removed));
>  
>  	if (retval)
>  		return retval;
>  
> +	if (!data)
> +		return -ENODEV;
> +
>  	retval = 1;
>  
>  	byte_data = min_t(u32, data, 255);
> @@ -84,6 +104,7 @@ static int smo8800_misc_open(struct inode *inode, struct file *file)
>  		return -EBUSY; /* already open */
>  
>  	atomic_set(&smo8800->counter, 0);
> +	kref_get(&smo8800->kref);
>  	return 0;
>  }
>  
> @@ -93,6 +114,7 @@ static int smo8800_misc_release(struct inode *inode, struct file *file)
>  					 struct smo8800_device, miscdev);
>  
>  	clear_bit(0, &smo8800->misc_opened); /* release the device */
> +	smo8800_put(smo8800);
>  	return 0;
>  }
>  
> @@ -108,11 +130,16 @@ static int smo8800_probe(struct platform_device *device)
>  	int err;
>  	struct smo8800_device *smo8800;
>  
> -	smo8800 = devm_kzalloc(&device->dev, sizeof(*smo8800), GFP_KERNEL);
> +	/* An open file can outlive the device, so it holds a reference. */
> +	smo8800 = kzalloc(sizeof(*smo8800), GFP_KERNEL);
>  	if (!smo8800) {
>  		dev_err(&device->dev, "failed to allocate device data\n");
>  		return -ENOMEM;
>  	}
> +	kref_init(&smo8800->kref);
> +	err = devm_add_action_or_reset(&device->dev, smo8800_put, smo8800);
> +	if (err)
> +		return err;
>  
>  	smo8800->dev = &device->dev;
>  	smo8800->miscdev.minor = MISC_DYNAMIC_MINOR;
> @@ -151,6 +178,8 @@ static int smo8800_probe(struct platform_device *device)
>  
>  error:
>  	misc_deregister(&smo8800->miscdev);
> +	WRITE_ONCE(smo8800->removed, true);
> +	wake_up_interruptible(&smo8800->misc_wait);
>  	return err;
>  }
>  
> @@ -160,6 +189,8 @@ static void smo8800_remove(struct platform_device *device)
>  
>  	free_irq(smo8800->irq, smo8800);
>  	misc_deregister(&smo8800->miscdev);
> +	WRITE_ONCE(smo8800->removed, true);
> +	wake_up_interruptible(&smo8800->misc_wait);
>  	dev_dbg(&device->dev, "device /dev/freefall unregistered\n");
>  }
>  
> -- 
> 2.43.0
> 
> 

Re: [PATCH] platform/x86: dell-smo8800: keep the state until the last file is closed

[Chad Aldreda]

On Tue, 10 Nov 2026, Pali Rohár wrote:
> Hello, I have not checked this patch too deeply, but I have feeling that
> something is probably wrong if we need to add a new manual refcounter
> around a whole device.

Fair question. The count covers only the lifetime of struct
smo8800_device: one reference while the driver is bound and one per
open /dev/freefall file. It is needed because the struct comes from
devm_kzalloc(), which frees it when the driver unbinds, while a file
opened before the unbind stays open and still reaches it through
file->private_data.

> Anyway, this freefall logic was originally copied from the lis3lv02d.c
> driver, which now lives in the file:
> linux/drivers/misc/lis3lv02d/lis3lv02d.c
> So if in the freefall device of dell-smo8800.c is some error then
> probably the same issue would be in the freefall device of lis3lv02d.c.

Thanks for the pointer. lis3lv02d does not have this use-after-free: its
freefall state is the static global lis3_dev, which is never freed, and
its file operations reach it through the miscdevice embedded there. It
does share the other half: a read() blocked at remove is not woken there
either, but with static state that only leaves the reader asleep until a
signal, without touching freed memory.

Making smo8800_device static like lis3_dev would also fix the
use-after-free. I kept it per device because with static state a file
left open across an unbind and bind keeps the device marked open, so new
opens get -EBUSY until it is closed, and the old file sees the new
binding's events, as in lis3lv02d. Either way I would keep the wakeup
at remove, so that a blocked read() returns -ENODEV instead of sleeping
until a signal. If you prefer the static version, I can send a v2 doing
that.

Chad

This reply was drafted with an AI assistant (Claude Opus 5.5).