linux-kernel - Re: [PATCH] regulator: Fix deadlock during regulator registration

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]

Message-ID: <56FC607D.3020108@osg.samsung.com>
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