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]

Date:   Wed, 8 Dec 2021 09:54:44 +0100
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     "Modilaynen, Pavel" <pavel.modilaynen@...vocars.com>
Cc:     "drew@...gleboard.org" <drew@...gleboard.org>,
        "linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
        "menschel.p@...teo.de" <menschel.p@...teo.de>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "will@...china.cc" <will@...china.cc>
Subject: Re: [net-next 6/6] can: mcp251xfd: mcp251xfd_regmap_crc_read(): work
 around broken CRC on TBC register

On 07.12.2021 16:53:12, Modilaynen, Pavel wrote:
> Hello Marc,
> 
> We observe the very similar issue with MCP2517FD.
> 
> >The CRC errors the patch works around are CRC errors introduced by a
> >chip erratum, not by electromagnetic interference. In my observation
> 
> Are you referring this errata doc
> https://datasheet.octopart.com/MCP2517FD-H-JHA-Microchip-datasheet-136609045.pdf ?

ACK - although there are newer versions available. The newest erratum
for the mcp2517fd is the "C" issue:

| https://ww1.microchip.com/downloads/en/DeviceDoc/MCP2517FD-Silicon-Errata-and-Data-Sheet-Clarification-DS80000792C.pdf

The mpc2518fd can be downloaded here:

| https://ww1.microchip.com/downloads/en/DeviceDoc/MCP2518FD-Silicon-Errata-and-Data-Sheet-Clarification-DS80000789C.pdf

> We have the similar CRC read errors but
> the lowest byte is not 0x00 and 0x80, it's actually 0x0x or 0x8x, e.g.
> 
> mcp251xfd spi0.0 can0: CRC read error at address 0x0010 (length=4, data=82 d1 fa 6c, CRC=0xd9c2) retrying.
> 
> 0xb0 0x10 0x04 0x82 0xd1 0xfa 0x6c => 0x59FD (not matching)
> 
> but if I flip the first received bit  (highest bit in the lowest byte):
> 0xb0 0x10 0x04 0x02 0xd1 0xfa 0x6c => 0xD9C2 (matching!)
> 
> So, your fix covers only the case of 0x00 and 0x80, 
> do you think that the workaround should be extended so check
>      (buf_rx->data[0] == 0x0 || buf_rx->data[0] == 0x80)) {
> turns into 
>      ((buf_rx->data[0] & 0xf0) == 0x0 || (buf_rx->data[0] & 0xf0) == 0x80)) {
> 
> Errata, actually says
> "Only bits 7/15/23/31 of the following registers can be affected:"
> 
> So, we could basically, in simplest case flip bit 31 and re-check CRC
> without any check of rx->data[0]....

Can you send a patch that updates the workaround and description.

regards,
Marc

-- 
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)