Re: [PATCH 1/1] ACPI: platform-profile: Fix possible deadlock in platform_profile_remove()

["Rafael J. Wysocki" <rafael@kernel.org>]

On Tue, Jan 26, 2021 at 3:10 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> After a rmmod thinkpad_acpi, lockdep pointed out this possible deadlock:
>
> Our _show and _store sysfs attr functions get called with the kn->active
> lock held for the sysfs attr and then take the profile_lock.
> sysfs_remove_group() also takes the kn->active lock for the sysfs attr,
> so if we call it with the profile_lock held, then we get an ABBA deadlock.
>
> platform_profile_remove() must only be called by drivers which have
> first *successfully* called platform_profile_register(). Anything else
> is a driver bug. So the check for cur_profile being set before calling
> sysfs_remove_group() is not necessary and it can be dropped.
>
> It is safe to call sysfs_remove_group() without holding the profile_lock
> since the attr-group group cannot be re-added until after we clear
> cur_profile.
>
> Change platform_profile_remove() to only hold the profile_lock while
> clearing the cur_profile, fixing the deadlock.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied on top of the previous platform-profile material, thanks!

> ---
>  drivers/acpi/platform_profile.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> index 80e9df427eb8..4a59c5993bde 100644
> --- a/drivers/acpi/platform_profile.c
> +++ b/drivers/acpi/platform_profile.c
> @@ -164,13 +164,9 @@ EXPORT_SYMBOL_GPL(platform_profile_register);
>
>  int platform_profile_remove(void)
>  {
> -       mutex_lock(&profile_lock);
> -       if (!cur_profile) {
> -               mutex_unlock(&profile_lock);
> -               return -ENODEV;
> -       }
> -
>         sysfs_remove_group(acpi_kobj, &platform_profile_group);
> +
> +       mutex_lock(&profile_lock);
>         cur_profile = NULL;
>         mutex_unlock(&profile_lock);
>         return 0;
> --
> 2.29.2
>