Re: [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init()

[Brian Norris <briannorris@chromium.org>]

On Thu, Nov 17, 2022 at 08:08:23AM +0000, Yuan Can wrote:
> The following WARNING message was given when rmmod cros_usbpd_notify:
> 
>  Unexpected driver unregister!
>  WARNING: CPU: 0 PID: 253 at drivers/base/driver.c:270 driver_unregister+0x8a/0xb0
>  Modules linked in: cros_usbpd_notify(-)
>  CPU: 0 PID: 253 Comm: rmmod Not tainted 6.1.0-rc3 #24
>  ...
>  Call Trace:
>   <TASK>
>   cros_usbpd_notify_exit+0x11/0x1e [cros_usbpd_notify]
>   __x64_sys_delete_module+0x3c7/0x570
>   ? __ia32_sys_delete_module+0x570/0x570
>   ? lock_is_held_type+0xe3/0x140
>   ? syscall_enter_from_user_mode+0x17/0x50
>   ? rcu_read_lock_sched_held+0xa0/0xd0
>   ? syscall_enter_from_user_mode+0x1c/0x50
>   do_syscall_64+0x37/0x90
>   entry_SYSCALL_64_after_hwframe+0x63/0xcd
>  RIP: 0033:0x7f333fe9b1b7
> 
> The reason is that the cros_usbpd_notify_init() does not check the return
> value of platform_driver_register(), and the cros_usbpd_notify can
> install successfully even if platform_driver_register() failed.
> 
> Fix by checking the return value of platform_driver_register() and
> unregister cros_usbpd_notify_plat_driver when it failed.

On first glance, the last part looked wrong (if register() fails, we
don't need to call unregister()), but these are two different drivers
getting registered. Looks good to me, then.

> Fixes: ec2daf6e33f9 ("platform: chrome: Add cros-usbpd-notify driver")
> Signed-off-by: Yuan Can <yuancan@huawei.com>

Reviewed-by: Brian Norris <briannorris@chromium.org>

> ---
>  drivers/platform/chrome/cros_usbpd_notify.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/chrome/cros_usbpd_notify.c b/drivers/platform/chrome/cros_usbpd_notify.c
> index 4b5a81c9dc6d..10670b6588e3 100644
> --- a/drivers/platform/chrome/cros_usbpd_notify.c
> +++ b/drivers/platform/chrome/cros_usbpd_notify.c
> @@ -239,7 +239,11 @@ static int __init cros_usbpd_notify_init(void)
>  		return ret;
>  
>  #ifdef CONFIG_ACPI
> -	platform_driver_register(&cros_usbpd_notify_acpi_driver);
> +	ret = platform_driver_register(&cros_usbpd_notify_acpi_driver);
> +	if (ret) {
> +		platform_driver_unregister(&cros_usbpd_notify_plat_driver);
> +		return ret;
> +	}
>  #endif
>  	return 0;
>  }
> -- 
> 2.17.1
> 
>