linux-kernel - Re: [PATCH 5.4 055/191] mfd: mfd-core: Honour Device Trees request to disable a child-device

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: <20200103102347.GA879@gerhold.net>
Date:   Fri, 3 Jan 2020 11:23:47 +0100
From:   Stephan Gerhold <stephan@...hold.net>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Barry Song <Baohua.Song@....com>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Mark Brown <broonie@...nel.org>,
        Lee Jones <lee.jones@...aro.org>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 5.4 055/191] mfd: mfd-core: Honour Device Trees request
 to disable a child-device

On Thu, Jan 02, 2020 at 11:05:37PM +0100, Greg Kroah-Hartman wrote:
> From: Lee Jones <lee.jones@...aro.org>
> 
> [ Upstream commit 6b5c350648b857047b47acf74a57087ad27d6183 ]
> 
> Until now, MFD has assumed all child devices passed to it (via
> mfd_cells) are to be registered. It does not take into account
> requests from Device Tree and the like to disable child devices
> on a per-platform basis.
> 
> Well now it does.
> 
> Link: https://www.spinics.net/lists/arm-kernel/msg366309.html
> Link: https://lkml.org/lkml/2019/8/22/1350

As far as I understand it, this commit is not suitable for backporting.
The link above explains the problem for a similar patch:

	But I believe this would introduce a rather ugly bug in
	mfd_remove_devices() if the first sub-device is set to disabled:
	It iterates over the children devices to find the base address
	of the allocated "usage count" array, which is then used to free it.
	If the first sub-device is missing, it would free the wrong address.

This problem does not exist in mainline because the MFD usage counting
was removed entirely as part of the larger "Simplify MFD Core" series [1].

None of the device trees in the stable kernels should depend on
disabling MFD devices from the device tree. (They were written at a time
when the MFD core ignored that request entirely...)
Therefore, I would suggest to drop this patch entirely.

[1]: https://lore.kernel.org/lkml/20191101074518.26228-1-lee.jones@linaro.org/

> 
> Reported-by: Barry Song <Baohua.Song@....com>
> Reported-by: Stephan Gerhold <stephan@...hold.net>
> Reviewed-by: Daniel Thompson <daniel.thompson@...aro.org>
> Reviewed-by: Mark Brown <broonie@...nel.org>
> Tested-by: Stephan Gerhold <stephan@...hold.net>
> Signed-off-by: Lee Jones <lee.jones@...aro.org>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>  drivers/mfd/mfd-core.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index 23276a80e3b4..b0afefdc9eac 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -174,6 +174,11 @@ static int mfd_add_device(struct device *parent, int id,
>  	if (parent->of_node && cell->of_compatible) {
>  		for_each_child_of_node(parent->of_node, np) {
>  			if (of_device_is_compatible(np, cell->of_compatible)) {
> +				if (!of_device_is_available(np)) {
> +					/* Ignore disabled devices error free */
> +					ret = 0;
> +					goto fail_alias;
> +				}
>  				pdev->dev.of_node = np;
>  				pdev->dev.fwnode = &np->fwnode;
>  				break;
> -- 
> 2.20.1
> 
> 
>