linux-kernel - Re: [PATCH] powerpc/pseries/cmm: fix the error handling in cmm_sysfs

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 for Android: free password hash cracker in your pocket

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

Message-ID: <2025122248-obsession-urgent-648f@gregkh>
Date: Mon, 22 Dec 2025 07:20:01 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Haoxiang Li <lihaoxiang@...c.iscas.ac.cn>
Cc: maddy@...ux.ibm.com, mpe@...erman.id.au, npiggin@...il.com,
	chleroy@...nel.org, akpm@...ux-foundation.org, david@...nel.org,
	ritesh.list@...il.com, byungchul@...com, abarnas@...gle.com,
	kay.sievers@...y.org, linuxppc-dev@...ts.ozlabs.org,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] powerpc/pseries/cmm: fix the error handling in
 cmm_sysfs_register()

On Mon, Dec 22, 2025 at 11:12:25AM +0800, Haoxiang Li wrote:
> If device_register() fails, put_device() should be called to drop
> the device reference.
> Thus add put_device() after subsys_unregister label and change
> device_unregister() to device_del() in fail label.
> 
> Found by code review.
> 
> Fixes: 6c9d29095264 ("power: cmm - convert sysdev_class to a regular subsystem")
> Cc: stable@...r.kernel.org
> Signed-off-by: Haoxiang Li <lihaoxiang@...c.iscas.ac.cn>
> ---
>  arch/powerpc/platforms/pseries/cmm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/cmm.c b/arch/powerpc/platforms/pseries/cmm.c
> index 4cbbe2ee58ab..0666d3300bdb 100644
> --- a/arch/powerpc/platforms/pseries/cmm.c
> +++ b/arch/powerpc/platforms/pseries/cmm.c
> @@ -419,8 +419,9 @@ static int cmm_sysfs_register(struct device *dev)
>  fail:
>  	while (--i >= 0)
>  		device_remove_file(dev, cmm_attrs[i]);
> -	device_unregister(dev);
> +	device_del(dev);
>  subsys_unregister:
> +	put_device(dev);
>  	bus_unregister(&cmm_subsys);
>  	return rc;
>  }

this does not look to be correct, how was it tested?

Also, why not fix all of this up properly by calling the correct
functions so that you don't have to manually add/remove the sysfs files?
That would resolve the "problem" you seem to think is here in a much
simpler way, and also fix the real bug in this function (i.e. it races
with userspace and looses.)

thanks,

greg k-h