linux-kernel - Re: [PATCH] powerpc/fsl_rio: fix a improper release in fsl_rio

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: <aeab7b8c-cc9a-4db8-a36b-f54b842d3c18@kernel.org>
Date: Sat, 20 Dec 2025 19:01:32 +0100
From: "Christophe Leroy (CS GROUP)" <chleroy@...nel.org>
To: Haoxiang Li <lihaoxiang@...c.iscas.ac.cn>
Cc: alex.bou9@...il.com, linux-kernel@...r.kernel.org,
 linuxppc-dev@...ts.ozlabs.org, maddy@...ux.ibm.com, mpe@...erman.id.au,
 mporter@...nel.crashing.org, npiggin@...il.com
Subject: Re: [PATCH] powerpc/fsl_rio: fix a improper release in
 fsl_rio_setup()



Le 20/12/2025 à 14:42, Haoxiang Li a écrit :
> [Vous ne recevez pas souvent de courriers de lihaoxiang@...c.iscas.ac.cn. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Sat, 20 Dec 2025 11:45:10 +0100, Christophe Leroy wrote:
>> Your explanation is unclear. It is correct that put_device() is the
>> correct way to drop the device reference, and it is already what
>> rio_register_mport() does. Why do you need to move it out of
>> rio_register_mport() ? This is what you have to explain.
> 
> Sorry for the unclear changelog. I think if device_register() in
> rio_register_mport() fails, put_device() is already called. Then,
> the error path in fsl_rio_setup() also kfree() the port. I suspected
> a potential double-free issue here, although I am unsure whether this interpretation is correct.

The put_device() in rio_register_mport() releases the reference to 
port->dev taken by device_register(). See the documentation in 
https://elixir.bootlin.com/linux/v6.18/source/drivers/base/core.c#L3750

The kfree(port) in fsl_rio_setup() is to free the memory allocated by 
the kzalloc() at 
https://elixir.bootlin.com/linux/v6.18/source/arch/powerpc/sysdev/fsl_rio.c#L583

Those are two independant things.

> 
>> AI code review or human code review ?
> 
> Human review.
> 
>> Why do you remove this kfree() ? By doing this you will leak the port
>> pointer allocated earlier in the loop by kzalloc()
> 
> I just thought after device_register(), the cleanup should be done by
> put_device().

You are talking about the cleanup of port->dev. Once you have cleaned up 
port->dev you also have to cleanup port.

> 
> I would appreciate your clarification.

See above.

I might be wrong but from my point of view the existing code is correct.

Christophe