Re: [PATCH v2 13/13] fwd_rule: Rewrite forward rule parsing using parse.c helpers

[Stefano Brivio <sbrivio@redhat.com>]

On Wed,  1 Jul 2026 15:31:55 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> +++ b/parse.c
> @@ -18,6 +18,7 @@
>  #include <string.h>
>  #include <limits.h>
>  #include <arpa/inet.h>
> +#include <net/if.h>
>  
>  #include "common.h"
>  #include "parse.h"
> @@ -213,3 +214,31 @@ bool parse_inany_(const char **cursor, union inany_addr *addr,
>  
>  	return false;
>  }
> +
> +/**
> + * parse_ifspec() - Parse a interface name specifier (starting with %)
> + * @ifname:	On success updated with parsed name (must have IFNAMSIZ space)
> + *
> + * This will accept a missing specifier (empty string), setting ifname to ""
> + */
> +bool parse_ifspec(const char **cursor, char *ifname)
> +{
> +	const char *p = *cursor;
> +	size_t len;
> +
> +	if (!parse_literal(&p, "%")) {
> +		/* No interface specifier */
> +		ifname[0] = '\0';
> +		return true;
> +	}
> +
> +	/* ifnames can have anything that's not '/', '.' or whitespace */
> +	len = strcspn(p, "/. \f\n\r\t\v");

I was about to apply the series when I spotted this: '.' is definitely
valid in interface names, and even commonly used (think of VLANs), it's
just "." and ".." _as an interface name_ that are not supported.

If we forbid any interface name with '.' we might very well break
somebody's use case. I can fix this up with:

	/* ifnames can't contain /, :, or whitespace */
	len = strcspn(p, "/: \f\n\r\t\v");
	if (!len || len >= IFNAMSIZ)
		return false;

	/* ...and they can't be '..' or '.' either */
	if (!strcmp(p, ".") || !strcmp(p, ".."))
		return false;

if you're fine with it.

> +	if (!len || len >= IFNAMSIZ)
> +		return false;
> +
> +	memcpy(ifname, p, len);
> +	ifname[len] = '\0';
> +	*cursor = p + len;
> +	return true;
> +}
> diff --git a/parse.h b/parse.h
> index ab1d5adb..257b9c58 100644
> --- a/parse.h
> +++ b/parse.h
> @@ -32,4 +32,6 @@ bool parse_inany_(const char **cursor, union inany_addr *addr,
>  
>  #define parse_inany(cursor, addr)	parse_inany_((cursor), (addr), NULL)
>  
> +bool parse_ifspec(const char **cursor, char *ifname);
> +
>  #endif /* _PARSE_H */

-- 
Stefano