| 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
| ||
|
Message-ID: <ae41a37e-3ee4-484e-ba53-1cad9225df7a@linux.dev>
Date: Wed, 29 Jan 2025 10:44:39 +0100
From: Zhu Yanjun <yanjun.zhu@...ux.dev>
To: Eric Biggers <ebiggers@...nel.org>, linux-rdma@...r.kernel.org,
Mustafa Ismail <mustafa.ismail@...el.com>,
Tatyana Nikolova <tatyana.e.nikolova@...el.com>,
Jason Gunthorpe <jgg@...pe.ca>, Leon Romanovsky <leon@...nel.org>,
Zhu Yanjun <zyjzyj2000@...il.com>, Bernard Metzler <bmt@...ich.ibm.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/6] RDMA/rxe: handle ICRC correctly on big endian systems
在 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. The coefficients are transmitted in
> descending order from the x^31 coefficient to the x^0 one. When the
> result is interpreted as a 32-bit integer using the required reverse
> mapping between bits and polynomial coefficients, it's a __le32.
>
> The code used __be32, but it did not actually do an endianness
> conversion. So it actually just used the CPU's native endianness,
> making it comply with the spec on little endian systems only.
>
> Fix the code to use __le32 and do the needed le32_to_cpu() and
> cpu_to_le32() so that it complies with the spec on big endian systems.
>
> Also update the lower-level helper functions to use u32, as they are
> working with CPU native values. This part does not change behavior.
>
> Found through code review. I don't know whether anyone is actually
> using this code on big endian systems. Probably not.
>
> Signed-off-by: Eric Biggers <ebiggers@...gle.com>
In this commit, the little endian is used instead of big endian in the
rxe source code. To x86_64 architecture, I am fine with this commit.
To other big endian architecture, e.g. ppc/ppc64, I do not have such
host, thus, I can not make tests on the big endian host.
Thanks,
Reviewed-by: Zhu Yanjun <yanjun.zhu@...ux.dev>
Zhu Yanjun
> ---
> drivers/infiniband/sw/rxe/rxe_icrc.c | 41 +++++++++++++++-------------
> 1 file changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
> index fdf5f08cd8f17..ee417a0bbbdca 100644
> --- a/drivers/infiniband/sw/rxe/rxe_icrc.c
> +++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
> @@ -38,26 +38,26 @@ int rxe_icrc_init(struct rxe_dev *rxe)
> * @next: starting address of current segment
> * @len: length of current segment
> *
> * Return: the cumulative crc32 checksum
> */
> -static __be32 rxe_crc32(struct rxe_dev *rxe, __be32 crc, void *next, size_t len)
> +static u32 rxe_crc32(struct rxe_dev *rxe, u32 crc, void *next, size_t len)
> {
> - __be32 icrc;
> + u32 icrc;
> int err;
>
> SHASH_DESC_ON_STACK(shash, rxe->tfm);
>
> shash->tfm = rxe->tfm;
> - *(__be32 *)shash_desc_ctx(shash) = crc;
> + *(u32 *)shash_desc_ctx(shash) = crc;
> err = crypto_shash_update(shash, next, len);
> if (unlikely(err)) {
> rxe_dbg_dev(rxe, "failed crc calculation, err: %d\n", err);
> - return (__force __be32)crc32_le((__force u32)crc, next, len);
> + return crc32_le(crc, next, len);
> }
>
> - icrc = *(__be32 *)shash_desc_ctx(shash);
> + icrc = *(u32 *)shash_desc_ctx(shash);
> barrier_data(shash_desc_ctx(shash));
>
> return icrc;
> }
>
> @@ -67,18 +67,18 @@ static __be32 rxe_crc32(struct rxe_dev *rxe, __be32 crc, void *next, size_t len)
> * @skb: packet buffer
> * @pkt: packet information
> *
> * Return: the partial ICRC
> */
> -static __be32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> +static u32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> {
> unsigned int bth_offset = 0;
> struct iphdr *ip4h = NULL;
> struct ipv6hdr *ip6h = NULL;
> struct udphdr *udph;
> struct rxe_bth *bth;
> - __be32 crc;
> + u32 crc;
> int length;
> int hdr_size = sizeof(struct udphdr) +
> (skb->protocol == htons(ETH_P_IP) ?
> sizeof(struct iphdr) : sizeof(struct ipv6hdr));
> /* pseudo header buffer size is calculate using ipv6 header size since
> @@ -89,11 +89,11 @@ static __be32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> RXE_BTH_BYTES];
>
> /* This seed is the result of computing a CRC with a seed of
> * 0xfffffff and 8 bytes of 0xff representing a masked LRH.
> */
> - crc = (__force __be32)0xdebb20e3;
> + crc = 0xdebb20e3;
>
> if (skb->protocol == htons(ETH_P_IP)) { /* IPv4 */
> memcpy(pshdr, ip_hdr(skb), hdr_size);
> ip4h = (struct iphdr *)pshdr;
> udph = (struct udphdr *)(ip4h + 1);
> @@ -137,23 +137,27 @@ static __be32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> *
> * Return: 0 if the values match else an error
> */
> int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> {
> - __be32 *icrcp;
> - __be32 pkt_icrc;
> - __be32 icrc;
> -
> - icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> - pkt_icrc = *icrcp;
> + /*
> + * 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. The coefficients are transmitted in
> + * descending order from the x^31 coefficient to the x^0 one. When the
> + * result is interpreted as a 32-bit integer using the required reverse
> + * mapping between bits and polynomial coefficients, it's a __le32.
> + */
> + __le32 *icrcp = (__le32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> + u32 icrc;
>
> icrc = rxe_icrc_hdr(skb, pkt);
> icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
> payload_size(pkt) + bth_pad(pkt));
> icrc = ~icrc;
>
> - if (unlikely(icrc != pkt_icrc))
> + if (unlikely(icrc != le32_to_cpu(*icrcp)))
> return -EINVAL;
>
> return 0;
> }
>
> @@ -162,14 +166,13 @@ int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> * @skb: packet buffer
> * @pkt: packet information
> */
> void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> {
> - __be32 *icrcp;
> - __be32 icrc;
> + __le32 *icrcp = (__le32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> + u32 icrc;
>
> - icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> icrc = rxe_icrc_hdr(skb, pkt);
> icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
> payload_size(pkt) + bth_pad(pkt));
> - *icrcp = ~icrc;
> + *icrcp = cpu_to_le32(~icrc);
> }
Powered by blists - more mailing lists