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=202502 header.b=bK6/ZSsM; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 69EB15A061B for ; Thu, 03 Apr 2025 07:38:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1743658684; bh=F1RqXZzMSnnj5t0Vg2uYysFnogWwhTn0l2FW1havs8k=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=bK6/ZSsMhTG7lodjrLK7oJzFEnw4POn3yknM88ULt3l3tuS7gjz5xjlES25expl+m uNGG8QFDrkPziS9YKksWZegdtyiDdndCYb0SMRkfDOowBoo+qpKIgxDCGI0euI8UFj B/P0YrJbNJRdcZFMllHf1CBR8uVZXWtpDsHUwrH4QZNNBHpgw5zRaYxit2AMU0kXe1 4cWeEJ8+wZH1wOcP9i6z/hC2BTdI5/1esOJxQZjHZacIkGVYVCo0OUWsqPNv8I09Rr Kwsj310UR4AQkZ9qxCXWczfn3XQ0VfsKfYAXdrL6WUGAGp6PgKGqFmsI/noIDH5o4E BzcvTQHyciegw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZSr9m5tk0z4xND; Thu, 3 Apr 2025 16:38:04 +1100 (AEDT) Date: Thu, 3 Apr 2025 16:37:56 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 14/18] dhcpv6: Use iov_tail in dhcpv6_opt() Message-ID: References: <20250402172343.858187-1-lvivier@redhat.com> <20250402172343.858187-15-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="BsvYcVT7wzCqpJC4" Content-Disposition: inline In-Reply-To: <20250402172343.858187-15-lvivier@redhat.com> Message-ID-Hash: VG5DSZJLPNGKI4TVN5SXWVMEGDPCND3M X-Message-ID-Hash: VG5DSZJLPNGKI4TVN5SXWVMEGDPCND3M 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: --BsvYcVT7wzCqpJC4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 02, 2025 at 07:23:39PM +0200, Laurent Vivier wrote: Needs a commit message. > Signed-off-by: Laurent Vivier > --- > dhcpv6.c | 57 +++++++++++++++++++++++++++----------------------------- > 1 file changed, 27 insertions(+), 30 deletions(-) >=20 > diff --git a/dhcpv6.c b/dhcpv6.c > index ccc64172a480..1e83f2c2ad23 100644 > --- a/dhcpv6.c > +++ b/dhcpv6.c > @@ -278,30 +278,25 @@ 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: Data to look at > * @type: Option type to look up, network order > * > * Return: pointer to option header, or NULL on malformed or missing opt= ion > */ > -static struct opt_hdr *dhcpv6_opt(const struct pool *p, size_t *offset, > - uint16_t type) > +static struct opt_hdr *dhcpv6_opt(struct iov_tail *data, uint16_t type) > { > - struct opt_hdr *o; > - size_t left; > + struct opt_hdr *o, oc; > =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, oc))) { > unsigned int opt_len =3D ntohs(o->l) + sizeof(*o); > =20 > - if (ntohs(o->l) > left) > + if (opt_len > iov_tail_size(data)) > return NULL; > =20 > if (o->t =3D=3D type) > return o; This is no good. If peek_header() ended up copying the header you'll now be returning a pointer to a local variable. Also, you've only verified the contiguity of the option header with IOV_PEEK_HEADER, the body of the option could still be discontiguous, which is not what the callers expect. > - *offset +=3D opt_len; > + data->off +=3D opt_len; I think you want iov_drop() or REMOVE_HEADER() here rather than reaching into the iov_tail structure. > } > =20 > return NULL; > @@ -309,31 +304,31 @@ static struct opt_hdr *dhcpv6_opt(const struct pool= *p, size_t *offset, > =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 > * @la: Address we want to lease to the client > * > * Return: pointer to non-appropriate IA_NA or IA_TA, if any, NULL other= wise > */ > -static struct opt_hdr *dhcpv6_ia_notonlink(const struct pool *p, > +static struct opt_hdr *dhcpv6_ia_notonlink(const struct iov_tail *data, > struct in6_addr *la) > { > int ia_types[2] =3D { OPT_IA_NA, OPT_IA_TA }, *ia_type; > const struct opt_ia_addr *opt_addr; > char buf[INET6_ADDRSTRLEN]; > struct in6_addr req_addr; > + struct iov_tail current; > 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))) { > + current =3D *data; > + while ((ia =3D dhcpv6_opt(¤t, *ia_type))) { > if (ntohs(ia->l) < OPT_VSIZE(ia_na)) > return NULL; > =20 > - offset +=3D sizeof(struct opt_ia_na); > + current.off +=3D sizeof(struct opt_ia_na); > =20 > - while ((h =3D dhcpv6_opt(p, &offset, OPT_IAAADR))) { > + while ((h =3D dhcpv6_opt(¤t, OPT_IAAADR))) { > if (ntohs(h->l) !=3D OPT_VSIZE(ia_addr)) > return NULL; > =20 > @@ -342,7 +337,7 @@ static struct opt_hdr *dhcpv6_ia_notonlink(const stru= ct pool *p, > if (!IN6_ARE_ADDR_EQUAL(la, &req_addr)) > goto err; > =20 > - offset +=3D sizeof(struct opt_ia_addr); > + current.off +=3D sizeof(struct opt_ia_addr); > } > } > } > @@ -434,13 +429,15 @@ 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(struct iov_tail *data, > + const struct ctx *c, > char *buf, int offset) > =20 > { > @@ -463,9 +460,8 @@ static size_t dhcpv6_client_fqdn_fill(const struct po= ol *p, const struct ctx *c, > =20 > o =3D (struct opt_client_fqdn *)(buf + offset); > 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); > + data->off +=3D UDP_MSG_HDR_SIZE; > + req_opt =3D (struct opt_client_fqdn *)dhcpv6_opt(data, OPT_CLIENT_FQDN); > if (req_opt && req_opt->flags & 0x01 /* S flag */) > o->flags =3D 0x02 /* O flag */; > else > @@ -525,19 +521,19 @@ int dhcpv6(struct ctx *c, const struct pool *p, > =20 > src =3D &c->ip6.our_tap_ll; > =20 > - mh =3D IOV_PEEK_HEADER(&data, mhc); > + mh =3D IOV_REMOVE_HEADER(&data, mhc); > if (!mh) > return -1; > =20 > - client_id =3D dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_CLIENTID= ); > + client_id =3D dhcpv6_opt(&data, OPT_CLIENTID); > 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= ); > + server_id =3D dhcpv6_opt(&data, OPT_SERVERID); > if (server_id && ntohs(server_id->l) !=3D OPT_VSIZE(server_id)) > return -1; > =20 > - ia =3D dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_NA); > + ia =3D dhcpv6_opt(&data, OPT_IA_NA); > if (ia && ntohs(ia->l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta))) > return -1; > =20 > @@ -553,7 +549,7 @@ 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 ((bad_ia =3D dhcpv6_ia_notonlink(&data, &c->ip6.addr))) { > info("DHCPv6: received CONFIRM with inappropriate IA," > " sending NotOnLink status in REPLY"); > =20 > @@ -587,7 +583,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"); > @@ -619,7 +615,8 @@ int dhcpv6(struct ctx *c, const struct pool *p, > 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); > + packet_base(p, 0, &data); > + n =3D dhcpv6_client_fqdn_fill(&data, c, (char *)&resp, n); > =20 > resp.hdr.xid =3D mh->xid; > =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 --BsvYcVT7wzCqpJC4 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmfuHrMACgkQzQJF27ox 2GcEgQ//enPmCfjsGkiV9LySJC5GAUokpFtherhW1C8qQAoh/p7ZNo4JdXdeVxTE bt6ETRTTo5whzom031QpsT53JXUYVKF0znv3GSbJwRf3FbSRGMNcQSSjkdfAbMZa EXn6VFWPEqa2JpdqtYqY2XBk02QH/s3W2NQyViW3Zru8fWs+Gjl+sT6Gk4rsBTKJ O5SvyTkqWOxkaWjpBPhJrSBGVYjET8EaFgxmfnBh0PS162yx6ra7uyDPzWPLijPL GgHJsob3moXmbxPs2OrlEUJOx5wt4+AfL1R5D6OWjWh8Hg0C0okGUUzFvu26rCZC sTQIoBmwoqYW9QzdiTu2GfvzshPTfXB0I8NPHidPi1B+z1pcaddQTzAYNhlbqzPr yWsRq0/pvY42NMzsmIdKD6jOc7RnNpoMumatoVjwRv1BJhid+n+XbEZ/w6v9IjAT fNkucgz1twsxbZHv17osU8ZinlnMa+Y1wmsX2FwnXeU9+M9uJC2azbwLk1WPdjZT 4TuHzmijrf314hO0i7+QIUbGltAR76fKLLrwBR6T9RE1v7sPoSSI2mHgDoCQDt2l 2ZVWEdNzTutBZ6y7p1Nt7nVzXUGIknMcOZsNLY6spFgFmNqV5eCpFiLXrEA4moMM NrG3qxNTdKfkCh5u0/aQtxh81qTovCn/fAZgysr2OqMSypmsZ34= =saUv -----END PGP SIGNATURE----- --BsvYcVT7wzCqpJC4--