[PATCH] ARM: sa1111: fix put_device() before device_add() in error path

[PATCH] ARM: sa1111: fix put_device() before device_add() in error path

[Alfredo Cruz]

When request_resource() fails, a jump is made to err_resource label which
calls put_device() on &dev->res without adding it first.
Kept the err_resource label and moved put_device() to err_add label.

Signed-off-by: Alfredo Cruz <alfredo.carlon@gmail.com>
---
 arch/arm/common/sa1111.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/common/sa1111.c b/arch/arm/common/sa1111.c
index 1fbd7363cf11..76bf15da5115 100644
--- a/arch/arm/common/sa1111.c
+++ b/arch/arm/common/sa1111.c
@@ -779,8 +779,8 @@ sa1111_init_one_child(struct sa1111 *sachip, struct resource *parent,
 
  err_add:
 	release_resource(&dev->res);
- err_resource:
 	put_device(&dev->dev);
+ err_resource:
  err_alloc:
 	return ret;
 }
-- 
2.43.2

Re: [PATCH] ARM: sa1111: fix put_device() before device_add() in error path

[Russell King (Oracle)]

On Wed, Feb 21, 2024 at 10:38:37PM +0100, Alfredo Cruz wrote:
> When request_resource() fails, a jump is made to err_resource label which
> calls put_device() on &dev->res without adding it first.
> Kept the err_resource label and moved put_device() to err_add label.

You fail to explain why this is incorrect. I think this patch is
incorrect. The reason is as follows:

"dev" is allocated.
dev->dev is then initialised. This initialises the struct device,
setting its initial refcount to '1'.

We then request the resource. If this fails, then the device needs to
be released. As refcounting has been initialised, it is *not*
appropriate to simply kfree() "dev". The correct action, as noted by
the device_initialize() documentation is to call put_device() on it.
This is what the code does.

I _do_ hope you haven't submitted other patches like this, because if
you have, they could well also be wrong.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!