Re: [PATCH v5 17/18] pesto, conf: Send updated rules from pesto back to passt/pasta

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 24 Apr 2026 18:38:57 -0400
Jon Maloy <jmaloy@redhat.com> wrote:

> On 2026-04-21 02:25, David Gibson wrote:
> > Extend pesto to send the updated rule configuration back to passt/pasta.
> > Extend passt/pasta to read the new configuration and store the new rules in
> > a "pending" table.   We don't yet attempt to activate them.
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > Message-ID: <20260322141843.4095972-3-sbrivio@redhat.com>
> > [dwg: Based on an early draft from Stefano]\
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>  
> 
> [...]
> >   
> > +/**
> > + * conf_recv_rules() - Receive forwarding rules from configuration client
> > + * @c:		Execution context
> > + * @fd:		Socket to the client
> > + *
> > + * Return: 0 on success, -1 on failure
> > + */
> > +static int conf_recv_rules(const struct ctx *c, int fd)
> > +{
> > +	while (1) {
> > +		struct fwd_table *fwd;
> > +		struct fwd_rule r;
> > +		uint32_t count;
> > +		uint8_t pif;
> > +		unsigned i;
> > +
> > +		if (read_u8(fd, &pif))
> > +			return -1;
> > +
> > +		if (pif == PIF_NONE)
> > +			break;
> > +
> > +		if (pif >= ARRAY_SIZE(c->fwd_pending) ||
> > +		    !(fwd = c->fwd_pending[pif])) {
> > +			err("Received rules for non-existent table");
> > +			return -1;
> > +		}
> > +
> > +		if (read_u32(fd, &count))
> > +			return -1;
> > +
> > +		if (count > MAX_FWD_RULES) {
> > +			err("Received %"PRIu32" rules (maximum %u)",
> > +			    count, MAX_FWD_RULES);
> > +			return -1;
> > +		}
> > +
> > +		for (i = 0; i < count; i++) {
> > +			fwd_rule_read(fd, &r);  
> 
> Since we don't check the return value I think we risk passing an only 
> partially initialized fwd_rule to fwd_rule_add() if the read fails.
> Maybe:
>    if (fwd_rule_read(fd, &r))
>        return -1;

Right, yes, that makes sense in general, even though I think this will
need a small rework (I didn't get to that yet) to implement this point
of the to-do list (see cover letter):

> - Don't allow a client which sends a partial configuration then
>   blocks also block passt

...because at that point we'll want to permit partial reads and keep a
buffer with a counter of received bytes (perhaps rules / PIFs too).

But actually it doesn't even need to be in this series or in a first
implementation. It could simply be a limitation (in that case, I'll add
the return -1 you suggest).

A user who can connect to passt could anyway configure it to be useless
so I don't see any particular security concern with it.

-- 
Stefano