Re: [PATCH v4 1/2] lib/net_utils: allow mac_pton() to take a custom delimiter

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Nov 10, 2026 at 12:54:46PM +0000, netdev-bot+sashiko@kernel.org wrote:

> > +/**
> > + * parse_mac_pton - convert a MAC address string using a custom delimiter
> > + * @s: string to be parsed
> > + * @mac: buffer to store the result, left untouched if @s is malformed
> > + * @delim: characters accepted as a separator between the byte pairs
> > + *
> > + * @s must be MAC_ADDR_STR_LEN characters long and use a character from
> > + * @delim at every separator position, e.g. "aa.bb:cc.dd:ee.ff" with ".:".
> > + *
> > + * Return: true on success, false if @s is malformed.
> > + */
> 
> [Severity: Low]
> This isn't a bug, but does this kernel-doc describe what parse_mac_pton()
> actually does?
> 
> The only length check in the function is:
> 
> 	if (strnlen(s, MAC_ADDR_STR_LEN) < MAC_ADDR_STR_LEN)
> 		return false;
> 
> strnlen() is capped at MAC_ADDR_STR_LEN, so this check only rejects
> strings that are too short. The parsing loops read only s[0] through
> s[16], and nothing checks that s[MAC_ADDR_STR_LEN] is '\0'.
> 
> So input such as "aa:bb:cc:dd:ee:ffJUNK", or the 7-octet
> "aa:bb:cc:dd:ee:ff:00", returns true and fills @mac with the first six
> octets. That doesn't fit "@s must be MAC_ADDR_STR_LEN characters long"
> or "false if @s is malformed".

> The runtime behaviour is the same as the old mac_pton(). The current
> callers (the mac_pton() wrapper, and get_ether_addr() in u_ether.c later
> in this series) don't depend on long strings being rejected. However, a
> new caller that trusts this doc could skip its own end-of-string check.

> Could the doc say instead that @s must have at least MAC_ADDR_STR_LEN
> characters and that anything after them is ignored?

Yes, this needs to be fixed in v5. Adding two words "at least" into the
mentioned line of the kernel-doc.

> The other option is
> to reject strings where s[MAC_ADDR_STR_LEN] != '\0', but that would
> change behaviour for existing mac_pton() callers.

Yep, that's why it's a bad idea.

-- 
With Best Regards,
Andy Shevchenko