linux-kernel - Re: [PATCH 1/6] RDMA/rxe: handle ICRC correctly on big endian systems

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: <20250129185115.GA2619178@google.com>
Date: Wed, 29 Jan 2025 18:51:15 +0000
From: Eric Biggers <ebiggers@...nel.org>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: Zhu Yanjun <yanjun.zhu@...ux.dev>, linux-rdma@...r.kernel.org,
	Mustafa Ismail <mustafa.ismail@...el.com>,
	Tatyana Nikolova <tatyana.e.nikolova@...el.com>,
	Leon Romanovsky <leon@...nel.org>,
	Zhu Yanjun <zyjzyj2000@...il.com>,
	Bernard Metzler <bmt@...ich.ibm.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/6] RDMA/rxe: handle ICRC correctly on big endian systems

Hi Jason,

On Wed, Jan 29, 2025 at 02:30:09PM -0400, Jason Gunthorpe wrote:
> On Wed, Jan 29, 2025 at 10:44:39AM +0100, Zhu Yanjun wrote:
> > 在 2025/1/27 23:38, Eric Biggers 写道:
> > > From: Eric Biggers <ebiggers@...gle.com>
> > > 
> > > The usual big endian convention of InfiniBand does not apply to the
> > > ICRC field, whose transmission is specified in terms of the CRC32
> > > polynomial coefficients.  
> 
> This patch is on to something but this is not a good explanation.
> 
> The CRC32 in IB is stored as big endian and computed in big endian,
> the spec says so explicitly:
> 
> 2) The CRC calculation is done in big endian byte order with the least
>    significant bit of the most significant byte being the first
>    bits in the CRC calculation.

(2) just specifies the order in which the bits are passed to the CRC.  It says
nothing about how the CRC value is stored; that's in (4) instead.

The mention of "big endian" seems to refer to the bytes being passed from first
to last, which is the nearly universal convention.  (I would not have used the
term "big endian" here, as it's confusing.)  The rest clarifies that it's a
least-significant-bit (LSB)-first CRC, i.e. "little endian" or "le" in Linux
terminology.  (The Linux terminology here is a mistake too -- it would be
clearer to call it lsb rather than le, given that it's about bits.)

> In this context saying it is not "big endian" doesn't seem to be quite
> right..
> 
> The spec gives a sample data packet (in offset/value pairs):
> 
> 0  0xF0 15 0xB3 30 0x7A 45 0x8B
> 1  0x12 16 0x00 31 0x05 46 0xC0
> 2  0x37 17 0x0D 32 0x00 47 0x69
> 3  0x5C 18 0xEC 33 0x00 48 0x0E
> 4  0x00 19 0x2A 34 0x00 49 0xD4
> 5  0x0E 20 0x01 35 0x0E 50 0x00
> 6  0x17 21 0x71 36 0xBB 51 0x00
> 7  0xD2 22 0x0A 37 0x88
> 8  0x0A 23 0x1C 38 0x4D
> 9  0x20 24 0x01 39 0x85
> 10 0x24 25 0x5D 40 0xFD
> 11 0x87 26 0x40 41 0x5C
> 12 0xFF 27 0x02 42 0xFB
> 13 0x87 28 0x38 43 0xA4
> 14 0xB1 29 0xF2 44 0x72
> 
> If you feed that to the CRC32 you should get 0x9625B75A in a CPU
> register u32.
> 
> cpu_to_be32() will put it in the right order for on the wire.

I think cpu_to_be32() would put it in the wrong order.  Refer to the following:

    4. The resulting bits are sent in order from the bit representing the
    coefficient of the highest term of the remainder polynomial. The least
    significant bit, most significant byte first ordering of the packet does not
    apply to the ICRC field.

As per (2) it's a least-significant-bit first CRC, i.e. bit 0 represents the
coefficient of x^31 and bit 31 represents the coefficient of x^0.  So the least
significant byte of the u32 has the highest 8 polynomial terms (the coefficients
of x^24 through x^31) and must be sent first.  That's an __le32.  Yes, the fact
that the mapping between bits and polynomial terms is backwards makes it
confusing, but that's how LSB-first CRCs work.

> I assume the Linux CRC32 always gives the same CPU value regardless of
> LE or BE?

Yes, they return a u32.  (Unless you use crypto_shash_final or
crypto_shash_digest in which case you get a u8[4] a.k.a. __le32.)

- Eric