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=U3UdKRhv; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id E57F85A026E for ; Wed, 25 Mar 2026 05:44:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1774413865; bh=XuMIA9rTXHuLIP9AekMzkF6DqmjQGpvNZr/h8ykXHuc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=U3UdKRhvCSTUGrOF0rN6KjfDWoNbjxASDx9uYp93aUTsj7jJN5FnKNu8qO5u2xFlr J57D7TB/YSHHXfpBVfZ0TgXCGE1knT5v3JxwWfJvyDeOFn89tX5ROPdyFIgOzX66Va QkIJPmOq4vBcI9PartQPmYOnlYk0MdfOLfgGHw/OSJix0NYQ8zYNDbIZOqvCUECzZc ORYVcIZjgkzfmUFiQqqwg/HMEKAsugx4+0yu11EiHS2jRfIJZbG5gaLaNEGtAd9Ads tuvqQpFxbGkNvYbOtnZY/dCNCQjB2QqerL7JHnR7oCKlL2YJBHViKUYw7sOxGXWPNF dlLW/OGPW2M7g== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fgZ7Y2ZfXz4wM0; Wed, 25 Mar 2026 15:44:25 +1100 (AEDT) Date: Wed, 25 Mar 2026 15:34:02 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 15/25] pesto: Expose list of pifs to pesto Message-ID: References: <20260323073732.3158468-1-david@gibson.dropbear.id.au> <20260323073732.3158468-16-david@gibson.dropbear.id.au> <20260325015621.6f595f6a@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="6EON991/4PaoWXaL" Content-Disposition: inline In-Reply-To: <20260325015621.6f595f6a@elisabeth> Message-ID-Hash: GFVXUBW4V3YN5INTGPKK33WQ755LJDLU X-Message-ID-Hash: GFVXUBW4V3YN5INTGPKK33WQ755LJDLU 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: --6EON991/4PaoWXaL Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 25, 2026 at 01:56:22AM +0100, Stefano Brivio wrote: > On Mon, 23 Mar 2026 18:37:22 +1100 > David Gibson wrote: >=20 > > Extend the dynamic update protocol to expose the pif indices and names > > from a running passt/pasta to the pesto tool. pesto records that data > > and (for now) prints it out. > >=20 > > Signed-off-by: David Gibson > > --- > > conf.c | 38 +++++++++++++++++++++ > > pesto.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++- > > serialise.c | 21 ++++++++++++ > > serialise.h | 3 ++ > > 4 files changed, 159 insertions(+), 1 deletion(-) > >=20 > > diff --git a/conf.c b/conf.c > > index 7b960fe9..b878db94 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -2303,6 +2303,41 @@ void conf(struct ctx *c, int argc, char **argv) > > conf_print(c); > > } > > =20 > > +/** > > + * conf_send_pifs() - Send list of pifs to dynamic update client (pest= o) > > + * @c: Execution context > > + * @fd: Socket to the client > > + * > > + * Return: 0 on success, -1 on failure > > + */ > > +static int conf_send_pifs(const struct ctx *c, int fd) > > +{ > > + uint32_t num =3D 0; > > + unsigned pif; > > + > > + /* First count the number of pifs with tables */ > > + for (pif =3D 0; pif < PIF_NUM_TYPES; pif++) { > > + if (c->fwd[pif]) > > + num++; > > + } > > + > > + if (write_u32(fd, num)) > > + return -1; > > + > > + for (pif =3D 0; pif < PIF_NUM_TYPES; pif++) { > > + if (!c->fwd[pif]) > > + continue; > > + > > + if (write_u8(fd, pif)) > > + return -1; > > + > > + if (write_str(fd, pif_name(pif)) < 0) > > + return -1; > > + } > > + > > + return 0; > > +} > > + > > /** > > * conf_listen_handler() - Handle events on configuration listening so= cket > > * @c: Execution context > > @@ -2359,6 +2394,9 @@ void conf_listen_handler(struct ctx *c, uint32_t = events) > > "Warning: Using experimental unsupported configuration protocol"); > > } > > =20 > > + if (conf_send_pifs(c, fd) < 0) > > + goto fail; > > + > > return; > > =20 > > fail: > > diff --git a/pesto.c b/pesto.c > > index f8c9e01d..ed4f2aab 100644 > > --- a/pesto.c > > +++ b/pesto.c > > @@ -36,6 +36,8 @@ > > #include "serialise.h" > > #include "pesto.h" > > =20 > > +static int verbosity =3D 1; > > + > > #define die(...) \ > > do { \ > > FPRINTF(stderr, __VA_ARGS__); \ > > @@ -51,6 +53,19 @@ > > } \ > > } while (0) > > =20 > > +/** > > + * xmalloc() - Allocate memory, with fatal error on failure > > + * @size: Number of bytes to allocate > > + */ > > +static void *xmalloc(size_t size) > > +{ > > + void *p =3D malloc(size); >=20 > I would really really prefer to skip this unless it's absolutely > necessary, for a number of reasons: >=20 > 1. this thing is talking to passt which talks to untrusted workloads, > and there's a risk of arbitrary code execution, which implies the > possibility of specially crafted messages that might make us > allocate: > > a. in specific regions and build a heap spraying attack based on that >=20 > b. a lot, and cause a denial of service Eh, I suppose. > ...and, while this process should be short lived, we can't assume > that, if somebody achieves arbitrary code execution in passt and > malicious messages, because we risk looping on rules, or having > passt send us information very slowly (minutes / hours), etc. =2E..ah, those are good points. >=20 > 2. besides, we might want to add a "daemon" mode one day, and then it's > not short lived anymore, which opens the door to leaks and > double-frees (adding to a. and b. above) I really don't see any reason we'd want that, but I guess that's irrelevant given that previous paragraph. >=20 > 3. we might need to reuse functions from passt and not notice right > away that they call this xmalloc(), or have to eventually adapt them > anyway I mean.. when we test the path in question, we should hit the seccomp filter, which should make it pretty obvious. > 4. consistency, in messaging ("passt doesn't allocate dynamic memory", > without caveats) and for auditing reasons Yeah, fair. >=20 > Strings can be 32 bytes, or 1024 if we want to splurge. Unless we need > to pile a lot of them on the stack (but it's not the case in pesto) a > malloc() doesn't really bring any advantage compared to static buffers > here and there. It's not megabytes. So, you've convinced me in principle, but I'm not putting this at the top of my priorities. Using malloc() makes things a bit easier while we're playing around with the protocol a bunch. Once we look reasonably close to a v1 protocol, I'll do the malloc() removal. >=20 > > + > > + if (!p) > > + die("Memory allocation failure"); > > + return p; > > +} > > + > > /** > > * usage() - Print usage, exit with given status code > > * @name: Executable name > > @@ -69,6 +84,83 @@ static void usage(const char *name, FILE *f, int sta= tus) > > exit(status); > > } > > =20 > > +/** > > + * pesto_recv_str() - Receive a string from passt/pasta > > + * @fd: Control socket > > + * > > + * Return: pointer to malloc()ed string > > + */ > > +static const char *pesto_recv_str(int fd) > > +{ > > + uint32_t len; > > + char *buf; > > + > > + if (read_u32(fd, &len) < 0) > > + die("Error reading from control socket"); > > + > > + buf =3D xmalloc(len); > > + if (read_all_buf(fd, buf, len) < 0) > > + die("Error reading from control socket"); > > + > > + return buf; > > +} > > + > > +struct pif_state { > > + uint8_t pif; > > + const char *name; > > +}; > > + > > +struct conf_state { >=20 > More as a to-do list item rather than as a review comment: we need > documentation for those structs. Ah, yes. >=20 > > + uint32_t npifs; > > + struct pif_state pif[]; > > +}; > > + > > +/** > > + * pesto_read_pifs() - Read pif names and IDs from passt/pasta > > + * @fd: Control socket > > + */ > > +static const struct conf_state *pesto_read_pifs(int fd) > > +{ > > + uint32_t num; > > + struct conf_state *state; > > + unsigned i; > > + > > + if (read_u32(fd, &num) < 0) > > + die("Error reading from control socket"); > > + > > + debug("Receiving %"PRIu32" interface names", num); > > + > > + state =3D xmalloc(sizeof(*state) + num * sizeof(struct pif_state)); > > + state->npifs =3D num; > > + > > + for (i =3D 0; i < num; i++) { > > + struct pif_state *ps =3D &state->pif[i]; > > + > > + if (read_u8(fd, &ps->pif) < 0) > > + die("Error reading from control socket"); > > + ps->name =3D pesto_recv_str(fd); > > + > > + debug("%u: %s", ps->pif, ps->name); > > + } > > + > > + return state; > > +} > > + > > +/** > > + * show_state() - Show current rule state obtained from passt/pasta > > + * @pifs: PIF name information > > + */ > > +static void show_state(const struct conf_state *state) > > +{ > > + unsigned i; > > + > > + for (i =3D 0; i < state->npifs; i++) { > > + const struct pif_state *ps =3D &state->pif[i]; > > + printf("Forwarding rules for %s interface\n", ps->name); > > + printf("\tTBD\n"); > > + } > > +} > > + > > /** > > * main() - Entry point and whole program with loop > > * @argc: Argument count > > @@ -91,12 +183,12 @@ int main(int argc, char **argv) > > { 0 }, > > }; > > struct sockaddr_un a =3D { AF_UNIX, "" }; > > + const struct conf_state *state; > > const char *optstring =3D "vh"; > > struct pesto_hello hello; > > struct sock_fprog prog; > > int optname, ret, s; > > uint32_t s_version; > > - int verbosity =3D 1; > > =20 > > prog.len =3D (unsigned short)sizeof(filter_pesto) / > > sizeof(filter_pesto[0]); > > @@ -172,5 +264,9 @@ int main(int argc, char **argv) > > "Warning: Using experimental protocol version, client and server must = match\n"); > > } > > =20 > > + state =3D pesto_read_pifs(s); > > + > > + show_state(state); > > + > > exit(0); > > } > > diff --git a/serialise.c b/serialise.c > > index e3ea86e3..94c34d3c 100644 > > --- a/serialise.c > > +++ b/serialise.c > > @@ -19,6 +19,7 @@ > > #include > > #include > > #include > > +#include > > #include > > =20 > > #include "serialise.h" > > @@ -121,6 +122,26 @@ int write_all_buf(int fd, const void *buf, size_t = len) > > return write_all_buf(fd, &beval, sizeof(beval)); \ > > } > > =20 > > +#define be8toh(x) (x) > > +#define htobe8(x) (x) > > + > > +SERIALISE_UINT(8) > > SERIALISE_UINT(32) > > =20 > > #undef SERIALISE_UNIT > > + > > +/** > > + * write_str() - Write a string to an fd in length/value format > > + * @fd: Socket to the client > > + * @s: String to send > > + * > > + * Return: 0 on success, -1 on error > > + */ > > +int write_str(int fd, const char *s) > > +{ > > + uint32_t len =3D strlen(s) + 1; /* Include \0 */ > > + > > + if (write_u32(fd, len) < 0) > > + return -1; >=20 > Even if we don't need to allocate here, I would feel more comfortable > if we passed fixed-size strings only to passt to have a slightly lower > risk of buffer overflows, in general. That's fair. I'd basically already decided to move to fixed length pif names, just haven't implemented it yet. > > + return write_all_buf(fd, s, len); > > +} > > diff --git a/serialise.h b/serialise.h > > index 0a4ed086..7ef35786 100644 > > --- a/serialise.h > > +++ b/serialise.h > > @@ -16,6 +16,9 @@ int write_all_buf(int fd, const void *buf, size_t len= ); > > int read_u##bits(int fd, uint##bits##_t *val); \ > > int write_u##bits(int fd, uint##bits##_t val); > > =20 > > +SERIALISE_UINT_DECL(8) > > SERIALISE_UINT_DECL(32) > > =20 > > +int write_str(int fd, const char *s); > > + > > #endif /* _SERIALISE_H */ >=20 > --=20 > Stefano >=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 --6EON991/4PaoWXaL Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmnDZbkACgkQzQJF27ox 2GcIHQ/+J4bXKAOPrsODCJ78pmhyxG32Iva1S2fkqgpMdwiiSqagCsXQfe2YA7gm Sx4pTNMW0k9tI5fggX9r5njUMmGEmYxW+m+AXJ0l1HPnVQ0tOMeGBJIyGCmkCrTy VS2I4DowdMfTdDm+ztOoSYiDI8iDGwYZxD8YJidmKWI4FOlaD1wqXWdv9ckezjSR 4M2j92FdsNGYLk14vL5I87viNtVZQxYlN2Vlt3Zd+OV5EPBUSeCy+20DGlY1OkTL pd6TQcD5xJpTeQjjKvi24PVnLd5qt5vVLRi9fBSx9/zG2CfQ2Mp5laKp+6rbCa6O FgcUeqdBTxFS37hT9tpEKJwggxCo87iVWJdehX6HQIkalguZ7M17gSc4f2QV6h9G EVEG5esE+cs7XDvNnn4cGigfpt/IY7kW70GiKgvllTTTLe3AHmUUjfzX3WT7Eu+2 JPESghv3/2xlEpepa40AkhNv4ATz3dSyksyASqFniJVtJrvuv8TC7cKXTxc7ASUF AxJhG3STdeofaGqdL2PiKgv7GQPs5l8N+AGiCjhzMdDcKxIJqwKZOg6B7FNDhlg3 vPA2Aop1V4ontMcJ9ywzLeTIhUaAcH6eM0IM9OWBbkbDNWkjDdDQKel9l2+3SFeE 17qDyurBkO5aMiCtIM/imoDIeQqfM7HTDfA0OhN5Kky2Br/Cqhc= =d1xT -----END PGP SIGNATURE----- --6EON991/4PaoWXaL--