From: raub camaioni raubcameo@gmail.com
[ Upstream commit 956606bafb5fc6e5968aadcda86fc0037e1d7548 ]
This fix is already present in f_ecm.c and was never propagated to f_ncm.c
When creating multiple NCM ethernet devices on a composite usb gadget device each MAC address on the HOST side will be identical. Having the same MAC on different network interfaces is bad.
This fix updates the MAC address inside the ncm_strings_defs global during the ncm_bind call. This ensures each device has a unique MAC. In f_ecm.c ecm_string_defs is updated in the same way.
The defunct MAC assignment in ncm_alloc has been removed.
Signed-off-by: raub camaioni raubcameo@gmail.com Link: https://lore.kernel.org/r/20250815131358.1047525-1-raubcameo@gmail.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Explanation
- Problem fixed - Multiple NCM functions in a composite gadget end up advertising the same host-side MAC address because `f_ncm` writes the global string table once during allocation, not per-instance at bind time. In current code, `ncm_alloc()` sets the global string entry to the instance’s MAC: `drivers/usb/gadget/function/f_ncm.c:1738`. This global pointer is then reused for subsequent functions and can be overwritten before each function binds, causing duplicate MAC strings on the host for multiple NCM interfaces.
- What the patch changes - Moves MAC string assignment into bind: - Adds `ncm_string_defs[1].s = ncm->ethaddr;` immediately before the per-function call to `usb_gstrings_attach()` (i.e., just before `drivers/usb/gadget/function/f_ncm.c:1475`). That ensures each function instance updates the global strings to its own MAC right before string IDs are assigned. - Removes the early (and unsafe) assignment from allocation: - Deletes `ncm_string_defs[STRING_MAC_IDX].s = ncm->ethaddr;` from `ncm_alloc()` (currently at `drivers/usb/gadget/function/f_ncm.c:1738`). - The string ID is then bound to that instance via `ecm_desc.iMACAddress = us[STRING_MAC_IDX].id;` (`drivers/usb/gadget/function/f_ncm.c:1484`).
- Why this works (and matches ECM) - `usb_gstrings_attach()` assigns IDs for the string table entries at bind time and ties them into the composite device’s string tables. Updating the MAC string just before that call ensures each NCM function’s `iMACAddress` points to a unique string for that instance. - `f_ecm` has used this pattern since 2018: it assigns the MAC string in `ecm_bind()` right before `usb_gstrings_attach()` (`drivers/usb/gadget/function/f_ecm.c:715`), avoiding exactly this issue. This patch makes `f_ncm` consistent with the proven ECM approach.
- Impact and risk assessment - Bug fix scope is small and contained to `f_ncm` string handling: one added assignment in `ncm_bind()` and removal of the old one in `ncm_alloc()`. No API or structural changes. - Side effects are minimal: the per-function MAC string is set at the correct time; no change to descriptors other than ensuring the correct, unique `iMACAddress`. - Concurrency/regression risk is low: configfs binds functions sequentially; `ncm->ethaddr` is computed during allocation and is stable by bind; no timing-sensitive changes. - Subsystem is the USB gadget function driver (not a critical core subsystem); change mirrors an established, stable pattern in ECM.
- Stable backport criteria - Fixes a real user-visible bug (duplicate MAC addresses on host for multiple NCM functions), which can break networking and cause ARP/ND confusion. - Small, surgical change, no new features or architectural shifts. - Mirrors an existing fix in `f_ecm`, reducing risk. - Signed-off-by from Greg Kroah-Hartman indicates appropriateness for stable. The commit message is clear, with rationale and minimal scope.
- Backport notes - In some trees, the code around `ncm_bind()` may differ slightly (e.g., presence/absence of `max_segment_size`/MTU handling), but the core fix is independent: set `ncm_string_defs[STRING_MAC_IDX].s = ncm->ethaddr;` immediately before `usb_gstrings_attach()`, and remove the assignment from `ncm_alloc()`. - Index usage (`STRING_MAC_IDX` vs `[1]`) is equivalent in current kernels; prefer the macro where available for clarity.
Overall, this is a straightforward, low-risk bug fix that prevents duplicate MAC addresses across multiple NCM functions and aligns `f_ncm` with the established `f_ecm` practice. Backporting is advisable.
drivers/usb/gadget/function/f_ncm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c index 0148d60926dcf..0e38330271d5a 100644 --- a/drivers/usb/gadget/function/f_ncm.c +++ b/drivers/usb/gadget/function/f_ncm.c @@ -1465,6 +1465,8 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
ncm_opts->bound = true;
+ ncm_string_defs[1].s = ncm->ethaddr; + us = usb_gstrings_attach(cdev, ncm_strings, ARRAY_SIZE(ncm_string_defs)); if (IS_ERR(us)) @@ -1759,7 +1761,6 @@ static struct usb_function *ncm_alloc(struct usb_function_instance *fi) mutex_unlock(&opts->lock); return ERR_PTR(-EINVAL); } - ncm_string_defs[STRING_MAC_IDX].s = ncm->ethaddr;
spin_lock_init(&ncm->lock); ncm_reset_values(ncm);