linux-kernel - Re: [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check

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, 16 Jan 2019 14:20:31 +0100
From:   Thomas Petazzoni <thomas.petazzoni@...tlin.com>
To:     Adrian Hunter <adrian.hunter@...el.com>
Cc:     Kishon Vijay Abraham I <kishon@...com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-tegra@...r.kernel.org,
        Gregory Clement <gregory.clement@...tlin.com>
Subject: Re: [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro()

Hello Adrian,

On Wed, 16 Jan 2019 14:59:32 +0200, Adrian Hunter wrote:

> > The below patch simply changes sdhci_check_ro() to use the value of
> > the WP GPIO if available. We need to adjust the prototype of the
> > function to use a mmc_host* as argument instead of sdhci_host*, since
> > the mmc_can_gpio_ro() and mmc_gpio_get_ro() helpers take a mmc_host*.  
> 
> Why not just use host->mmc

Could do that. I just found it weird that the calling function has the
mmc_host structure, does some gymnastic to find sdhci_host, and then in
the called function, we do the opposite gymnastic to find mmc_host from
sdhci_host. But if that's the preference, I'm happy to change the patch
accordingly.

> >  	if (host->flags & SDHCI_DEVICE_DEAD)
> >  		is_readonly = 0;
> > +	else if (mmc_can_gpio_ro(mmc))
> > +		is_readonly = mmc_gpio_get_ro(mmc);  
> 
> Perhaps host->ops->get_ro should be checked before mmc_can_gpio_ro()?

That is actually a good point, using ->get_ro() should come before
using the GPIO. Indeed, some drivers may potentially have a ->get_ro
with custom logic *and* a WP GPIO, and in this case, we want ->get_ro
to take precedence. I'll send a v2 with this, once you let me know your
decision about the previous point.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com