[PATCH] powerpc/pseries/cmm: fix the error handling in cmm_sysfs_register()

[PATCH] powerpc/pseries/cmm: fix the error handling in cmm_sysfs_register()

[Haoxiang Li]

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@vger.kernel.org
Signed-off-by: Haoxiang Li <lihaoxiang@isrc.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;
 }
-- 
2.25.1

Re: [PATCH] powerpc/pseries/cmm: fix the error handling in cmm_sysfs_register()

[Greg KH]

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@vger.kernel.org
> Signed-off-by: Haoxiang Li <lihaoxiang@isrc.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