Re: [PATCH v2] ACPI / battery: Safe unregistering of hooks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wednesday, July 4, 2018 12:27:15 PM CEST Jouke Witteveen wrote:
> A hooking API was implemented for 4.17 in fa93854f7a7ed63d followed by
> hooks for Thinkpad laptops in 2801b9683f740012. The Thinkpad drivers
> did not support the Thinkpad 13 and the hooking API crashes on
> unsupported batteries by altering a list of hooks during unsafe
> iteration. Thus, Thinkpad 13 laptops could no longer boot.
> 
> Additionally, a lock was kept in place and debugging information was
> printed out of order.
> 
> Signed-off-by: Jouke Witteveen <j.witteveen@xxxxxxxxx>
> ---
> 
> This time with a more informative commit message (no functional changes).
> 
>  drivers/acpi/battery.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index b0113a58..d79ad844 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -717,10 +717,11 @@ void battery_hook_register(struct acpi_battery_hook *hook)
>  			 */
>  			pr_err("extension failed to load: %s", hook->name);
>  			__battery_hook_unregister(hook, 0);
> -			return;
> +			goto end;
>  		}
>  	}
>  	pr_info("new extension: %s\n", hook->name);
> +end:
>  	mutex_unlock(&hook_mutex);
>  }
>  EXPORT_SYMBOL_GPL(battery_hook_register);
> @@ -732,7 +733,7 @@ EXPORT_SYMBOL_GPL(battery_hook_register);
>  */
>  static void battery_hook_add_battery(struct acpi_battery *battery)
>  {
> -	struct acpi_battery_hook *hook_node;
> +	struct acpi_battery_hook *hook_node, *tmp;
>  
>  	mutex_lock(&hook_mutex);
>  	INIT_LIST_HEAD(&battery->list);
> @@ -744,15 +745,15 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
>  	 * when a battery gets hotplugged or initialized
>  	 * during the battery module initialization.
>  	 */
> -	list_for_each_entry(hook_node, &battery_hook_list, list) {
> +	list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
>  		if (hook_node->add_battery(battery->bat)) {
>  			/*
>  			 * The notification of the extensions has failed, to
>  			 * prevent further errors we will unload the extension.
>  			 */
> -			__battery_hook_unregister(hook_node, 0);
>  			pr_err("error in extension, unloading: %s",
>  					hook_node->name);
> +			__battery_hook_unregister(hook_node, 0);
>  		}
>  	}
>  	mutex_unlock(&hook_mutex);
> 

Applied and pushed for 4.18-rc4.

Thanks!


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux