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

[Greg KH <gregkh@linuxfoundation.org>]

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