On Fri, 18 Sep 2026 10:47:29 +0300
Aleksandr Khromov <[email protected]> wrote:
> In rte_ipv6_phdr_cksum() the next header field, a uint8_t, is promoted to
> a signed int before the left shift by 24. For protocol values >= 128
> (for example IPPROTO_SCTP), proto << 24 does not fit in int, which is
> undefined behaviour (signed left shift overflow) reported by UBSan:
>
> rte_ip6.h: runtime error: left shift of 132 by 24 places cannot be
> represented in type 'int'
>
> Cast the operand to uint32_t before the shift so it is performed in
> unsigned arithmetic. The resulting value is unchanged on two's
> complement platforms. The same idiom is already used in RTE_IPV4().
>
> Fixes: 6006818cfb26 ("net: new checksum functions")
> Cc: [email protected]
> Signed-off-by: Aleksandr Khromov <[email protected]>
> ---
Looks good, but there is also a pre-existing byte order issue here.
Review: [PATCH] net: fix signed shift overflow in IPv6 phdr cksum
Patchwork: 169805
Applies cleanly to main. Fixes: 6006818cfb26 verified; the line was
introduced there and carried over by 1a2b549bb4 (header split).
Cc: stable is correct.
The fix is right. ipv6_hdr->proto is uint8_t, promoted to int, and
proto << 24 for proto >= 128 (SCTP = 132) overflows int. Casting the
operand to uint32_t makes the shift unsigned; generated code is the
same.
Info
drivers/net/hinic/hinic_pmd_tx.c:740 has an identical copy of this
code with the same UB:
psd_hdr.proto = (ipv6_hdr->proto << 24);
Worth fixing in the same patch (or a v2 as a two patch series) so
the pattern does not survive in the driver.
Consider rte_cpu_to_be_32(ipv6_hdr->proto) instead of the shift.
psd_hdr.proto is rte_be32_t and must hold proto in the last byte in
memory. proto << 24 only achieves that on little-endian; on a
big-endian build it lands in the first byte and the pseudo-header
sum is wrong. rte_cpu_to_be_32() removes the shift, is correct for
both byte orders, and the compiler folds it to the same shift on
little-endian. Not a blocker since the endian issue is pre-existing.
