[PATCH v4 0/2] usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr

[PATCH v4 0/2] usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr

[Liu Chao]

Rework of

  [PATCH v3] usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr
  https://lore.kernel.org/all/20261105072147.3383565-1-liuc63@xiaopeng.com/

following Andy's review: 1/2 prepares a parse_mac_pton() helper in
lib/net_utils.c, 2/2 uses it in get_ether_addr().

The two patches belong to different trees: 1/2 touches lib/ and should go
via netdev or -mm, 2/2 is for the USB tree.  2/2 depends on 1/2, so it
can only be applied once 1/2 has reached Linus' tree.

Changes since v3:
  - no code change at all, the tree is byte identical to what Andy
    reviewed, only the log of 2/2 changed
  - 1/2: unchanged
  - 2/2: pick up Andy's Reviewed-by
  - 2/2: name 331a2d82bcaf ("[PATCH] USB: ethernet/rndis gadget address
    params") from v2.6.7 as the commit that introduced the loop, and
    record that it only resolves in the history.git tree, which is why
    the series carries no Fixes tag
  - 2/2: record that the Cisco notation and irregular groupings are
    historical and rarely met these days, and that the commit introducing
    the loop never documented accepting them

Changes since v2:
  - 1/2: pick up Andy's Reviewed-by, no code change
  - 2/2: keep the separator-less notation "aabbccddeeff" working by
    falling back to hex2bin() when parse_mac_pton() does not match, as
    suggested by Andy
  - 2/2: drop the Fixes tag, the loop predates the git history
  - 2/2: spell out in the log which notations stop being accepted

Changes since v1:
  - rework the fix on top of parse_mac_pton() as suggested by Andy
    instead of open coding the bounds checks in get_ether_addr()
  - add the KASAN report and the malformed address example that v1 was
    missing, both collected on a KASAN build by loading g_ether with a
    short and with a non-hex dev_addr
  - select GENERIC_NET_UTILS for USB_U_ETHER
  - drop Cc: stable: 2/2 calls parse_mac_pton(), which 1/2 introduces,
    so it cannot be backported to any stable tree on its own

One user visible effect remains: the Cisco notation "aabb.ccdd.eeff" and
irregular groupings such as "123456:789abc" are no longer parsed and now
fall back to a random address.  Details are in the log of 2/2.

Liu Chao (2):
  lib/net_utils: allow mac_pton() to take a custom delimiter
  usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr

 drivers/usb/gadget/Kconfig            |  1 +
 drivers/usb/gadget/function/u_ether.c | 18 ++++++++----------
 include/linux/hex.h                   |  7 ++++++-
 lib/net_utils.c                       | 17 ++++++++++++++---
 4 files changed, 29 insertions(+), 14 deletions(-)


base-commit: a5ebb76233b79db01e82e061173cb73c6d2b5c6b
-- 
2.50.1

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

[Liu Chao]

mac_pton() hardcodes ':' as the only accepted separator, which pushes
callers that need a different notation to open-code their own parser.

Split the body out into parse_mac_pton() taking the set of accepted
delimiters as a string, and turn mac_pton() into a static inline wrapper
passing ":" so that every existing caller keeps its current behaviour.
The strnlen() guard still runs before any delimiter is inspected, so
strchr() is never handed a NUL character.

No functional change intended.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Liu Chao <liuc63@xiaopeng.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
v4: no change.
v3: pick up Andy's Reviewed-by, no code change.
v2: new patch, split out of the u_ether fix at Andy's suggestion.
Link: https://lore.kernel.org/all/20261105072147.3383565-2-liuc63@xiaopeng.com/

 include/linux/hex.h |  7 ++++++-
 lib/net_utils.c     | 17 ++++++++++++++---
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/include/linux/hex.h b/include/linux/hex.h
index 2618382e5b0c..408825478cda 100644
--- a/include/linux/hex.h
+++ b/include/linux/hex.h
@@ -30,6 +30,11 @@ extern int hex_to_bin(unsigned char ch);
 extern int __must_check hex2bin(u8 *dst, const char *src, size_t count);
 extern char *bin2hex(char *dst, const void *src, size_t count);
 
-bool mac_pton(const char *s, u8 *mac);
+bool parse_mac_pton(const char *s, u8 *mac, const char *delim);
+
+static inline bool mac_pton(const char *s, u8 *mac)
+{
+	return parse_mac_pton(s, mac, ":");
+}
 
 #endif
diff --git a/lib/net_utils.c b/lib/net_utils.c
index 215cda672fee..011857cc978b 100644
--- a/lib/net_utils.c
+++ b/lib/net_utils.c
@@ -5,7 +5,18 @@
 #include <linux/export.h>
 #include <linux/hex.h>
 
-bool mac_pton(const char *s, u8 *mac)
+/**
+ * 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.
+ */
+bool parse_mac_pton(const char *s, u8 *mac, const char *delim)
 {
 	int i;
 
@@ -16,7 +27,7 @@ bool mac_pton(const char *s, u8 *mac)
 	for (i = 0; i < ETH_ALEN; i++) {
 		if (!isxdigit(s[i * 3]) || !isxdigit(s[i * 3 + 1]))
 			return false;
-		if (i != ETH_ALEN - 1 && s[i * 3 + 2] != ':')
+		if (i != ETH_ALEN - 1 && !strchr(delim, s[i * 3 + 2]))
 			return false;
 	}
 	for (i = 0; i < ETH_ALEN; i++) {
@@ -24,4 +35,4 @@ bool mac_pton(const char *s, u8 *mac)
 	}
 	return true;
 }
-EXPORT_SYMBOL(mac_pton);
+EXPORT_SYMBOL(parse_mac_pton);
-- 
2.50.1

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The kernel-doc this patch adds for parse_mac_pton() in
  lib/net_utils.c does not match what the function does.

--

lib/net_utils: allow mac_pton() to take a custom delimiter

This commit moves the body of mac_pton() into a new parse_mac_pton(),
which takes a string of accepted separator characters. mac_pton()
becomes a static inline wrapper that passes ":", so existing callers
behave as before.

Link: https://lore.kernel.org/all/20261105072147.3383565-2-liuc63@xiaopeng.com/

> diff --git a/lib/net_utils.c b/lib/net_utils.c
> index 215cda672fee1..011857cc978b4 100644
> --- a/lib/net_utils.c
> +++ b/lib/net_utils.c
> @@ -5,7 +5,18 @@
>  #include <linux/export.h>
>  #include <linux/hex.h>
>  
> -bool mac_pton(const char *s, u8 *mac)
> +/**
> + * 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? The other option is
to reject strings where s[MAC_ADDR_STR_LEN] != '\0', but that would
change behaviour for existing mac_pton() callers.

> +bool parse_mac_pton(const char *s, u8 *mac, const char *delim)
>  {
>  	int i;
>  

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106030605.576584-1-liuc63%40xiaopeng.com

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

[Andy Shevchenko]

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

[PATCH v4 2/2] usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr

[Liu Chao]

get_ether_addr() walks the MAC address string with a fixed six iteration
loop, calling hex_to_bin() twice per iteration, and never checks for the
terminating NUL.  A short string therefore makes it read past the end of
the buffer.  The dev_addr and host_addr module parameters of the legacy
gadget drivers are copied into their own kmalloc'ed buffer by
param_set_charp(), so the overrun crosses a slab boundary and KASAN
reports it at insmod time:

  # insmod g_ether.ko dev_addr=ab

  BUG: KASAN: slab-out-of-bounds in get_ether_addr+0x1ec/0x230
  Read of size 1 at addr ffff888009f91f53 by task insmod/70
  ...
   get_ether_addr+0x1ec/0x230
   gether_set_dev_addr+0x6b/0x180
   eth_bind+0xfd/0x3b0 [g_ether]
   composite_bind+0x1bb/0x7b0
   gadget_bind_driver+0x229/0xb00
  ...
  Allocated by task 70:
   __kmalloc_noprof+0x1d1/0x4a0
   param_set_charp+0x4a/0x220
   parse_args+0x267/0x5c0
   load_module+0x4f6a/0x6600
  ...
  The buggy address is located 0 bytes to the right of
   allocated 19-byte region [ffff888009f91f40, ffff888009f91f53)

Besides reading past the end of the string, the loop ignores what
hex_to_bin() returns: it yields -1 for a character that is not a hex
digit, and that value is shifted and OR'ed into dev_addr unchecked, so a
malformed address can be accepted as a valid one and the interface comes
up with an address that nobody asked for:

  # insmod g_ether.ko dev_addr=aazz:cc:dd:ee:ff
  # cat /sys/class/net/usb0/address
  aa:ff:cc:dd:ee:ff

The configfs dev_addr and host_addr attributes of the ECM and NCM
functions reach the same parser through gether_set_dev_addr() and
gether_set_host_addr().  Their store handler passes the whole configfs
buffer on, so an overrun there stays inside the same object and is never
reported, but a malformed address is silently accepted all the same.

Replace the open coded loop with parse_mac_pton() for the delimited
notations and hex2bin() for the raw one, and keep the existing
eth_random_addr() fallback for everything both of them reject.
parse_mac_pton() checks the length and every digit before writing
anything to dev_addr, and hex2bin() stops at the first character that is
not a hex digit, which the NUL terminator is, so neither reads past the
end of the string.

The notations that keep working are the byte pair ones, ':' and '.', in
any mixture, and the separator-less "aabbccddeeff".  What stops working
is the Cisco notation "aabb.ccdd.eeff" and irregular groupings such as
"123456:789abc", which the old loop only accepted because it skipped a
delimiter wherever it happened to find one.  Supporting them would need
a parser per grouping, so such input now falls back to a random address.

Neither of the two should be missed: they are historical notations that
are rarely met these days, and the commit that introduced the loop never
said that it accepted them.  That commit only explains that it resolves
a FIXME by adding module parameters for stable addresses, so skipping a
delimiter wherever one happens to turn up reads as a simplification of
the parser rather than as support for non-standard formats.

Select GENERIC_NET_UTILS so that parse_mac_pton() is built in.

The loop is older than the git history.  It was introduced by
331a2d82bcaf ("[PATCH] USB: ethernet/rndis gadget address params") from
v2.6.7 in drivers/usb/gadget/ether.c, and commit 2b3d942c4878 ("usb
ethernet gadget: split out network core") copied it into u_ether.c.
331a2d82bcaf predates the git conversion and only resolves in the
historic tree history.git, so a Fixes: tag naming it would not resolve
for most readers and is left out.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Liu Chao <liuc63@xiaopeng.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
v4: pick up Andy's Reviewed-by; name 331a2d82bcaf from v2.6.7 as the
    commit that introduced the loop and note that it only resolves in
    history.git; record that the notations being dropped are historical
    and were never documented as supported.  No code change.
v3: fall back to hex2bin() so that the separator-less notation keeps
    working, as suggested by Andy; drop the Fixes tag, the loop predates
    the git history.
v2: rework on top of parse_mac_pton(), add the KASAN report and the
    malformed address example, select GENERIC_NET_UTILS, drop Cc: stable
    and the Reviewed-by.
Link: https://lore.kernel.org/all/20261105072147.3383565-3-liuc63@xiaopeng.com/

 drivers/usb/gadget/Kconfig            |  1 +
 drivers/usb/gadget/function/u_ether.c | 18 ++++++++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 76521555e3c1..b83c42be686b 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -158,6 +158,7 @@ config USB_U_SERIAL
 
 config USB_U_ETHER
 	tristate
+	select GENERIC_NET_UTILS
 
 config USB_U_AUDIO
 	tristate
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index 59d85d6a84a8..deedea8a19fd 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -686,18 +686,16 @@ static int eth_stop(struct net_device *net)
 static int get_ether_addr(const char *str, u8 *dev_addr)
 {
 	if (str) {
-		unsigned	i;
+		bool valid;
 
-		for (i = 0; i < 6; i++) {
-			unsigned char num;
+		/* Try the delimited formats first */
+		valid = parse_mac_pton(str, dev_addr, ".:");
+		if (valid && is_valid_ether_addr(dev_addr))
+			return 0;
 
-			if ((*str == '.') || (*str == ':'))
-				str++;
-			num = hex_to_bin(*str++) << 4;
-			num |= hex_to_bin(*str++);
-			dev_addr [i] = num;
-		}
-		if (is_valid_ether_addr(dev_addr))
+		/* Fall back to the raw format */
+		valid = hex2bin(dev_addr, str, ETH_ALEN) == 0;
+		if (valid && is_valid_ether_addr(dev_addr))
 			return 0;
 	}
 	eth_random_addr(dev_addr);
-- 
2.50.1

Re: [PATCH v4 2/2] usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message gets the effect of the dropped notations
  wrong for configfs.
- [Low] The new get_ether_addr() still silently accepts malformed
  addresses that have extra characters after the MAC, in both branches.
- [Low] The patch fixes a real slab-out-of-bounds read reported by KASAN
  (with the old loop, `insmod g_ether.ko dev_addr=ab` reads str[3] and…
- [Low] The kernel-doc for parse_mac_pton() does not match its
  implementation, and the new caller depends on the undocumented…

--

usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr

This commit replaces the open-coded MAC parsing loop in get_ether_addr()
with parse_mac_pton() for the ':' and '.' delimited notation and hex2bin()
for the raw 12 digit notation. This fixes an out-of-bounds read on short
strings and the silent acceptance of non-hex characters. It also makes
USB_U_ETHER select GENERIC_NET_UTILS.

Link: https://lore.kernel.org/all/20261105072147.3383565-3-liuc63@xiaopeng.com/

> The configfs dev_addr and host_addr attributes of the ECM and NCM
> functions reach the same parser through gether_set_dev_addr() and
> gether_set_host_addr().

[ ... ]

> The notations that keep working are the byte pair ones, ':' and '.', in
> any mixture, and the separator-less "aabbccddeeff".  What stops working
> is the Cisco notation "aabb.ccdd.eeff" and irregular groupings such as
> "123456:789abc", which the old loop only accepted because it skipped a
> delimiter wherever it happened to find one.  Supporting them would need
> a parser per grouping, so such input now falls back to a random address.

[Severity: Medium]
Is the "falls back to a random address" part accurate for configfs?

It holds for the legacy module parameters, which reach get_ether_addr()
through gether_setup_name(). The configfs path goes through
gether_set_dev_addr() and gether_set_host_addr() instead, and both throw
away the random bytes:

drivers/usb/gadget/function/u_ether.c:gether_set_dev_addr() {
    ...
	if (get_ether_addr(dev_addr, new_addr))
		return -EINVAL;
    ...
}

The store handlers generated by USB_ETHERNET_CONFIGFS_ITEM_ATTR_DEV_ADDR and
USB_ETHERNET_CONFIGFS_ITEM_ATTR_HOST_ADDR in u_ether_configfs.h return that
error to the writer.

"echo aabb.ccdd.eeff > functions/ecm.usb0/dev_addr" used to succeed. The
old loop skipped the '.' and produced aa:bb:cc:dd:ee:ff. Now
parse_mac_pton() fails the length check and hex2bin() fails at the '.'.
The write therefore returns EINVAL and the attribute keeps its old value.
"aabb:cc:dd:ee:ff" behaves the same way.

The message also names only ECM and NCM. The same macros are used in
f_ecm.c, f_eem.c, f_ncm.c, f_rndis.c and f_subset.c. All five are
documented in Documentation/ABI/testing/configfs-usb-gadget-{ecm,eem,ncm,
rndis,subset} with no format restriction.

Could the commit message describe the configfs behaviour change and list
all five affected functions?

> 331a2d82bcaf predates the git conversion and only resolves in the
> historic tree history.git, so a Fixes: tag naming it would not resolve
> for most readers and is left out.

[Severity: Low]
Should this still carry a Fixes: tag?

The function this patch changes, get_ether_addr() in u_ether.c, was
introduced by commit 2b3d942c4878 ("usb ethernet gadget: split out network
core"), and that commit is in git. The usual convention for bugs older
than git is also available:

    Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

Cc: stable was dropped in v2, and there is no Fixes: tag either. Stable
tooling has nothing to match on, so the out-of-bounds read from
"insmod g_ether.ko dev_addr=ab" stays in all maintained stable trees.

The fix also depends on parse_mac_pton() from the preceding commit
"lib/net_utils: allow mac_pton() to take a custom delimiter".

Would a Cc: stable with a dependency note, or a separate minimal fix for
stable, help backporters here?

> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index 59d85d6a84a89..deedea8a19fd7 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -686,18 +686,16 @@ static int eth_stop(struct net_device *net)
>  static int get_ether_addr(const char *str, u8 *dev_addr)
>  {
>  	if (str) {
> -		unsigned	i;
> +		bool valid;
>  
> -		for (i = 0; i < 6; i++) {
> -			unsigned char num;
> +		/* Try the delimited formats first */
> +		valid = parse_mac_pton(str, dev_addr, ".:");

[Severity: Low]
This isn't a bug today, but the kernel-doc for parse_mac_pton() added in
the preceding commit "lib/net_utils: allow mac_pton() to take a custom
delimiter" says:

 * @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.

The implementation only rejects strings shorter than MAC_ADDR_STR_LEN and
ignores everything after the 17th character.

This caller depends on that prefix-only behaviour. The configfs store
handlers in u_ether_configfs.h pass the raw page down, and it normally ends
in '\n':

_f_##_opts_dev_addr_store()
    gether_set_dev_addr(opts->net, page)
        get_ether_addr()
            parse_mac_pton(str, dev_addr, ".:")

So "echo aa:bb:cc:dd:ee:ff > dev_addr" passes 18 characters to
parse_mac_pton().

Suppose the helper were later changed to enforce the exact length that its
documentation describes. Wouldn't every colon notation configfs write then
fail with -EINVAL? The hex2bin() fallback would not help, because it
rejects ':'.

Could the kernel-doc say that only the first MAC_ADDR_STR_LEN characters
are checked? Alternatively, could this caller strip the trailing newline
itself?

> +		if (valid && is_valid_ether_addr(dev_addr))
> +			return 0;
>  
> -			if ((*str == '.') || (*str == ':'))
> -				str++;
> -			num = hex_to_bin(*str++) << 4;
> -			num |= hex_to_bin(*str++);
> -			dev_addr [i] = num;
> -		}
> -		if (is_valid_ether_addr(dev_addr))
> +		/* Fall back to the raw format */
> +		valid = hex2bin(dev_addr, str, ETH_ALEN) == 0;
> +		if (valid && is_valid_ether_addr(dev_addr))
>  			return 0;

[Severity: Low]
Can both branches still silently accept an address with extra characters
after the MAC?

parse_mac_pton() only enforces a minimum length:

lib/net_utils.c:parse_mac_pton() {
	if (strnlen(s, MAC_ADDR_STR_LEN) < MAC_ADDR_STR_LEN)
		return false;
    ...
}

It never looks at s[17] or beyond. "aa:bb:cc:dd:ee:ff:11",
"aa:bb:cc:dd:ee:ffff" and "aa:bb:cc:dd:ee:ffZZZ" all return true.

hex2bin(dev_addr, str, ETH_ALEN) consumes exactly 12 hex digits and returns
0 regardless of what follows. "aabbccddeeff0", "aabbccddeeff00112233" and
"aabbccddeeffzz" are all accepted as aa:bb:cc:dd:ee:ff.

In each of these cases get_ether_addr() returns 0, gether_setup_name()
marks the address NET_ADDR_SET, and the configfs store reports success.

The commit message says parse_mac_pton() "checks the length", but that
check is only a lower bound.

Would it make sense to require str[17] in the delimited case, or str[12] in
the raw case, to be '\0' or '\n'? A single trailing '\n' has to be allowed,
because the configfs store passes the raw page buffer from echo.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106030605.576584-1-liuc63%40xiaopeng.com