[PATCH] comedi: fix device lifecycle handling in comedi_dev_kref_release()

[PATCH] comedi: fix device lifecycle handling in comedi_dev_kref_release()

[Salah Triki]

`comedi_dev_kref_release()` calls `put_device()` on `dev->class_dev` and
then immediately frees the `comedi_device` structure with `kfree()`.
If the device reference count reaches zero, the device model release
callback may be invoked, potentially accessing or freeing parts of
the structure. Calling `kfree()` directly after `put_device()` can
cause a use-after-free or double-free.

After `device_initialize()` and once the struct device is in the device
model, all cleanup must go through `put_device()` and the release
callback. The kref release should not free the structure directly,
ensuring that `comedi_device` lifetime is properly managed through
device reference counting and preventing potential memory corruption.

Fixes: 5b13ed94a7d24 ("staging: comedi: add a kref to comedi device")

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 drivers/comedi/comedi_fops.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/comedi/comedi_fops.c b/drivers/comedi/comedi_fops.c
index 2c3eb9e89571..24bb06fe6b45 100644
--- a/drivers/comedi/comedi_fops.c
+++ b/drivers/comedi/comedi_fops.c
@@ -118,7 +118,6 @@ static void comedi_dev_kref_release(struct kref *kref)
 
 	mutex_destroy(&dev->mutex);
 	put_device(dev->class_dev);
-	kfree(dev);
 }
 
 /**
-- 
2.43.0

Re: [PATCH] comedi: fix device lifecycle handling in comedi_dev_kref_release()

[Ian Abbott]

On 30/01/2026 22:35, Salah Triki wrote:
> `comedi_dev_kref_release()` calls `put_device()` on `dev->class_dev` and
> then immediately frees the `comedi_device` structure with `kfree()`.
> If the device reference count reaches zero, the device model release
> callback may be invoked, potentially accessing or freeing parts of
> the structure. Calling `kfree()` directly after `put_device()` can
> cause a use-after-free or double-free.
> 
> After `device_initialize()` and once the struct device is in the device
> model, all cleanup must go through `put_device()` and the release
> callback. The kref release should not free the structure directly,
> ensuring that `comedi_device` lifetime is properly managed through
> device reference counting and preventing potential memory corruption.
> 
> Fixes: 5b13ed94a7d24 ("staging: comedi: add a kref to comedi device")
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>   drivers/comedi/comedi_fops.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/comedi/comedi_fops.c b/drivers/comedi/comedi_fops.c
> index 2c3eb9e89571..24bb06fe6b45 100644
> --- a/drivers/comedi/comedi_fops.c
> +++ b/drivers/comedi/comedi_fops.c
> @@ -118,7 +118,6 @@ static void comedi_dev_kref_release(struct kref *kref)
>   
>   	mutex_destroy(&dev->mutex);
>   	put_device(dev->class_dev);
> -	kfree(dev);
>   }
>   
>   /**

NAK

This isn't freeing the `struct device`, it is freeing the `struct 
comedi_device`.  The `struct device` will be freed by 
`device_create_release()` in "drivers/base/core.c".

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-

Re: [PATCH] comedi: fix device lifecycle handling in comedi_dev_kref_release()

[Ian Abbott]

On 02/02/2026 12:09, Ian Abbott wrote:
> On 30/01/2026 22:35, Salah Triki wrote:
>> `comedi_dev_kref_release()` calls `put_device()` on `dev->class_dev` and
>> then immediately frees the `comedi_device` structure with `kfree()`.
>> If the device reference count reaches zero, the device model release
>> callback may be invoked, potentially accessing or freeing parts of
>> the structure. Calling `kfree()` directly after `put_device()` can
>> cause a use-after-free or double-free.
>>
>> After `device_initialize()` and once the struct device is in the device
>> model, all cleanup must go through `put_device()` and the release
>> callback. The kref release should not free the structure directly,
>> ensuring that `comedi_device` lifetime is properly managed through
>> device reference counting and preventing potential memory corruption.
>>
>> Fixes: 5b13ed94a7d24 ("staging: comedi: add a kref to comedi device")
>>
>> Signed-off-by: Salah Triki <salah.triki@gmail.com>
>> ---
>>   drivers/comedi/comedi_fops.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/comedi/comedi_fops.c b/drivers/comedi/comedi_fops.c
>> index 2c3eb9e89571..24bb06fe6b45 100644
>> --- a/drivers/comedi/comedi_fops.c
>> +++ b/drivers/comedi/comedi_fops.c
>> @@ -118,7 +118,6 @@ static void comedi_dev_kref_release(struct kref 
>> *kref)
>>       mutex_destroy(&dev->mutex);
>>       put_device(dev->class_dev);
>> -    kfree(dev);
>>   }
>>   /**
> 
> NAK
> 
> This isn't freeing the `struct device`, it is freeing the `struct 
> comedi_device`.  The `struct device` will be freed by 
> `device_create_release()` in "drivers/base/core.c".

To clarify, the call to `put_device()` from `comedi_dev_kref_release()` 
matches the call to `get_device()` after a successful call to 
`device_create()` from `comedi_alloc_board_minor()`.

The counterpart to `comedi_alloc_board_minor()` is 
`comedi_free_board_dev()` which calls `device_destroy()` (matching the 
call to `device_create()`), followed by a call to `comedi_dev_put()`. 
`comedi_dev_put()` will call `comedi_dev_kref_release()` via 
`kref_put()` when its reference count reaches zero, and as mentioned 
previously, its call to `put_device()` matches the previous call to 
`get_device()` after the device was created.  So there are no hooks back 
to the comedi module code or the `struct comedi_device` at that point 
because the `struct device` has already been removed from the system.

The reason for the extra calls to `get_device()` and `put_device()` is 
explained by commit 8f988d8784e3 ("staging/comedi: keep reference to 
class device after destroyed").

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-

Re: [PATCH] comedi: fix device lifecycle handling in comedi_dev_kref_release()

[Salah Triki]

On Mon, Feb 02, 2026 at 03:12:29PM +0000, Ian Abbott wrote:
> On 02/02/2026 12:09, Ian Abbott wrote:
> > NAK
> > 
> > This isn't freeing the `struct device`, it is freeing the `struct
> > comedi_device`.  The `struct device` will be freed by
> > `device_create_release()` in "drivers/base/core.c".
> 
> To clarify, the call to `put_device()` from `comedi_dev_kref_release()`
> matches the call to `get_device()` after a successful call to
> `device_create()` from `comedi_alloc_board_minor()`.
> 
> The counterpart to `comedi_alloc_board_minor()` is `comedi_free_board_dev()`
> which calls `device_destroy()` (matching the call to `device_create()`),
> followed by a call to `comedi_dev_put()`. `comedi_dev_put()` will call
> `comedi_dev_kref_release()` via `kref_put()` when its reference count
> reaches zero, and as mentioned previously, its call to `put_device()`
> matches the previous call to `get_device()` after the device was created.
> So there are no hooks back to the comedi module code or the `struct
> comedi_device` at that point because the `struct device` has already been
> removed from the system.
> 
> The reason for the extra calls to `get_device()` and `put_device()` is
> explained by commit 8f988d8784e3 ("staging/comedi: keep reference to class
> device after destroyed").
> 
> -- 
> -=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
> -=( registered in England & Wales.  Regd. number: 02862268.  )=-
> -=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
> -=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-

Thanks for the detailed explanation, that makes sense. I’ll drop this patch.

Best regards,
Salah