[PATCH] remoteproc: core: Do not put the device when device_add() fails

[PATCH] remoteproc: core: Do not put the device when device_add() fails

[Yuqiang Sun]

When device_add() fails, rproc_add() calls put_device() and then
removes the character device.  The reference it drops is the one
rproc_alloc() took with device_initialize(), which belongs to the
caller: after a failed rproc_add(), every driver frees the rproc,
either with rproc_free() or through devm_rproc_alloc(), and both drop
that reference again.

So the rproc is freed inside rproc_add(): by the put_device() itself,
or, with CONFIG_REMOTEPROC_CDEV, by the cdev_del() that follows, when
the character device drops its reference on the parent device.  The
caller then uses the freed rproc and puts it a second time.

With one allocation failure injected into device_add() (failslab
restricted to rproc_add) while st_fdma probes, KASAN reports:

  BUG: KASAN: slab-use-after-free in slim_clk_disable+0x38/0xf4 [st_slim_rproc]
  Read of size 8 by task insmod/69
  Call trace:
   slim_clk_disable
   st_slim_rproc_alloc
   st_fdma_probe
  Freed by task 69:
   kfree
   rproc_type_release
   device_release
   kobject_put
   cdev_default_release
   kobject_put
   cdev_del
   rproc_char_device_remove
   rproc_add
   st_slim_rproc_alloc

followed by use-after-free reports in kobject_put() and
refcount_warn_saturate() from the rproc_free() in
st_slim_rproc_alloc().

Remove the put_device(): the caller's rproc_free() drops the reference
and releases the rproc, as it does for the other failures in
rproc_add().

The problem was found with kbound, a static analysis prototype that
checks whether callers rely on an object after a callee returns a value
for which the callee may have released it, on v7.2.12; it is present
in rproc-next.

Build-tested with x86_64 and arm64 allmodconfig (W=1, sparse): no new
warnings.  Run-tested on QEMU's arm64 virt machine (TCG) with KASAN,
using a device tree node for an ST FDMA with a SLIM core, so that
st_fdma_probe() reaches rproc_add() without the hardware (no clocks,
MMIO windows only mapped), and failing each allocation in rproc_add()
in turn with failslab: before this patch every failure in device_add()
(9 of 9) gives the reports above; with it nothing is reported, and the
rproc is released once, by rproc_free().  Not tested on remoteproc
hardware.

Fixes: 7dbdb8bd7c02 ("remoteproc: core: Cleanup device in case of failure")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5 kbound
Signed-off-by: Yuqiang Sun <izaiahsun@gmail.com>
---
 drivers/remoteproc/remoteproc_core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index b88bf179507d..e4f44a3c8b4d 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -2309,10 +2309,8 @@ int rproc_add(struct rproc *rproc)
 		return ret;
 
 	ret = device_add(dev);
-	if (ret < 0) {
-		put_device(dev);
+	if (ret < 0)
 		goto rproc_remove_cdev;
-	}
 
 	dev_info(dev, "%s is available\n", rproc->name);
 

base-commit: 82d0bbd67c42a44f0e18b0b7b0da0f3ace0c9527
-- 
2.53.0

Re: [PATCH] remoteproc: core: Do not put the device when device_add() fails

[Mathieu Poirier]

On Thu, Oct 29, 2026 at 09:00:37PM +0800, Yuqiang Sun wrote:
> When device_add() fails, rproc_add() calls put_device() and then
> removes the character device.  The reference it drops is the one
> rproc_alloc() took with device_initialize(), which belongs to the
> caller: after a failed rproc_add(), every driver frees the rproc,
> either with rproc_free() or through devm_rproc_alloc(), and both drop
> that reference again.
> 
> So the rproc is freed inside rproc_add(): by the put_device() itself,
> or, with CONFIG_REMOTEPROC_CDEV, by the cdev_del() that follows, when
> the character device drops its reference on the parent device.  The
> caller then uses the freed rproc and puts it a second time.
> 
> With one allocation failure injected into device_add() (failslab
> restricted to rproc_add) while st_fdma probes, KASAN reports:
> 
>   BUG: KASAN: slab-use-after-free in slim_clk_disable+0x38/0xf4 [st_slim_rproc]
>   Read of size 8 by task insmod/69
>   Call trace:
>    slim_clk_disable
>    st_slim_rproc_alloc
>    st_fdma_probe
>   Freed by task 69:
>    kfree
>    rproc_type_release
>    device_release
>    kobject_put
>    cdev_default_release
>    kobject_put
>    cdev_del
>    rproc_char_device_remove
>    rproc_add
>    st_slim_rproc_alloc
> 
> followed by use-after-free reports in kobject_put() and
> refcount_warn_saturate() from the rproc_free() in
> st_slim_rproc_alloc().
> 
> Remove the put_device(): the caller's rproc_free() drops the reference
> and releases the rproc, as it does for the other failures in
> rproc_add().
> 
> The problem was found with kbound, a static analysis prototype that
> checks whether callers rely on an object after a callee returns a value
> for which the callee may have released it, on v7.2.12; it is present
> in rproc-next.
> 
> Build-tested with x86_64 and arm64 allmodconfig (W=1, sparse): no new
> warnings.  Run-tested on QEMU's arm64 virt machine (TCG) with KASAN,
> using a device tree node for an ST FDMA with a SLIM core, so that
> st_fdma_probe() reaches rproc_add() without the hardware (no clocks,
> MMIO windows only mapped), and failing each allocation in rproc_add()
> in turn with failslab: before this patch every failure in device_add()
> (9 of 9) gives the reports above; with it nothing is reported, and the
> rproc is released once, by rproc_free().  Not tested on remoteproc
> hardware.
> 
> Fixes: 7dbdb8bd7c02 ("remoteproc: core: Cleanup device in case of failure")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5 kbound
> Signed-off-by: Yuqiang Sun <izaiahsun@gmail.com>
> ---
>  drivers/remoteproc/remoteproc_core.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index b88bf179507d..e4f44a3c8b4d 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2309,10 +2309,8 @@ int rproc_add(struct rproc *rproc)
>  		return ret;
>  
>  	ret = device_add(dev);
> -	if (ret < 0) {
> -		put_device(dev);

The documentation for device_add() is clear, and looking at the implementation
of device_add(), I understand why put_device() needs to be called here.

Unfortunately I do not have the time to read through and digest what Claude has
generated in the changelog.

Thanks,
Mathieu


> +	if (ret < 0)
>  		goto rproc_remove_cdev;
> -	}
>  
>  	dev_info(dev, "%s is available\n", rproc->name);
>  
> 
> base-commit: 82d0bbd67c42a44f0e18b0b7b0da0f3ace0c9527
> -- 
> 2.53.0
> 

Re: [PATCH] remoteproc: core: Do not put the device when device_add() fails

[Yuqiang Sun]

> Unfortunately I do not have the time to read through and digest what Claude has
> generated in the changelog.

Sorry for the long content.
There is a human-written summary about the bug.

> The documentation for device_add() is clear, and looking at the implementation
> of device_add(), I understand why put_device() needs to be called here.

I agree that as documented in device_add, the put_device must be called
if it fails.

But in st_slim_rproc_alloc() (drivers/remoteproc/st_slim_rproc.c), when
rproc_add() returns the error, the driver calls rproc_free() at its err
label, which is put_device(&rproc->dev). With the put_device() in
rproc_add(), that reference is dropped twice, and the driver uses the
rproc after it is freed.

The same happens in every other caller: xlnx_r5_remoteproc calls
rproc_free() too, and the other 23 use devm_rproc_alloc(), whose
devm_rproc_free() does it.

That is why I did not change device_add() or one caller, but
rproc_add().

If necessary, I can send out the full document and PoC on
how to trigger the KASAN.

Thanks,
Yuqiang

Mathieu Poirier <mathieu.poirier@linaro.org> 于2026年11月4日周三 00:28写道：
>
> On Thu, Oct 29, 2026 at 09:00:37PM +0800, Yuqiang Sun wrote:
> > When device_add() fails, rproc_add() calls put_device() and then
> > removes the character device.  The reference it drops is the one
> > rproc_alloc() took with device_initialize(), which belongs to the
> > caller: after a failed rproc_add(), every driver frees the rproc,
> > either with rproc_free() or through devm_rproc_alloc(), and both drop
> > that reference again.
> >
> > So the rproc is freed inside rproc_add(): by the put_device() itself,
> > or, with CONFIG_REMOTEPROC_CDEV, by the cdev_del() that follows, when
> > the character device drops its reference on the parent device.  The
> > caller then uses the freed rproc and puts it a second time.
> >
> > With one allocation failure injected into device_add() (failslab
> > restricted to rproc_add) while st_fdma probes, KASAN reports:
> >
> >   BUG: KASAN: slab-use-after-free in slim_clk_disable+0x38/0xf4 [st_slim_rproc]
> >   Read of size 8 by task insmod/69
> >   Call trace:
> >    slim_clk_disable
> >    st_slim_rproc_alloc
> >    st_fdma_probe
> >   Freed by task 69:
> >    kfree
> >    rproc_type_release
> >    device_release
> >    kobject_put
> >    cdev_default_release
> >    kobject_put
> >    cdev_del
> >    rproc_char_device_remove
> >    rproc_add
> >    st_slim_rproc_alloc
> >
> > followed by use-after-free reports in kobject_put() and
> > refcount_warn_saturate() from the rproc_free() in
> > st_slim_rproc_alloc().
> >
> > Remove the put_device(): the caller's rproc_free() drops the reference
> > and releases the rproc, as it does for the other failures in
> > rproc_add().
> >
> > The problem was found with kbound, a static analysis prototype that
> > checks whether callers rely on an object after a callee returns a value
> > for which the callee may have released it, on v7.2.12; it is present
> > in rproc-next.
> >
> > Build-tested with x86_64 and arm64 allmodconfig (W=1, sparse): no new
> > warnings.  Run-tested on QEMU's arm64 virt machine (TCG) with KASAN,
> > using a device tree node for an ST FDMA with a SLIM core, so that
> > st_fdma_probe() reaches rproc_add() without the hardware (no clocks,
> > MMIO windows only mapped), and failing each allocation in rproc_add()
> > in turn with failslab: before this patch every failure in device_add()
> > (9 of 9) gives the reports above; with it nothing is reported, and the
> > rproc is released once, by rproc_free().  Not tested on remoteproc
> > hardware.
> >
> > Fixes: 7dbdb8bd7c02 ("remoteproc: core: Cleanup device in case of failure")
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:claude-opus-5-5 kbound
> > Signed-off-by: Yuqiang Sun <izaiahsun@gmail.com>
> > ---
> >  drivers/remoteproc/remoteproc_core.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> > index b88bf179507d..e4f44a3c8b4d 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -2309,10 +2309,8 @@ int rproc_add(struct rproc *rproc)
> >               return ret;
> >
> >       ret = device_add(dev);
> > -     if (ret < 0) {
> > -             put_device(dev);
>
> The documentation for device_add() is clear, and looking at the implementation
> of device_add(), I understand why put_device() needs to be called here.
>
> Unfortunately I do not have the time to read through and digest what Claude has
> generated in the changelog.
>
> Thanks,
> Mathieu
>
>
> > +     if (ret < 0)
> >               goto rproc_remove_cdev;
> > -     }
> >
> >       dev_info(dev, "%s is available\n", rproc->name);
> >
> >
> > base-commit: 82d0bbd67c42a44f0e18b0b7b0da0f3ace0c9527
> > --
> > 2.53.0
> >