Should we merge this patch and expect another one for the endian issue?
21/09/2026 23:15, Stephen Hemminger: > 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. > >
