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=202508 header.b=e3OzBijX; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id C4BCC5A0278 for ; Wed, 06 Aug 2025 06:38:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1754455097; bh=6FZWUcNa9kh0n68vsIp6RZRXFHJ4sjXJpFNF6v1Gbos=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=e3OzBijXEtXZE4lnka7zizPepmAR58779cpbYYmzzfGZ+JKNfWqo/p7OUx3y2AOyH sF8n9DQIqn1nzYCLpbQrclQfKH86Pf5CVtSlxbx13+DHkDs+4/VmwuAtXjaQD5GG57 R+CHDPeR4ZqRiOWU9WxdQ5msNiH+8BduZlFkVz8yhy2h3rCrWiiYphC1prlkCEHiIt bcNl0mRk7TdCzdH5OvWl4GSF+BobwOCjkngOzRuvWx/JoMwZHyjcuQ422oFGVo9irZ 5+6getSPjGm1YtTzUxqn4FDctIvUraz9U8F7+Nw9jF2aaVrWZqFppuF/5LLqgi2h59 R9K8LdJAMpaPA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4bxcx55FKSz4xcb; Wed, 6 Aug 2025 14:38:17 +1000 (AEST) Date: Wed, 6 Aug 2025 14:38:08 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v8 17/30] dhcp: Convert to iov_tail Message-ID: References: <20250805154628.301343-1-lvivier@redhat.com> <20250805154628.301343-18-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="hkpK1J7+7uPKUEUC" Content-Disposition: inline In-Reply-To: <20250805154628.301343-18-lvivier@redhat.com> Message-ID-Hash: IOSIAIDLTTQML3TL4RIALGTWZKVCTC6L X-Message-ID-Hash: IOSIAIDLTTQML3TL4RIALGTWZKVCTC6L 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: --hkpK1J7+7uPKUEUC Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 05, 2025 at 05:46:15PM +0200, Laurent Vivier wrote: > Use packet_data() and extract headers using IOV_REMOVE_HEADER() > and IOV_PEEK_HEADER() rather than packet_get(). Unlike the previous patch, I think using iov_tail does work here, because there's a single scan through the options, rather than repeatedly scanning for options of specific types. > Signed-off-by: Laurent Vivier > --- > dhcp.c | 46 ++++++++++++++++++++++++++++------------------ > 1 file changed, 28 insertions(+), 18 deletions(-) >=20 > diff --git a/dhcp.c b/dhcp.c > index b0de04be6f27..cf73d4b07767 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -302,27 +302,33 @@ static void opt_set_dns_search(const struct ctx *c,= size_t max_len) > */ > int dhcp(const struct ctx *c, const struct pool *p) > { > - size_t mlen, dlen, offset =3D 0, opt_len, opt_off =3D 0; > char macstr[ETH_ADDRSTRLEN]; > + size_t mlen, dlen, opt_len; > struct in_addr mask, dst; > + struct ethhdr eh_storage; > + struct iphdr iph_storage; > + struct udphdr uh_storage; > const struct ethhdr *eh; > const struct iphdr *iph; > const struct udphdr *uh; > + struct iov_tail data; > struct msg const *m; Pre-existing, but I'm a bit baffled as to what the (const *) is doing here. > struct msg reply; > unsigned int i; > + struct msg m_storage; > =20 > - eh =3D packet_get(p, 0, offset, sizeof(*eh), NULL); > - offset +=3D sizeof(*eh); > + if (!packet_data(p, 0, &data)) > + return -1; > =20 > - iph =3D packet_get(p, 0, offset, sizeof(*iph), NULL); > + eh =3D IOV_REMOVE_HEADER(&data, eh_storage); > + iph =3D IOV_PEEK_HEADER(&data, iph_storage); > if (!eh || !iph) > return -1; > =20 > - offset +=3D iph->ihl * 4UL; > - uh =3D packet_get(p, 0, offset, sizeof(*uh), &mlen); > - offset +=3D sizeof(*uh); > + if (!iov_tail_drop(&data, iph->ihl * 4UL)) > + return -1; > =20 > + uh =3D IOV_REMOVE_HEADER(&data, uh_storage); > if (!uh) > return -1; > =20 > @@ -332,7 +338,10 @@ int dhcp(const struct ctx *c, const struct pool *p) > if (c->no_dhcp) > return 1; > =20 > - m =3D packet_get(p, 0, offset, offsetof(struct msg, o), &opt_len); > + mlen =3D iov_tail_size(&data); > + m =3D (struct msg const *)iov_remove_header_(&data, &m_storage, > + offsetof(struct msg, o), > + __alignof__(struct msg)); > if (!m || > mlen !=3D ntohs(uh->len) - sizeof(*uh) || > mlen < offsetof(struct msg, o) || > @@ -355,27 +364,28 @@ int dhcp(const struct ctx *c, const struct pool *p) > memset(&reply.file, 0, sizeof(reply.file)); > reply.magic =3D m->magic; > =20 > - offset +=3D offsetof(struct msg, o); > - > for (i =3D 0; i < ARRAY_SIZE(opts); i++) > opts[i].clen =3D -1; > =20 > - while (opt_off + 2 < opt_len) { > - const uint8_t *olen, *val; > + opt_len =3D iov_tail_size(&data); > + while (opt_len >=3D 2) { > + uint8_t olen_storage, type_storage; > + const uint8_t *olen; > uint8_t *type; > =20 > - type =3D packet_get(p, 0, offset + opt_off, 1, NULL); > - olen =3D packet_get(p, 0, offset + opt_off + 1, 1, NULL); > + type =3D IOV_REMOVE_HEADER(&data, type_storage); > + olen =3D IOV_REMOVE_HEADER(&data, olen_storage); It seems a bit mad to access single bytes via 8-byte pointers, but it's probably not worth the hassle of handling it differently in this one case. > if (!type || !olen) > return -1; > =20 > - val =3D packet_get(p, 0, offset + opt_off + 2, *olen, NULL); > - if (!val) > + opt_len =3D iov_tail_size(&data); > + if (opt_len < *olen) > return -1; > =20 > - memcpy(&opts[*type].c, val, *olen); > + iov_to_buf(&data.iov[0], data.cnt, data.off, &opts[*type].c, *olen); So, IIUC, if *olen is much too big, this is still safe.. > opts[*type].clen =3D *olen; =2E. but recording *olen unedited as the length of the option is probably wrong in that case. > - opt_off +=3D *olen + 2; > + iov_tail_drop(&data, *olen); > + opt_len -=3D *olen; Isn't the stanza above doing the equivalent of an iov_remove_header_()? > } > =20 > opts[80].slen =3D -1; --=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 --hkpK1J7+7uPKUEUC Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmiS3CgACgkQzQJF27ox 2Gc/Lw//bO9zLFpsP3YCUJGD2mlNyWfiru6R6bud3dNlSoEs4Aje0f3OG3MQWZSI 7vaYj8XXB3hLLkkj387oZeXRyts7m7co6ZJVcswhXIQoqrw5kDJZRDmTHDYefv3H xbh8tPByikEA9KgFw0QWjcoNvAP+Ic+7HqQF3562zNisI8vqcoXe2gE6EqZ5QgSW R8Zp4l9jzMTcZLoMqqgz3oA7RoNj7yecjlkxCSIUt+KzHed0TtzYj+qknw+OSdCh P2VU+GvYfbmub7JAxhhBvdqn38N0C34GTgjgZFES1ZYJzFV1kqRdjqJpQB9NBjFm WjksLPI9R+f0EwBAfIV3gVZsvJ0WNumfyuQeagJJ4F/kHJr1dZgMrrJnstbnYPJl JLS4VqAxVXxp3UR7APTL0dNnmBYTGmhZDYQJIcEKY5+z86zljYmIJq68TWnPCSpY +yuFdZeH+z5KIpUQjZYzhOmXLzBChj8QxntApicJotI5IgnS1dLJzohM9IxCPbhc 4GR99R0jU5ZLldsw3qyGEZ+fT4C6CdV7MQU7olWP+JQKNElrq2/vUlVHE7R1z2AQ zaVhAEfWHMOCIBuwxrkscR2lIix+OWUfXA3B+/zqHErpCtYfKWSYqTSkEF09DqRl N6VzXi/WxME0LkbmtO/P11txqhIkFOOtiZwIddwpfdtc+GGXRNI= =+bdv -----END PGP SIGNATURE----- --hkpK1J7+7uPKUEUC--