1. Patchew
  2. linux
  3. [PATCH v5 0/4] uacce: driver fixes for memory leaks and state management
  4. View patch

[PATCH v5 1/4] uacce: fix cdev handling in register and remove paths

Chenghai Huang posted 4 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH v5 1/4] uacce: fix cdev handling in register and remove paths
Posted by Chenghai Huang 2 months, 4 weeks ago 
From: Wenkai Lin <linwenkai6@hisilicon.com>

This patch addresses a potential issue in the uacce driver where the
cdev was not properly managed during error handling and cleanup paths.

Changes made:
1. In uacce_register(), store the return value of cdev_device_add()
   and clear the cdev owner as a flag if registration fails.
2. In uacce_remove(), add additional check for cdev owner before
   calling cdev_device_del() to prevent potential issues.

Fixes: 015d239ac014 ("uacce: add uacce driver")
Cc: stable@vger.kernel.org
Signed-off-by: Wenkai Lin <linwenkai6@hisilicon.com>
Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
---
 drivers/misc/uacce/uacce.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
index 42e7d2a2a90c..688050c35d88 100644
--- a/drivers/misc/uacce/uacce.c
+++ b/drivers/misc/uacce/uacce.c
@@ -519,6 +519,8 @@ EXPORT_SYMBOL_GPL(uacce_alloc);
  */
 int uacce_register(struct uacce_device *uacce)
 {
+	int ret;
+
 	if (!uacce)
 		return -ENODEV;
 
@@ -529,7 +531,11 @@ int uacce_register(struct uacce_device *uacce)
 	uacce->cdev->ops = &uacce_fops;
 	uacce->cdev->owner = THIS_MODULE;
 
-	return cdev_device_add(uacce->cdev, &uacce->dev);
+	ret = cdev_device_add(uacce->cdev, &uacce->dev);
+	if (ret)
+		uacce->cdev->owner = NULL;
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(uacce_register);
 
@@ -568,7 +574,7 @@ void uacce_remove(struct uacce_device *uacce)
 		unmap_mapping_range(q->mapping, 0, 0, 1);
 	}
 
-	if (uacce->cdev)
+	if (uacce->cdev && uacce->cdev->owner)
 		cdev_device_del(uacce->cdev, &uacce->dev);
 	xa_erase(&uacce_xa, uacce->dev_id);
 	/*
-- 
2.33.0
Re: [PATCH v5 1/4] uacce: fix cdev handling in register and remove paths
Posted by Zhangfei Gao 2 months, 4 weeks ago 
On Tue, 11 Nov 2025 at 17:35, Chenghai Huang <huangchenghai2@huawei.com> wrote:
>
> From: Wenkai Lin <linwenkai6@hisilicon.com>
>
> This patch addresses a potential issue in the uacce driver where the
> cdev was not properly managed during error handling and cleanup paths.
Can we clarify that it was caused by cdev_device_add?

>
> Changes made:
> 1. In uacce_register(), store the return value of cdev_device_add()
>    and clear the cdev owner as a flag if registration fails.
> 2. In uacce_remove(), add additional check for cdev owner before
>    calling cdev_device_del() to prevent potential issues.
>
> Fixes: 015d239ac014 ("uacce: add uacce driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wenkai Lin <linwenkai6@hisilicon.com>
> Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
> ---
>  drivers/misc/uacce/uacce.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
> index 42e7d2a2a90c..688050c35d88 100644
> --- a/drivers/misc/uacce/uacce.c
> +++ b/drivers/misc/uacce/uacce.c
> @@ -519,6 +519,8 @@ EXPORT_SYMBOL_GPL(uacce_alloc);
>   */
>  int uacce_register(struct uacce_device *uacce)
>  {
> +       int ret;
> +
>         if (!uacce)
>                 return -ENODEV;
>
> @@ -529,7 +531,11 @@ int uacce_register(struct uacce_device *uacce)
>         uacce->cdev->ops = &uacce_fops;
>         uacce->cdev->owner = THIS_MODULE;
>
> -       return cdev_device_add(uacce->cdev, &uacce->dev);
> +       ret = cdev_device_add(uacce->cdev, &uacce->dev);
> +       if (ret)
> +               uacce->cdev->owner = NULL;
If uacce is build in, THIS_MODULE = 0.


how about this, handle cdev_device_add fail for no device_add case.

+       if (ret) {
+               if (!device_is_registered(&uacce->dev)) {
+                       cdev_del(uacce->cdev);
+                       uacce->cdev = NULL;
+               }
+       }



> +
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(uacce_register);
>
> @@ -568,7 +574,7 @@ void uacce_remove(struct uacce_device *uacce)
>                 unmap_mapping_range(q->mapping, 0, 0, 1);
>         }
>
> -       if (uacce->cdev)
> +       if (uacce->cdev && uacce->cdev->owner)
>                 cdev_device_del(uacce->cdev, &uacce->dev);
is there an potential issue, uacce->cdev is not freed?

Thanks
Re: [PATCH v5 1/4] uacce: fix cdev handling in register and remove paths
Posted by Zhangfei Gao 2 months, 4 weeks ago 
.

On Wed, 12 Nov 2025 at 17:46, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:
>
> On Tue, 11 Nov 2025 at 17:35, Chenghai Huang <huangchenghai2@huawei.com> wrote:
> >
> > From: Wenkai Lin <linwenkai6@hisilicon.com>
> >
> > This patch addresses a potential issue in the uacce driver where the
> > cdev was not properly managed during error handling and cleanup paths.
> Can we clarify that it was caused by cdev_device_add?
>
> >
> > Changes made:
> > 1. In uacce_register(), store the return value of cdev_device_add()
> >    and clear the cdev owner as a flag if registration fails.
> > 2. In uacce_remove(), add additional check for cdev owner before
> >    calling cdev_device_del() to prevent potential issues.
> >
> > Fixes: 015d239ac014 ("uacce: add uacce driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Wenkai Lin <linwenkai6@hisilicon.com>
> > Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
> > ---
> >  drivers/misc/uacce/uacce.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
> > index 42e7d2a2a90c..688050c35d88 100644
> > --- a/drivers/misc/uacce/uacce.c
> > +++ b/drivers/misc/uacce/uacce.c
> > @@ -519,6 +519,8 @@ EXPORT_SYMBOL_GPL(uacce_alloc);
> >   */
> >  int uacce_register(struct uacce_device *uacce)
> >  {
> > +       int ret;
> > +
> >         if (!uacce)
> >                 return -ENODEV;
> >
> > @@ -529,7 +531,11 @@ int uacce_register(struct uacce_device *uacce)
> >         uacce->cdev->ops = &uacce_fops;
> >         uacce->cdev->owner = THIS_MODULE;
> >
> > -       return cdev_device_add(uacce->cdev, &uacce->dev);
> > +       ret = cdev_device_add(uacce->cdev, &uacce->dev);
> > +       if (ret)
> > +               uacce->cdev->owner = NULL;
> If uacce is build in, THIS_MODULE = 0.
>
>
> how about this, handle cdev_device_add fail for no device_add case.
>
> +       if (ret) {
> +               if (!device_is_registered(&uacce->dev)) {
> +                       cdev_del(uacce->cdev);
> +                       uacce->cdev = NULL;
> +               }
> +       }
>
>
>
> > +
> > +       return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(uacce_register);
> >
> > @@ -568,7 +574,7 @@ void uacce_remove(struct uacce_device *uacce)
> >                 unmap_mapping_range(q->mapping, 0, 0, 1);
> >         }
> >
> > -       if (uacce->cdev)
> > +       if (uacce->cdev && uacce->cdev->owner)
> >                 cdev_device_del(uacce->cdev, &uacce->dev);
> is there an potential issue, uacce->cdev is not freed?

And cdev_device_add(uacce->cdev, &uacce->dev) will not add refcount to
&uacce->dev
as usual when failing beforehand, so uacce_remove:
put_device(&uacce->dev) will have
refcount_t: underflow; use-after-free, need check as well.

@@ -558,8 +558,12 @@ int uacce_register(struct uacce_device *uacce)
        uacce->cdev->owner = THIS_MODULE;

        ret = cdev_device_add(uacce->cdev, &uacce->dev);
-       if (ret)
-               uacce->cdev->owner = NULL;
+       if (ret) {
+               if (!device_is_registered(&uacce->dev)) {
+                       kobject_put(&uacce->cdev->kobj);
+                       uacce->cdev = NULL;
+               }
+       }
        return ret;
 }

@@ -610,7 +614,8 @@ void uacce_remove(struct uacce_device *uacce)
        uacce->ops = NULL;
        uacce->parent = NULL;
        mutex_unlock(&uacce->mutex);
-       put_device(&uacce->dev);
+       if (device_is_registered(&uacce->dev))
+               put_device(&uacce->dev);
 }

Just one possible solution, FYI.

Thanks

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.