linux-kernel - Re: [PATCH] ARM: sa1111: fix put_device() before device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZdZxsrhLNCt+bCOB@shell.armlinux.org.uk>
Date: Wed, 21 Feb 2024 21:57:06 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Alfredo Cruz <alfredo.carlon@...il.com>
Cc: arnd@...db.de, u.kleine-koenig@...gutronix.de, shawnguo@...nel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ARM: sa1111: fix put_device() before device_add() in
 error path

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!