Re: [nft PATCH 3/4] netlink: Keep going after set element parsing failures

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, May 21, 2025 at 07:19:19PM +0200, Phil Sutter wrote:
> On Wed, May 21, 2025 at 04:17:49PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, May 21, 2025 at 03:12:41PM +0200, Phil Sutter wrote:
> > > Print an error message and try to deserialize the remaining elements
> > > instead of calling BUG().
> > > 
> > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > ---
> > >  src/netlink.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/netlink.c b/src/netlink.c
> > > index 1222919458bae..3221d9f8ffc93 100644
> > > --- a/src/netlink.c
> > > +++ b/src/netlink.c
> > > @@ -1475,7 +1475,9 @@ int netlink_delinearize_setelem(struct netlink_ctx *ctx,
> > >  		key->byteorder = set->key->byteorder;
> > >  		key->len = set->key->len;
> > >  	} else {
> > > -		BUG("Unexpected set element with no key\n");
> > > +		netlink_io_error(ctx, NULL,
> > > +			         "Unexpected set element with no key\n");
> > > +		return 0;
> > 
> > If set element has no key, then something is very wrong. There is
> > already one exception that is the catch-all element (which has no
> > key).
> 
> Yes, in these cases the ruleset parser fails and the output is very
> likely broken or at least incomplete. This series merely aligns error
> handling: Take netlink_parse_cmp() for instance: If NFTNL_EXPR_CMP_SREG
> attribute is missing or bogus, netlink_error() is called and the
> function returns (void). No error status propagation happens (which we
> could change easily), but most importantly the parser continues to
> deserialize as much as possible.
> 
> > This is enqueuing an error record, but 0 is returned, I am not sure if
> > this is ever going to be printed.
> 
> It does: Forcing the code to enter that third branch, listing a set with three
> elements looks like this:
> 
> % sudo ./src/nft list ruleset
> table ip t {
> 	set s {
> 		type inet_service
> 	}
> }
> netlink: Error: Unexpected set element with no key
> 
> netlink: Error: Unexpected set element with no key
> 
> netlink: Error: Unexpected set element with no key
> 
> > I am not sure this patch works.
> 
> Well, that extra newline is indeed a bug. :)

Go ahead push it out then.