[PATCH] net: fix signed shift overflow in IPv6 phdr cksum

[PATCH] net: fix signed shift overflow in IPv6 phdr cksum

[Aleksandr Khromov]

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: stable@dpdk.org
Signed-off-by: Aleksandr Khromov <haa@amicon.ru>
---
 .mailmap          | 1 +
 lib/net/rte_ip6.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 9e45cdce8f..2d55627dc5 100644
--- a/.mailmap
+++ b/.mailmap
@@ -46,6 +46,7 @@ Alan Liu <zaoxingliu@gmail.com>
 Alan Winkowski <walan@marvell.com>
 Alejandro Lucero <alejandro.lucero@netronome.com>
 Aleksander Gajewski <aleksanderx.gajewski@intel.com>
+Aleksandr Khromov <haa@amicon.ru>
 Aleksandr Loktionov <aleksandr.loktionov@intel.com>
 Aleksandr Miloshenko <a.miloshenko@f5.com>
 Aleksey Baulin <aleksey.baulin@gmail.com>
diff --git a/lib/net/rte_ip6.h b/lib/net/rte_ip6.h
index d1abf1f5d5..14e0101c14 100644
--- a/lib/net/rte_ip6.h
+++ b/lib/net/rte_ip6.h
@@ -566,7 +566,7 @@ rte_ipv6_phdr_cksum(const struct rte_ipv6_hdr *ipv6_hdr, uint64_t ol_flags)
 		rte_be32_t proto; /* L4 protocol - top 3 bytes must be zero */
 	} psd_hdr;
 
-	psd_hdr.proto = (uint32_t)(ipv6_hdr->proto << 24);
+	psd_hdr.proto = (uint32_t)ipv6_hdr->proto << 24;
 	if (ol_flags & (RTE_MBUF_F_TX_TCP_SEG | RTE_MBUF_F_TX_UDP_SEG))
 		psd_hdr.len = 0;
 	else
-- 
2.48.1

Re: [PATCH] net: fix signed shift overflow in IPv6 phdr cksum

[Stephen Hemminger]

On Fri, 18 Sep 2026 10:47:29 +0300
Aleksandr Khromov <haa@amicon.ru> 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: stable@dpdk.org
> Signed-off-by: Aleksandr Khromov <haa@amicon.ru>
> ---

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.

Re: [PATCH] net: fix signed shift overflow in IPv6 phdr cksum

[Thomas Monjalon]

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 <haa@amicon.ru> 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: stable@dpdk.org
> > Signed-off-by: Aleksandr Khromov <haa@amicon.ru>
> > ---
> 
> 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.
> 
> 

Re: [PATCH] net: fix signed shift overflow in IPv6 phdr cksum

[Aleksandr Khromov]

On 28/09/2026 16:18, Thomas Monjalon wrote:
> Should we merge this patch and expect another one for the endian issue?

Yes, please apply it as is. The byte order problem is a separate,
pre-existing bug that only shows up on big-endian builds, so I would
rather fix it in its own commit.

I will send a follow-up series with two patches:
  - net: use rte_cpu_to_be_32() for the next header in the IPv6
    pseudo-header, as Stephen suggested;
  - net/hinic: same fix for the copy in hinic_ipv6_phdr_cksum().

Thanks Stephen for the review.