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=ERy++Oti; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 716A85A027A for ; Wed, 13 Aug 2025 05:27:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1755055626; bh=L93O2XwlAZrZfigDueD6A32oKw5iB6i+YaND89xHk3o=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ERy++OtiijsrkcgzO9MurqoM5duu3zYqTUsB6hluS48ZI7JNA4QkmBgPrTfpQiHaV d6LVrHtAwL7O837j7PeBn5H1O+qQTEwPTFxrkU1/EVa5PLnwj5RIbZl9vaEkfToS70 lZ8ShW5+NDQtjPRB38b+jWfYngNncNM+JgU+Ss0Uizf+a6u95fykICEdBot/g4WRch /Ls57ZVlgJDQLOkRmBXhZ4EklsfH/akVkdFgDIoaTXtZ5jqlBpo9OvkBWhga1yPQai wWOlvRTGlUYay7w1iElhK4TdOLwgmSB84d+epLrgl/8A46FA6p1GUBfDnSDC/Ui8FT 25qWhtRFWj8sw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4c1v1k4rV8z4xPG; Wed, 13 Aug 2025 13:27:06 +1000 (AEST) Date: Wed, 13 Aug 2025 13:13:50 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v9 16/30] dhcpv6: Use iov_tail in dhcpv6_opt() Message-ID: References: <20250808140142.3404325-1-lvivier@redhat.com> <20250808140142.3404325-17-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="gYOmkF6KDXMJzhWZ" Content-Disposition: inline In-Reply-To: <20250808140142.3404325-17-lvivier@redhat.com> Message-ID-Hash: IKIQYDUMYWIXJP5344BTX2S5RERLH6WV X-Message-ID-Hash: IKIQYDUMYWIXJP5344BTX2S5RERLH6WV 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: --gYOmkF6KDXMJzhWZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Aug 08, 2025 at 04:01:28PM +0200, Laurent Vivier wrote: > dhcpv6_opt() and its callers are refactored for iov_tail option parsing, > replacing direct offset management for improved robustness. >=20 > Its signature is now `bool dhcpv6_opt(iov_tail *data, type)`. `*data` (in= /out) > points to a found option on `true` return or is restored on `false`. > The main dhcpv6() function uses IOV_REMOVE_HEADER for the msg_hdr, then > passes the iov_tail (now at options start) to the new dhcpv6_opt(). >=20 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson > --- > dhcpv6.c | 179 ++++++++++++++++++++++++++++++++----------------------- > iov.c | 1 - > 2 files changed, 104 insertions(+), 76 deletions(-) >=20 > diff --git a/dhcpv6.c b/dhcpv6.c > index ae06e646f92f..e93acaf9955e 100644 > --- a/dhcpv6.c > +++ b/dhcpv6.c > @@ -280,112 +280,125 @@ static struct resp_not_on_link_t { > =20 > /** > * dhcpv6_opt() - Get option from DHCPv6 message > - * @p: Packet pool, single packet with UDP header > - * @offset: Offset to look at, 0: end of header, set to option start > + * @data: Buffer with options, set to matching option on return > * @type: Option type to look up, network order > * > - * Return: pointer to option header, or NULL on malformed or missing opt= ion > + * Return: true if found and @data points to the option header, > + * or false on malformed or missing option and @data is > + * unmodified. > */ > -static struct opt_hdr *dhcpv6_opt(const struct pool *p, size_t *offset, > - uint16_t type) > +static bool dhcpv6_opt(struct iov_tail *data, uint16_t type) > { > - struct opt_hdr *o; > - size_t left; > + struct iov_tail head =3D *data; > + struct opt_hdr o_storage; > + const struct opt_hdr *o; > =20 > - ASSERT(*offset >=3D UDP_MSG_HDR_SIZE); > - > - while ((o =3D packet_get_try(p, 0, *offset, sizeof(*o), &left))) { > + while ((o =3D IOV_PEEK_HEADER(data, o_storage))) { > unsigned int opt_len =3D ntohs(o->l) + sizeof(*o); > =20 > - if (ntohs(o->l) > left) > - return NULL; > + if (opt_len > iov_tail_size(data)) > + break; > =20 > if (o->t =3D=3D type) > - return o; > + return true; > =20 > - *offset +=3D opt_len; > + iov_tail_drop(data, opt_len); > } > =20 > - return NULL; > + *data =3D head; > + return false; > } > =20 > /** > * dhcpv6_ia_notonlink() - Check if any IA contains non-appropriate addr= esses > - * @p: Packet pool, single packet starting from UDP header > + * @data: Data to look at, packet starting from UDP header (input/output) > * @la: Address we want to lease to the client > * > - * Return: pointer to non-appropriate IA_NA or IA_TA, if any, NULL other= wise > + * Return: true and @data points to non-appropriate IA_NA or IA_TA, if a= ny, > + * false otherwise and @data is unmodified > */ > -static struct opt_hdr *dhcpv6_ia_notonlink(const struct pool *p, > - struct in6_addr *la) > +static bool dhcpv6_ia_notonlink(struct iov_tail *data, > + struct in6_addr *la) > { > int ia_types[2] =3D { OPT_IA_NA, OPT_IA_TA }, *ia_type; > + struct opt_ia_addr opt_addr_storage; > const struct opt_ia_addr *opt_addr; > + struct iov_tail current, ia_base; > + struct opt_ia_na ia_storage; > char buf[INET6_ADDRSTRLEN]; > + const struct opt_ia_na *ia; > struct in6_addr req_addr; > + struct opt_hdr h_storage; > const struct opt_hdr *h; > - struct opt_hdr *ia; > - size_t offset; > =20 > foreach(ia_type, ia_types) { > - offset =3D UDP_MSG_HDR_SIZE; > - while ((ia =3D dhcpv6_opt(p, &offset, *ia_type))) { > - if (ntohs(ia->l) < OPT_VSIZE(ia_na)) > - return NULL; > - > - offset +=3D sizeof(struct opt_ia_na); > + current =3D *data; > + while (dhcpv6_opt(¤t, *ia_type)) { > + ia_base =3D current; > + ia =3D IOV_REMOVE_HEADER(¤t, ia_storage); > + if (!ia || ntohs(ia->hdr.l) < OPT_VSIZE(ia_na)) > + goto notfound; > + > + while (dhcpv6_opt(¤t, OPT_IAAADR)) { > + h =3D IOV_PEEK_HEADER(¤t, h_storage); > + if (!h || ntohs(h->l) !=3D OPT_VSIZE(ia_addr)) > + goto notfound; > + > + opt_addr =3D IOV_REMOVE_HEADER(¤t, > + opt_addr_storage); > + if (!opt_addr) > + goto notfound; > =20 > - while ((h =3D dhcpv6_opt(p, &offset, OPT_IAAADR))) { > - if (ntohs(h->l) !=3D OPT_VSIZE(ia_addr)) > - return NULL; > - > - opt_addr =3D (const struct opt_ia_addr *)h; > req_addr =3D opt_addr->addr; > if (!IN6_ARE_ADDR_EQUAL(la, &req_addr)) > - goto err; > - > - offset +=3D sizeof(struct opt_ia_addr); > + goto notonlink; > } > } > } > =20 > - return NULL; > +notfound: > + return false; > =20 > -err: > +notonlink: > info("DHCPv6: requested address %s not on link", > inet_ntop(AF_INET6, &req_addr, buf, sizeof(buf))); > - return ia; > + *data =3D ia_base; > + return true; > } > =20 > /** > * dhcpv6_send_ia_notonlink() - Send NotOnLink status > - * @c: Execution context > - * @ia: Pointer to non-appropriate IA_NA or IA_TA > - * @client_id: Client ID message option > - * xid: Transaction ID for message exchange > + * @c: Execution context > + * @ia_base: Non-appropriate IA_NA or IA_TA base > + * @client_id_base: Client ID message option base > + * @len: Client ID length > + * @xid: Transaction ID for message exchange > */ > -static void dhcpv6_send_ia_notonlink(struct ctx *c, struct opt_hdr *ia, > - const struct opt_hdr *client_id, > - uint32_t xid) > +static void dhcpv6_send_ia_notonlink(struct ctx *c, > + const struct iov_tail *ia_base, > + const struct iov_tail *client_id_base, > + int len, uint32_t xid) > { > const struct in6_addr *src =3D &c->ip6.our_tap_ll; > + struct opt_hdr *ia =3D (struct opt_hdr *)resp_not_on_link.var; > size_t n; > =20 > info("DHCPv6: received CONFIRM with inappropriate IA," > " sending NotOnLink status in REPLY"); > =20 > - ia->l =3D htons(OPT_VSIZE(ia_na) + sizeof(sc_not_on_link)); > - > n =3D sizeof(struct opt_ia_na); > - memcpy(resp_not_on_link.var, ia, n); > + iov_to_buf(&ia_base->iov[0], ia_base->cnt, ia_base->off, > + resp_not_on_link.var, n); > + ia->l =3D htons(OPT_VSIZE(ia_na) + sizeof(sc_not_on_link)); > memcpy(resp_not_on_link.var + n, &sc_not_on_link, > sizeof(sc_not_on_link)); > =20 > n +=3D sizeof(sc_not_on_link); > - memcpy(resp_not_on_link.var + n, client_id, > - sizeof(struct opt_hdr) + ntohs(client_id->l)); > + iov_to_buf(&client_id_base->iov[0], client_id_base->cnt, > + client_id_base->off, resp_not_on_link.var + n, > + sizeof(struct opt_hdr) + len); > =20 > - n +=3D sizeof(struct opt_hdr) + ntohs(client_id->l); > + n +=3D sizeof(struct opt_hdr) + len; > =20 > n =3D offsetof(struct resp_not_on_link_t, var) + n; > =20 > @@ -474,17 +487,19 @@ search: > =20 > /** > * dhcpv6_client_fqdn_fill() - Fill in client FQDN option > + * @data: Data to look at > * @c: Execution context > * @buf: Response message buffer where options will be appended > * @offset: Offset in message buffer for new options > * > * Return: updated length of response message buffer. > */ > -static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct= ctx *c, > +static size_t dhcpv6_client_fqdn_fill(const struct iov_tail *data, > + const struct ctx *c, > char *buf, int offset) > =20 > { > - struct opt_client_fqdn const *req_opt; > + struct iov_tail current =3D *data; > struct opt_client_fqdn *o; > size_t opt_len; > =20 > @@ -502,14 +517,16 @@ static size_t dhcpv6_client_fqdn_fill(const struct = pool *p, const struct ctx *c, > } > =20 > o =3D (struct opt_client_fqdn *)(buf + offset); > + o->flags =3D 0x00; > encode_domain_name(o->domain_name, c->fqdn); > - req_opt =3D (struct opt_client_fqdn *)dhcpv6_opt(p, > - &(size_t){ UDP_MSG_HDR_SIZE }, > - OPT_CLIENT_FQDN); > - if (req_opt && req_opt->flags & 0x01 /* S flag */) > - o->flags =3D 0x02 /* O flag */; > - else > - o->flags =3D 0x00; > + if (dhcpv6_opt(¤t, OPT_CLIENT_FQDN)) { > + struct opt_client_fqdn req_opt_storage; > + struct opt_client_fqdn const *req_opt; > + > + req_opt =3D IOV_PEEK_HEADER(¤t, req_opt_storage); > + if (req_opt && req_opt->flags & 0x01 /* S flag */) > + o->flags =3D 0x02 /* O flag */; > + } > =20 > opt_len++; > =20 > @@ -531,14 +548,18 @@ static size_t dhcpv6_client_fqdn_fill(const struct = pool *p, const struct ctx *c, > int dhcpv6(struct ctx *c, const struct pool *p, > const struct in6_addr *saddr, const struct in6_addr *daddr) > { > - const struct opt_hdr *client_id, *server_id, *ia; > + const struct opt_server_id *server_id =3D NULL; > + struct iov_tail data, opt, client_id_base; > + const struct opt_hdr *client_id =3D NULL; > + struct opt_server_id server_id_storage; > + const struct opt_ia_na *ia =3D NULL; > + struct opt_hdr client_id_storage; > + struct opt_ia_na ia_storage; > const struct in6_addr *src; > struct msg_hdr mh_storage; > const struct msg_hdr *mh; > struct udphdr uh_storage; > const struct udphdr *uh; > - struct opt_hdr *bad_ia; > - struct iov_tail data; > size_t mlen, n; > =20 > if (!packet_data(p, 0, &data)) > @@ -565,20 +586,26 @@ int dhcpv6(struct ctx *c, const struct pool *p, > =20 > src =3D &c->ip6.our_tap_ll; > =20 > - mh =3D IOV_PEEK_HEADER(&data, mh_storage); > + mh =3D IOV_REMOVE_HEADER(&data, mh_storage); > if (!mh) > return -1; > =20 > - client_id =3D dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_CLIENTID= ); > + client_id_base =3D data; > + if (dhcpv6_opt(&client_id_base, OPT_CLIENTID)) > + client_id =3D IOV_PEEK_HEADER(&client_id_base, client_id_storage); > if (!client_id || ntohs(client_id->l) > OPT_VSIZE(client_id)) > return -1; > =20 > - server_id =3D dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_SERVERID= ); > - if (server_id && ntohs(server_id->l) !=3D OPT_VSIZE(server_id)) > + opt =3D data; > + if (dhcpv6_opt(&opt, OPT_SERVERID)) > + server_id =3D IOV_PEEK_HEADER(&opt, server_id_storage); > + if (server_id && ntohs(server_id->hdr.l) !=3D OPT_VSIZE(server_id)) > return -1; > =20 > - ia =3D dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_NA); > - if (ia && ntohs(ia->l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta))) > + opt =3D data; > + if (dhcpv6_opt(&opt, OPT_IA_NA)) > + ia =3D IOV_PEEK_HEADER(&opt, ia_storage); > + if (ia && ntohs(ia->hdr.l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta))) > return -1; > =20 > resp.hdr.type =3D TYPE_REPLY; > @@ -593,9 +620,10 @@ int dhcpv6(struct ctx *c, const struct pool *p, > if (mh->type =3D=3D TYPE_CONFIRM && server_id) > return -1; > =20 > - if ((bad_ia =3D dhcpv6_ia_notonlink(p, &c->ip6.addr))) { > + if (dhcpv6_ia_notonlink(&data, &c->ip6.addr)) { > =20 > - dhcpv6_send_ia_notonlink(c, bad_ia, client_id, mh->xid); > + dhcpv6_send_ia_notonlink(c, &data, &client_id_base, > + ntohs(client_id->l), mh->xid); > =20 > return 1; > } > @@ -607,7 +635,7 @@ int dhcpv6(struct ctx *c, const struct pool *p, > memcmp(&resp.server_id, server_id, sizeof(resp.server_id))) > return -1; > =20 > - if (ia || dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_TA)) > + if (ia || dhcpv6_opt(&data, OPT_IA_TA)) > return -1; > =20 > info("DHCPv6: received INFORMATION_REQUEST, sending REPLY"); > @@ -633,13 +661,14 @@ int dhcpv6(struct ctx *c, const struct pool *p, > if (ia) > resp.ia_na.iaid =3D ((struct opt_ia_na *)ia)->iaid; > =20 > - memcpy(&resp.client_id, client_id, > - ntohs(client_id->l) + sizeof(struct opt_hdr)); > + iov_to_buf(&client_id_base.iov[0], client_id_base.cnt, > + client_id_base.off, &resp.client_id, > + ntohs(client_id->l) + sizeof(struct opt_hdr)); > =20 > n =3D offsetof(struct resp_t, client_id) + > sizeof(struct opt_hdr) + ntohs(client_id->l); > n =3D dhcpv6_dns_fill(c, (char *)&resp, n); > - n =3D dhcpv6_client_fqdn_fill(p, c, (char *)&resp, n); > + n =3D dhcpv6_client_fqdn_fill(&data, c, (char *)&resp, n); > =20 > resp.hdr.xid =3D mh->xid; > =20 > diff --git a/iov.c b/iov.c > index 9d423d0f521e..1d734acdfea6 100644 > --- a/iov.c > +++ b/iov.c > @@ -109,7 +109,6 @@ size_t iov_from_buf(const struct iovec *iov, size_t i= ov_cnt, > * > * Return: the number of bytes successfully copied. > */ > -/* cppcheck-suppress [staticFunction] */ > size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, > size_t offset, void *buf, size_t bytes) > { --=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 --gYOmkF6KDXMJzhWZ Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmicAu0ACgkQzQJF27ox 2GcfKA/9HIHHmSNxUmTDvvW4F/ILcveZ5Dq1kS/9Q1LOiepvgA0nP2H3oD9U1rYB gMNNRriN8UQawePF82BRzH0D0dDlKedG3+0vOydXLNlBS86B3ZIeHWwBs4YjxjNg bJNnhiWyqcIzgjWplstzNfd0NwCJkVrULNQ/PuAwCXGPSSUtbZAFmO3HOamnI9xK dOAAUqmOeTuBAlybAplSn4Jjgw08mC/katgPnt/4pwLGzcrS5kE9Rxaky3vYxeG+ fUkK5cVKSsps/jR+UahnqteB1fba9hXMZ6slGjUPBvPb0tab/E7/dAYDvqxud6Ny 4/J2zCBXz/OgQwGFXNQTd7Z/iAblxONmEk6F2NzpzanABXOBUt1LYftA9Jio/Gi6 AQvifpOa6tVgc4wiejft5+MTd46kpEfZsBpiHlq8Me9SAjElILH3L7PI2ETMT+Ai icKuzVX0DfWNQ5cqcIxdAtYvNzxMX1Lv7MRPm9lozUgsKKdxg9QyLaAMXw3EZBFx yZG2zKKgq8+Mf5zmWB31ow/W+qT4WWNNy+NZLRgr4JkvSDKQYNKkwvHE4vEiyACW p6ey6h+0YBHmBg37+7Ho9KNr/sNAvuE5hALo9e5/7mkQshAJRzlQWK7gUiUyu56x pEytzI8EwkF7MmWmsnrLNKbwCoy9cCBWRvFHMkbPzsBG8xoG4uc= =BOiw -----END PGP SIGNATURE----- --gYOmkF6KDXMJzhWZ--