* [PATCH v4 1/2] lib/net_utils: allow mac_pton() to take a custom delimiter
2026-11-06 3:06 [PATCH v4 0/2] usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr Liu Chao
@ 2026-11-06 3:06 ` Liu Chao
2026-11-06 3:06 ` [PATCH v4 2/2] usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr Liu Chao
1 sibling, 0 replies; 3+ messages in thread
From: Liu Chao @ 2026-11-06 3:06 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andrew Morton
Cc: Andy Shevchenko, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, khtsai, fangxy, Randy Dunlap,
dbrownell, linux-usb, linux-kernel, netdev
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
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v4 2/2] usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr
2026-11-06 3:06 [PATCH v4 0/2] usb: gadget: u_ether: fix out-of-bounds read in get_ether_addr Liu Chao
2026-11-06 3:06 ` [PATCH v4 1/2] lib/net_utils: allow mac_pton() to take a custom delimiter Liu Chao
@ 2026-11-06 3:06 ` Liu Chao
1 sibling, 0 replies; 3+ messages in thread
From: Liu Chao @ 2026-11-06 3:06 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andrew Morton
Cc: Andy Shevchenko, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, khtsai, fangxy, Randy Dunlap,
dbrownell, linux-usb, linux-kernel, netdev
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
^ permalink raw reply [flat|nested] 3+ messages in thread