linux-kernel - Re: [PATCH] spi: bitbang: Fix lsb-first Rx

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, 3 Aug 2022 16:02:29 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Robin Murphy <robin.murphy@....com>,
        Andreas Färber <afaerber@...e.de>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Christian Hewitt <christianshewitt@...il.com>
Cc:     linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] spi: bitbang: Fix lsb-first Rx

On Wed, Aug 03, 2022 at 03:58:57PM +0100, Robin Murphy wrote:
> Shifting the recieved bit by "bits" inserts it at the top of the
> *currently remaining* Tx data, so we end up accumulating the whole
> transfer into bit 0 of the output word. Oops.
> 
> For the algorithm to work as intended, we need to remember where the
> top of the *original* word was, and shift Rx to there.

So if this never worked we presumably have some systems out there which
somehow rely on the old behaviour that we need to fix somehow - copying
in everyone from the original change...

> Fixes: 1847e3046c52 ("spi: gpio: Implement LSB First bitbang support")
> Signed-off-by: Robin Murphy <robin.murphy@....com>
> ---
>  drivers/spi/spi-bitbang-txrx.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h
> index 267342dfa738..2dcbe166df63 100644
> --- a/drivers/spi/spi-bitbang-txrx.h
> +++ b/drivers/spi/spi-bitbang-txrx.h
> @@ -116,6 +116,7 @@ bitbang_txrx_le_cpha0(struct spi_device *spi,
>  {
>  	/* if (cpol == 0) this is SPI_MODE_0; else this is SPI_MODE_2 */
>  
> +	u8 rxbit = bits - 1;
>  	u32 oldbit = !(word & 1);
>  	/* clock starts at inactive polarity */
>  	for (; likely(bits); bits--) {
> @@ -135,7 +136,7 @@ bitbang_txrx_le_cpha0(struct spi_device *spi,
>  		/* sample LSB (from slave) on leading edge */
>  		word >>= 1;
>  		if ((flags & SPI_MASTER_NO_RX) == 0)
> -			word |= getmiso(spi) << (bits - 1);
> +			word |= getmiso(spi) << rxbit;
>  		setsck(spi, cpol);
>  	}
>  	return word;
> @@ -148,6 +149,7 @@ bitbang_txrx_le_cpha1(struct spi_device *spi,
>  {
>  	/* if (cpol == 0) this is SPI_MODE_1; else this is SPI_MODE_3 */
>  
> +	u8 rxbit = bits - 1;
>  	u32 oldbit = !(word & 1);
>  	/* clock starts at inactive polarity */
>  	for (; likely(bits); bits--) {
> @@ -168,7 +170,7 @@ bitbang_txrx_le_cpha1(struct spi_device *spi,
>  		/* sample LSB (from slave) on trailing edge */
>  		word >>= 1;
>  		if ((flags & SPI_MASTER_NO_RX) == 0)
> -			word |= getmiso(spi) << (bits - 1);
> +			word |= getmiso(spi) << rxbit;
>  	}
>  	return word;
>  }
> -- 
> 2.36.1.dirty
> 

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)