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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 30 Mar 2016 19:25:49 -0400
From:	Javier Martinez Canillas <javier@....samsung.com>
To:	Jon Hunter <jonathanh@...dia.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>
Cc:	linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH] regulator: Fix deadlock during regulator registration

Hello Jon,

On 03/30/2016 12:09 PM, Jon Hunter wrote:
> Commit 5e3ca2b349b1 ("regulator: Try to resolve regulators supplies on
> registration") added a call to regulator_resolve_supply() within
> regulator_register() where the regulator_list_mutex is held. This causes
> the following deadlock to occur on the Tegra114 Dalmore board when the
> palmas PMIC is registered because regulator_register_resolve_supply()
> calls regulator_dev_lookup() which may try to acquire the
> regulator_list_mutex again.
>

Sorry for missing that. I didn't notice because on my machine the regulators
are looked up using OF and in that case the regulator_list_mutex isn't grabbed.

I believe your patch is correct, I have just one trivial comment below:

>  
> @@ -4016,15 +4015,16 @@ scrub:
>  	regulator_ena_gpio_free(rdev);
>  	device_unregister(&rdev->dev);
>  	/* device core frees rdev */
> -	rdev = ERR_PTR(ret);
>  	goto out;
>  
>  wash:
>  	regulator_ena_gpio_free(rdev);
>  clean:
>  	kfree(rdev);
> -	rdev = ERR_PTR(ret);

You are doing some cleanup of the clean and scrub error paths by removing
rdev and returning ERR_PTR(ret) directly. I believe that should be in a
separate patch since is not related to the fix.

> -	goto out;
> +out:
> +	mutex_unlock(&regulator_list_mutex);
> +	kfree(config);
> +	return ERR_PTR(ret);
>  }
>  EXPORT_SYMBOL_GPL(regulator_register);
>  
> 

If you split the cleanup and address Mark's comments, feel free to add:

Reviewed-by: Javier Martinez Canillas <javier@....samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Powered by blists - more mailing lists