netdev - Re: [net-next 6/6] can: mcp251xfd: mcp251xfd_regmap_crc_read(): work around broken CRC on TBC register

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: <20220621142515.4xgxhj6oxo5kuepn@pengutronix.de>
Date:   Tue, 21 Jun 2022 16:25:15 +0200
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     Thomas.Kopp@...rochip.com
Cc:     pavel.modilaynen@...vocars.com, drew@...gleboard.org,
        linux-can@...r.kernel.org, menschel.p@...teo.de,
        netdev@...r.kernel.org, will@...china.cc
Subject: Re: [net-next 6/6] can: mcp251xfd: mcp251xfd_regmap_crc_read(): work
 around broken CRC on TBC register

Picking up this old thread....

On 21.12.2021 22:24:52, Thomas.Kopp@...rochip.com wrote:
> Thanks for the data. I've looked into this and it seems that the
> second bit being set in your case does not depend on the SPI-Rate (or
> the quirks for that matter) but it seems to be hardware setup related.
> 
> I'm fine with changing the driver so that it ignores set LSBs but
> would limit it to 2 or 3 bits:

> (buf_rx->data[0] == 0x0 || buf_rx->data[0] == 0x80))
> becomes
> ((buf_rx->data[0] & 0xf8) == 0x0 || (buf_rx->data[0] & 0xf8) == 0x80)) {
> 
> The action also needs to be changed and the flip back of the bit needs
> to be removed. In this case the flipped databit that produces a
> matching CRC is actually  correct (i.e. consistent with the 7 LSBs in
> that byte.)
> 
> A patch could look like this (I'm currently not close to a setup where
> I can compile/test this.)

Thomas, can I have your Signed-off-by for this patch?

Marc

> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> index 297491516a26..e5bc897f37e8 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c
> @@ -332,12 +332,10 @@ mcp251xfd_regmap_crc_read(void *context,
>                  *
>                  * If the highest bit in the lowest byte is flipped
>                  * the transferred CRC matches the calculated one. We
> -                * assume for now the CRC calculation in the chip
> -                * works on wrong data and the transferred data is
> -                * correct.
> +                * assume for now the CRC operates on the correct data.
>                  */
>                 if (reg == MCP251XFD_REG_TBC &&
> -                   (buf_rx->data[0] == 0x0 || buf_rx->data[0] == 0x80)) {
> +                   ((buf_rx->data[0] & 0xF8) == 0x0 || (buf_rx->data[0] & 0xF8) == 0x80)) {
>                         /* Flip highest bit in lowest byte of le32 */
>                         buf_rx->data[0] ^= 0x80;
> 
> @@ -347,10 +345,8 @@ mcp251xfd_regmap_crc_read(void *context,
>                                                                   val_len);
>                         if (!err) {
>                                 /* If CRC is now correct, assume
> -                                * transferred data was OK, flip bit
> -                                * back to original value.
> +                                * flipped data was OK.
>                                  */
> -                               buf_rx->data[0] ^= 0x80;
>                                 goto out;
>                         }
>                 }
> 
> Thanks,
> Thomas
> 

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

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