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

On Thu, Jul 02, 2026 at 01:12:00AM +0200, Stefano Brivio wrote:

On Wed, 1 Jul 2026 15:31:55 +1000 David Gibson wrote:

...

+++ b/parse.c @@ -18,6 +18,7 @@ #include #include #include +#include

#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.

Oh, right, I misuderstood. Should have looked at the kernel code myself to confirm, oops.

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

That won't quite work: the string doesn't end at the end of the interface name. Plus, in my experience you get much better error reporting if you generally try to lexically accept as *much* as possible, then validate, rather than restricting the parser to only accept the minimum it must. I've added a validation of the parsed ifname up in fwd_rule_parse(), instead. I'll post the new spin momentarily. .

...

+ 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

-- 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