[PATCH] HSI: cmt_speech: Fix crash on device removal with open file descriptor

[PATCH] HSI: cmt_speech: Fix crash on device removal with open file descriptor

[Shengzhuo Wei]

misc_deregister() does not drain file descriptors that are already
open, so after cs_hsi_client_remove() clears and frees
cs_char_data.hi, the ioctl and write paths still dereference it
without any lock or NULL check:

  open("/dev/cmt_speech")                          [userspace]
  rmmod cmt_speech                                 [removal]
  ioctl(fd, CS_GET_STATE, 0)                       [userspace]

Fix it by reading cs_char_data.hi under cs_char_data.lock and
rejecting calls with -ENODEV once the interface is gone;
cs_char_release() now steals the pointer under the lock like
cs_hsi_client_remove() does, so the interface is stopped exactly
once between them.

Fixes: 7f62fe8a5851 ("HSI: cmt_speech: Add cmt-speech driver")
Cc: stable@vger.kernel.org

Assisted-by: GLM:5.3
Signed-off-by: Shengzhuo Wei <me@cherr.cc>
---
Verified on a KASAN kernel with a software HSI controller: opening the
character device, unbinding the client and issuing the ioctl gives

  KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
  RIP: 0010:cs_char_ioctl+0x29e/0x340

and with the patch applied the same sequence returns -ENODEV.
---
 drivers/hsi/clients/cmt_speech.c | 47 ++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/hsi/clients/cmt_speech.c b/drivers/hsi/clients/cmt_speech.c
index 7226677ebde7..4325152a5a62 100644
--- a/drivers/hsi/clients/cmt_speech.c
+++ b/drivers/hsi/clients/cmt_speech.c
@@ -732,6 +732,17 @@ static int cs_hsi_write_on_data(struct cs_hsi_iface *hi, unsigned int slot)
 	return ret;
 }
 
+static struct cs_hsi_iface *cs_char_hsi_get(struct cs_char *csdata)
+{
+	struct cs_hsi_iface *hi;
+
+	spin_lock_bh(&csdata->lock);
+	hi = csdata->hi;
+	spin_unlock_bh(&csdata->lock);
+
+	return hi;
+}
+
 static unsigned int cs_hsi_get_state(struct cs_hsi_iface *hi)
 {
 	return hi->iface_state;
@@ -1180,6 +1191,7 @@ static ssize_t cs_char_write(struct file *file, const char __user *buf,
 	u32 data;
 	int err;
 	ssize_t	retval;
+	struct cs_hsi_iface *hi;
 
 	if (count < sizeof(data))
 		return -EINVAL;
@@ -1189,7 +1201,10 @@ static ssize_t cs_char_write(struct file *file, const char __user *buf,
 	else
 		retval = count;
 
-	err = cs_hsi_command(csdata->hi, data);
+	hi = cs_char_hsi_get(csdata);
+	if (!hi)
+		return -ENODEV;
+	err = cs_hsi_command(hi, data);
 	if (err < 0)
 		retval = err;
 
@@ -1205,8 +1220,11 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 	switch (cmd) {
 	case CS_GET_STATE: {
 		unsigned int state;
+		struct cs_hsi_iface *hi = cs_char_hsi_get(csdata);
 
-		state = cs_hsi_get_state(csdata->hi);
+		if (!hi)
+			return -ENODEV;
+		state = cs_hsi_get_state(hi);
 		if (copy_to_user((void __user *)arg, &state, sizeof(state)))
 			r = -EFAULT;
 
@@ -1214,6 +1232,7 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 	}
 	case CS_SET_WAKELINE: {
 		unsigned int state;
+		struct cs_hsi_iface *hi;
 
 		if (copy_from_user(&state, (void __user *)arg, sizeof(state))) {
 			r = -EFAULT;
@@ -1225,7 +1244,10 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 			break;
 		}
 
-		cs_hsi_set_wakeline(csdata->hi, !!state);
+		hi = cs_char_hsi_get(csdata);
+		if (!hi)
+			return -ENODEV;
+		cs_hsi_set_wakeline(hi, !!state);
 
 		break;
 	}
@@ -1239,12 +1261,17 @@ static long cs_char_ioctl(struct file *file, unsigned int cmd,
 	}
 	case CS_CONFIG_BUFS: {
 		struct cs_buffer_config buf_cfg;
+		struct cs_hsi_iface *hi;
 
 		if (copy_from_user(&buf_cfg, (void __user *)arg,
-							sizeof(buf_cfg)))
+							sizeof(buf_cfg))) {
 			r = -EFAULT;
-		else
-			r = cs_hsi_buf_config(csdata->hi, &buf_cfg);
+			break;
+		}
+		hi = cs_char_hsi_get(csdata);
+		if (!hi)
+			return -ENODEV;
+		r = cs_hsi_buf_config(hi, &buf_cfg);
 
 		break;
 	}
@@ -1334,10 +1361,16 @@ static void cs_free_char_queue(struct list_head *head)
 static int cs_char_release(struct inode *unused, struct file *file)
 {
 	struct cs_char *csdata = file->private_data;
+	struct cs_hsi_iface *hi;
 
-	cs_hsi_stop(csdata->hi);
 	spin_lock_bh(&csdata->lock);
+	hi = csdata->hi;
 	csdata->hi = NULL;
+	spin_unlock_bh(&csdata->lock);
+
+	if (hi)
+		cs_hsi_stop(hi);
+	spin_lock_bh(&csdata->lock);
 	free_page(csdata->mmap_base);
 	cs_free_char_queue(&csdata->chardev_queue);
 	cs_free_char_queue(&csdata->dataind_queue);

---
base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
change-id: 20260827-cmt-speech-uaf-df2c7577ce7e

Best regards,
--  
Shengzhuo Wei <me@cherr.cc>

Re: [PATCH] HSI: cmt_speech: Fix crash on device removal with open file descriptor

[Greg KH]

On Fri, Aug 28, 2026 at 04:09:27PM +0800, Shengzhuo Wei wrote:
> misc_deregister() does not drain file descriptors that are already
> open, so after cs_hsi_client_remove() clears and frees
> cs_char_data.hi, the ioctl and write paths still dereference it
> without any lock or NULL check:
> 
>   open("/dev/cmt_speech")                          [userspace]
>   rmmod cmt_speech                                 [removal]
>   ioctl(fd, CS_GET_STATE, 0)                       [userspace]
> 
> Fix it by reading cs_char_data.hi under cs_char_data.lock and
> rejecting calls with -ENODEV once the interface is gone;
> cs_char_release() now steals the pointer under the lock like
> cs_hsi_client_remove() does, so the interface is stopped exactly
> once between them.
> 
> Fixes: 7f62fe8a5851 ("HSI: cmt_speech: Add cmt-speech driver")
> Cc: stable@vger.kernel.org
> 
> Assisted-by: GLM:5.3
> Signed-off-by: Shengzhuo Wei <me@cherr.cc>

No blank line above the assisted-by line.


> ---
> Verified on a KASAN kernel with a software HSI controller: opening the
> character device, unbinding the client and issuing the ioctl gives

why are you unbinding?  You do know that will taint the kernel in the
future, and this is not a "normal" operation that a user could do,
right?

> 
>   KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
>   RIP: 0010:cs_char_ioctl+0x29e/0x340
> 
> and with the patch applied the same sequence returns -ENODEV.
> ---
>  drivers/hsi/clients/cmt_speech.c | 47 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 40 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hsi/clients/cmt_speech.c b/drivers/hsi/clients/cmt_speech.c
> index 7226677ebde7..4325152a5a62 100644
> --- a/drivers/hsi/clients/cmt_speech.c
> +++ b/drivers/hsi/clients/cmt_speech.c
> @@ -732,6 +732,17 @@ static int cs_hsi_write_on_data(struct cs_hsi_iface *hi, unsigned int slot)
>  	return ret;
>  }
>  
> +static struct cs_hsi_iface *cs_char_hsi_get(struct cs_char *csdata)
> +{
> +	struct cs_hsi_iface *hi;
> +
> +	spin_lock_bh(&csdata->lock);
> +	hi = csdata->hi;
> +	spin_unlock_bh(&csdata->lock);

That is not a normal "get" type operation.

Why are you not using the proper reference counting structures that you
already have here?

thanks,

greg k-h