From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202602 header.b=T4k6QeVn; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 0B0095A0269 for ; Mon, 11 May 2026 03:42:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1778463758; bh=tCgXFliooHRKCkOh/ePVxA6yURXm6cwoHifi4gX/zVo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=T4k6QeVnDncD0Bv4A/0kj3jKRLHmycsdyDLnwFaURIO4Ul/sJ7+yCLsM4M/wyPGIf BpU3/LLX7hKkN0k5V8hZWZtg8rea2mbRwITGk52KEwfnwJaHcwOp/vdG7dHqBkkq4b CY2AtdkYYop0S+RyeaeAuoJ7AifpEs//E0zrXAmguZT1afHgTwr9REjN7jnAQgyrkG luGRdTx/k+o0r3bZ11VbDICvwx9VVMobL12qVXSf6yUVnzehJXq0ncnhAXPRpcm3FU BJfCx2Tgqn0zPXzoUrLRJmd0TuOaKv77rT/ijKRMdkDYNFKZSIlB2GFYS+Sb3rCheV YTToGeuJKXijg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gDMt64pjhz4wJS; Mon, 11 May 2026 11:42:38 +1000 (AEST) Date: Mon, 11 May 2026 11:33:39 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v3 06/10] checksum: Pass explicit L4 length to checksum functions Message-ID: References: <20260416155721.3807225-1-lvivier@redhat.com> <20260416155721.3807225-7-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="GUnfAMFc5V0NXVEh" Content-Disposition: inline In-Reply-To: <20260416155721.3807225-7-lvivier@redhat.com> Message-ID-Hash: HILPCV4VHJTZU5D4TCX3YWHWTG2LSGVA X-Message-ID-Hash: HILPCV4VHJTZU5D4TCX3YWHWTG2LSGVA X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --GUnfAMFc5V0NXVEh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Apr 16, 2026 at 05:57:17PM +0200, Laurent Vivier wrote: > The iov_tail passed to csum_iov_tail() may contain padding or trailing > data beyond the actual L4 payload. Rather than relying on > iov_tail_size() to determine how many bytes to checksum, pass the > length explicitly so that only the relevant payload bytes are included > in the checksum computation. >=20 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson > --- > checksum.c | 43 +++++++++++++++++++++++++------------------ > checksum.h | 6 +++--- > tap.c | 4 ++-- > tcp.c | 12 +++++++----- > udp.c | 5 +++-- > udp_vu.c | 21 +++++++++------------ > 6 files changed, 49 insertions(+), 42 deletions(-) >=20 > diff --git a/checksum.c b/checksum.c > index 828f9ecc9c02..7c62e42d6d4c 100644 > --- a/checksum.c > +++ b/checksum.c > @@ -182,21 +182,22 @@ static uint16_t csum(const void *buf, size_t len, u= int32_t init) > * @saddr: IPv4 source address > * @daddr: IPv4 destination address > * @data: UDP payload (as IO vector tail) > + * @dlen: UDP payload length > */ > void csum_udp4(struct udphdr *udp4hr, > struct in_addr saddr, struct in_addr daddr, > - struct iov_tail *data) > + struct iov_tail *data, size_t dlen) > { > /* UDP checksums are optional, so don't bother */ > udp4hr->check =3D 0; > =20 > if (UDP4_REAL_CHECKSUMS) { > - uint16_t l4len =3D iov_tail_size(data) + sizeof(struct udphdr); > - uint32_t psum =3D proto_ipv4_header_psum(l4len, IPPROTO_UDP, > - saddr, daddr); > + uint32_t psum =3D proto_ipv4_header_psum(sizeof(*udp4hr) + dlen, > + IPPROTO_UDP, saddr, > + daddr); > =20 > - psum =3D csum_unfolded(udp4hr, sizeof(struct udphdr), psum); > - udp4hr->check =3D csum_iov_tail(data, psum); > + psum =3D csum_unfolded(udp4hr, sizeof(*udp4hr), psum); > + udp4hr->check =3D csum_iov_tail(data, psum, dlen); > } > } > =20 > @@ -245,19 +246,19 @@ uint32_t proto_ipv6_header_psum(uint16_t payload_le= n, uint8_t protocol, > * @saddr: Source address > * @daddr: Destination address > * @data: UDP payload (as IO vector tail) > + * @dlen: UDP payload length > */ > void csum_udp6(struct udphdr *udp6hr, > const struct in6_addr *saddr, const struct in6_addr *daddr, > - struct iov_tail *data) > + struct iov_tail *data, size_t dlen) > { > - uint16_t l4len =3D iov_tail_size(data) + sizeof(struct udphdr); > - uint32_t psum =3D proto_ipv6_header_psum(l4len, IPPROTO_UDP, > - saddr, daddr); > + uint32_t psum =3D proto_ipv6_header_psum(dlen + sizeof(*udp6hr), > + IPPROTO_UDP, saddr, daddr); > =20 > udp6hr->check =3D 0; > =20 > - psum =3D csum_unfolded(udp6hr, sizeof(struct udphdr), psum); > - udp6hr->check =3D csum_iov_tail(data, psum); > + psum =3D csum_unfolded(udp6hr, sizeof(*udp6hr), psum); > + udp6hr->check =3D csum_iov_tail(data, psum, dlen); > } > =20 > /** > @@ -604,20 +605,26 @@ uint32_t csum_unfolded(const void *buf, size_t len,= uint32_t init) > /** > * csum_iov_tail() - Calculate unfolded checksum for the tail of an IO v= ector > * @tail: IO vector tail to checksum > - * @init Initial 32-bit checksum, 0 for no pre-computed checksum > + * @init: Initial 32-bit checksum, 0 for no pre-computed checksum > + * @len: Number of bytes to checksum from @tail > * > * Return: 16-bit folded, complemented checksum > */ > -uint16_t csum_iov_tail(struct iov_tail *tail, uint32_t init) > +uint16_t csum_iov_tail(struct iov_tail *tail, uint32_t init, size_t len) > { > if (iov_tail_prune(tail)) { > - size_t i; > + size_t i, n; > =20 > + n =3D MIN(len, tail->iov[0].iov_len - tail->off); > init =3D csum_unfolded((char *)tail->iov[0].iov_base + tail->off, > - tail->iov[0].iov_len - tail->off, init); > - for (i =3D 1; i < tail->cnt; i++) { > + n, init); > + len -=3D n; > + > + for (i =3D 1; len && i < tail->cnt; i++) { > const struct iovec *iov =3D &tail->iov[i]; > - init =3D csum_unfolded(iov->iov_base, iov->iov_len, init); > + n =3D MIN(len, iov->iov_len); > + init =3D csum_unfolded(iov->iov_base, n, init); > + len -=3D n; > } > } > return (uint16_t)~csum_fold(init); > diff --git a/checksum.h b/checksum.h > index 4e3b098db072..6270f1457a73 100644 > --- a/checksum.h > +++ b/checksum.h > @@ -21,18 +21,18 @@ uint32_t proto_ipv4_header_psum(uint16_t l4len, uint8= _t protocol, > struct in_addr saddr, struct in_addr daddr); > void csum_udp4(struct udphdr *udp4hr, > struct in_addr saddr, struct in_addr daddr, > - struct iov_tail *data); > + struct iov_tail *data, size_t dlen); > void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t dle= n); > uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t protocol, > const struct in6_addr *saddr, > const struct in6_addr *daddr); > void csum_udp6(struct udphdr *udp6hr, > const struct in6_addr *saddr, const struct in6_addr *daddr, > - struct iov_tail *data); > + struct iov_tail *data, size_t dlen); > void csum_icmp6(struct icmp6hdr *icmp6hr, > const struct in6_addr *saddr, const struct in6_addr *daddr, > const void *payload, size_t dlen); > uint32_t csum_unfolded(const void *buf, size_t len, uint32_t init); > -uint16_t csum_iov_tail(struct iov_tail *tail, uint32_t init); > +uint16_t csum_iov_tail(struct iov_tail *tail, uint32_t init, size_t len); > =20 > #endif /* CHECKSUM_H */ > diff --git a/tap.c b/tap.c > index 1049e023bcd2..41a61a36c279 100644 > --- a/tap.c > +++ b/tap.c > @@ -252,7 +252,7 @@ void *tap_push_uh4(struct udphdr *uh, struct in_addr = src, in_port_t sport, > uh->source =3D htons(sport); > uh->dest =3D htons(dport); > uh->len =3D htons(l4len); > - csum_udp4(uh, src, dst, &payload); > + csum_udp4(uh, src, dst, &payload, dlen); > return (char *)uh + sizeof(*uh); > } > =20 > @@ -357,7 +357,7 @@ void *tap_push_uh6(struct udphdr *uh, > uh->source =3D htons(sport); > uh->dest =3D htons(dport); > uh->len =3D htons(l4len); > - csum_udp6(uh, src, dst, &payload); > + csum_udp6(uh, src, dst, &payload, dlen); > return (char *)uh + sizeof(*uh); > } > =20 > diff --git a/tcp.c b/tcp.c > index 8ea9be84a9f3..5b2a732cf26d 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -815,13 +815,14 @@ static void tcp_sock_set_nodelay(int s) > * @psum: Unfolded partial checksum of the IPv4 or IPv6 pseudo-header > * @th: TCP header (updated) > * @payload: TCP payload > + * @dlen: TCP payload length > */ > static void tcp_update_csum(uint32_t psum, struct tcphdr *th, > - struct iov_tail *payload) > + struct iov_tail *payload, size_t dlen) > { > th->check =3D 0; > psum =3D csum_unfolded(th, sizeof(*th), psum); > - th->check =3D csum_iov_tail(payload, psum); > + th->check =3D csum_iov_tail(payload, psum, dlen); > } > =20 > /** > @@ -958,7 +959,8 @@ size_t tcp_fill_headers(const struct ctx *c, struct t= cp_tap_conn *conn, > bool no_tcp_csum) > { > const struct flowside *tapside =3D TAPFLOW(conn); > - size_t l4len =3D iov_tail_size(payload) + sizeof(*th); > + size_t dlen =3D iov_tail_size(payload); > + size_t l4len =3D dlen + sizeof(*th); > uint8_t *omac =3D conn->f.tap_omac; > size_t l3len =3D l4len; > uint32_t psum =3D 0; > @@ -1019,7 +1021,7 @@ size_t tcp_fill_headers(const struct ctx *c, struct= tcp_tap_conn *conn, > if (no_tcp_csum) > th->check =3D 0; > else > - tcp_update_csum(psum, th, payload); > + tcp_update_csum(psum, th, payload, dlen); > =20 > return MAX(l3len + sizeof(struct ethhdr), ETH_ZLEN); > } > @@ -2196,7 +2198,7 @@ static void tcp_rst_no_conn(const struct ctx *c, in= t af, > rsth->ack =3D 1; > } > =20 > - tcp_update_csum(psum, rsth, &payload); > + tcp_update_csum(psum, rsth, &payload, 0); > rst_l2len =3D ((char *)rsth - buf) + sizeof(*rsth); > tap_send_single(c, buf, rst_l2len); > } > diff --git a/udp.c b/udp.c > index 1fc5a42c5ca7..4eef10854d8a 100644 > --- a/udp.c > +++ b/udp.c > @@ -289,7 +289,7 @@ size_t udp_update_hdr4(struct iphdr *ip4h, struct udp= _payload_t *bp, > .iov_len =3D dlen > }; > struct iov_tail data =3D IOV_TAIL(&iov, 1, 0); > - csum_udp4(&bp->uh, *src, *dst, &data); > + csum_udp4(&bp->uh, *src, *dst, &data, dlen); > } > =20 > return l4len; > @@ -334,7 +334,8 @@ size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct u= dp_payload_t *bp, > .iov_len =3D dlen > }; > struct iov_tail data =3D IOV_TAIL(&iov, 1, 0); > - csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data); > + csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data, > + dlen); > } > =20 > return l4len; > diff --git a/udp_vu.c b/udp_vu.c > index 277bd5688907..1a73d997f683 100644 > --- a/udp_vu.c > +++ b/udp_vu.c > @@ -105,14 +105,11 @@ static ssize_t udp_vu_sock_recv(struct iovec *iov, = size_t *cnt, int s, bool v6) > * @iov: IO vector for the frame (including vnet header) > * @toside: Address information for one side of the flow > * @dlen: Packet data length > - * > - * Return: Layer-4 length > */ > -static size_t udp_vu_prepare(const struct ctx *c, const struct iovec *io= v, > +static void udp_vu_prepare(const struct ctx *c, const struct iovec *iov, > const struct flowside *toside, ssize_t dlen) > { > struct ethhdr *eh; > - size_t l4len; > =20 > /* ethernet header */ > eh =3D vu_eth(iov[0].iov_base); > @@ -129,7 +126,7 @@ static size_t udp_vu_prepare(const struct ctx *c, con= st struct iovec *iov, > =20 > *iph =3D (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_UDP); > =20 > - l4len =3D udp_update_hdr4(iph, bp, toside, dlen, true); > + udp_update_hdr4(iph, bp, toside, dlen, true); > } else { > struct ipv6hdr *ip6h =3D vu_ip(iov[0].iov_base); > struct udp_payload_t *bp =3D vu_payloadv6(iov[0].iov_base); > @@ -138,10 +135,8 @@ static size_t udp_vu_prepare(const struct ctx *c, co= nst struct iovec *iov, > =20 > *ip6h =3D (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_UDP); > =20 > - l4len =3D udp_update_hdr6(ip6h, bp, toside, dlen, true); > + udp_update_hdr6(ip6h, bp, toside, dlen, true); > } > - > - return l4len; > } > =20 > /** > @@ -149,9 +144,10 @@ static size_t udp_vu_prepare(const struct ctx *c, co= nst struct iovec *iov, > * @toside: Address information for one side of the flow > * @iov: IO vector for the frame > * @cnt: Number of IO vector entries > + * @dlen: Data length > */ > static void udp_vu_csum(const struct flowside *toside, const struct iove= c *iov, > - size_t cnt) > + size_t cnt, size_t dlen) > { > const struct in_addr *src4 =3D inany_v4(&toside->oaddr); > const struct in_addr *dst4 =3D inany_v4(&toside->eaddr); > @@ -162,11 +158,12 @@ static void udp_vu_csum(const struct flowside *tosi= de, const struct iovec *iov, > if (src4 && dst4) { > bp =3D vu_payloadv4(base); > data =3D IOV_TAIL(iov, cnt, (char *)&bp->data - base); > - csum_udp4(&bp->uh, *src4, *dst4, &data); > + csum_udp4(&bp->uh, *src4, *dst4, &data, dlen); > } else { > bp =3D vu_payloadv6(base); > data =3D IOV_TAIL(iov, cnt, (char *)&bp->data - base); > - csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data); > + csum_udp6(&bp->uh, &toside->oaddr.a6, &toside->eaddr.a6, &data, > + dlen); > } > } > =20 > @@ -229,7 +226,7 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, i= nt n, flow_sidx_t tosidx) > if (iov_cnt > 0) { > udp_vu_prepare(c, iov_vu, toside, dlen); > if (*c->pcap) { > - udp_vu_csum(toside, iov_vu, iov_cnt); > + udp_vu_csum(toside, iov_vu, iov_cnt, dlen); > pcap_iov(iov_vu, iov_cnt, VNET_HLEN); > } > vu_flush(vdev, vq, elem, elem_used); > --=20 > 2.53.0 >=20 --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --GUnfAMFc5V0NXVEh Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmoBMe4ACgkQzQJF27ox 2GftNBAAj2NoL+gNxBcLvpKwFawdmKhRQmUrS1AA+G07MjrUteEabK6BIVBG/iNX Hitzbbnq01B1rFbvUvj5xS/2QM6H7q7PbIvfLy/sKiPEUucUCw08qbdkEm3vVP2g +GZf4p/khMCBP22ack/i0JrZk+cFpRBlcMXk0uZMJk5M9YmGRcTs1YO2KuaiZh3v UaVZQa7KJrqM9ZLcFSdHyE2itJKOMGI6e9u5mzZSR6lSWKKjZY0CIeykaRHdrdGO jHbHW0Df6KDq/rNlaghQz1YLLsk7HhyHuoA9Otkilk8XOQY3EEHfBs4ni/UJnzE/ enY/NYzwuwBn38XWfWg8u/+lJBl3hnX0Puurq5OAsUMRt1/wJJ+S9UdzQSOnmr+9 dT2XUruJUTpmrJf86laPAraua/3xG7pn27qEjl77f5Ea+J+sS49ErQYoLTpdw23Y wc9DX6huRVspGSpQplBNBeDxA8Za/esN3VyBen7GssyTqIjojrWWducjb3iuBIKU py4v1TvszETO/tTUqMwt4wKXKRTEWR+NYqpj0PnjBQKsWunoFOmUxQK0X2s1SNm0 PddEnit72Bgg52wmuhpcoVCmJYCqZFWQFpbWd2Yxz+1uV0yHt0X9/fAw5/TKjcFr SYkpbuiNfmgKpHNTqFt1mqT5Yh0ZWQz98Hk8DHMT1+lYbQvUxw8= =rAH0 -----END PGP SIGNATURE----- --GUnfAMFc5V0NXVEh--