linux-kernel - Re: [PATCH] mmd: core: Terminate infinite loop in SD-UHS voltage switch

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: <20220914101608.GA1325043@roeck-us.net>
Date:   Wed, 14 Sep 2022 03:16:08 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Brian Norris <briannorris@...omium.org>
Cc:     Ulf Hansson <ulf.hansson@...aro.org>, stable@...r.kernel.org,
        Arindam Nath <arindam.nath@....com>,
        Chris Ball <cjb@...top.org>,
        Philip Rakity <prakity@...vell.com>,
        Zhangfei Gao <zhangfei.gao@...vell.com>,
        linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org
Subject: Re: [PATCH] mmd: core: Terminate infinite loop in SD-UHS voltage
 switch

On Tue, Sep 13, 2022 at 06:40:10PM -0700, Brian Norris wrote:
> This loop intends to retry a max of 10 times, with some implicit
> termination based on the SD_{R,}OCR_S18A bit. Unfortunately, the
> termination condition depends on the value reported by the SD card
> (*rocr), which may or may not correctly reflect what we asked it to do.
> 
> Needless to say, it's not wise to rely on the card doing what we expect;
> we should at least terminate the loop regardless. So, check both the
> input and output values, so we ensure we will terminate regardless of
> the SD card behavior.
> 
> Note that SDIO learned a similar retry loop in commit 0797e5f1453b
> ("mmc: core: Fixup signal voltage switch"), but that used the 'ocr'
> result, and so the current pre-terminating condition looks like:
> 
>     rocr & ocr & R4_18V_PRESENT
> 
> (i.e., it doesn't have the same bug.)
> 
> This addresses a number of crash reports seen on ChromeOS that look
> like the following:
> 
>     ... // lots of repeated: ...
>     <4>[13142.846061] mmc1: Skipping voltage switch
>     <4>[13143.406087] mmc1: Skipping voltage switch
>     <4>[13143.964724] mmc1: Skipping voltage switch
>     <4>[13144.526089] mmc1: Skipping voltage switch
>     <4>[13145.086088] mmc1: Skipping voltage switch
>     <4>[13145.645941] mmc1: Skipping voltage switch
>     <3>[13146.153969] INFO: task halt:30352 blocked for more than 122 seconds.
>     ...
> 
> Fixes: f2119df6b764 mmc: sd: add support for signal voltage switch procedure
> Cc: <stable@...r.kernel.org>
> Signed-off-by: Brian Norris <briannorris@...omium.org>

Reviewed-by: Guenter Roeck <linux@...ck-us.net>

> ---
> 
>  drivers/mmc/core/sd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 06aa62ce0ed1..3662bf5320ce 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -870,7 +870,8 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
>  	 * the CCS bit is set as well. We deliberately deviate from the spec in
>  	 * regards to this, which allows UHS-I to be supported for SDSC cards.
>  	 */
> -	if (!mmc_host_is_spi(host) && rocr && (*rocr & SD_ROCR_S18A)) {
> +	if (!mmc_host_is_spi(host) && (ocr & SD_OCR_S18R) &&
> +	    rocr && (*rocr & SD_ROCR_S18A)) {
>  		err = mmc_set_uhs_voltage(host, pocr);
>  		if (err == -EAGAIN) {
>  			retries--;
> -- 
> 2.37.2.789.g6183377224-goog
>