[PATCH v3] usb: gadget: f_ncm: Fix MAC assignment NCM ethernet

[PATCH v3] usb: gadget: f_ncm: Fix MAC assignment NCM ethernet

[raub camaioni]

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>
---
 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 58b0dd575af3..186fbb9d0a3d 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1463,6 +1463,8 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
 
 	ncm_opts->bound = true;
 
+	ncm_string_defs[1].s = ecm->ethaddr;
+
 	us = usb_gstrings_attach(cdev, ncm_strings,
 				 ARRAY_SIZE(ncm_string_defs));
 	if (IS_ERR(us)) {
@@ -1771,7 +1773,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);
-- 
2.34.1

Re: [PATCH v3] usb: gadget: f_ncm: Fix MAC assignment NCM ethernet

[Greg Kroah-Hartman]

On Thu, Aug 14, 2025 at 11:50:51AM -0400, raub camaioni wrote:
> 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>
> ---
>  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 58b0dd575af3..186fbb9d0a3d 100644
> --- a/drivers/usb/gadget/function/f_ncm.c
> +++ b/drivers/usb/gadget/function/f_ncm.c
> @@ -1463,6 +1463,8 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
>  
>  	ncm_opts->bound = true;
>  
> +	ncm_string_defs[1].s = ecm->ethaddr;
> +
>  	us = usb_gstrings_attach(cdev, ncm_strings,
>  				 ARRAY_SIZE(ncm_string_defs));
>  	if (IS_ERR(us)) {
> @@ -1771,7 +1773,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);
> -- 
> 2.34.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

[PATCH v3] usb: gadget: f_ncm: Fix MAC assignment NCM ethernet

[raub camaioni]

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>
---
V2 -> V3: Whitespace and line length style errors
V1 -> V2: From and Signed-off set to be identical

 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 58b0dd575af3..186fbb9d0a3d 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1463,6 +1463,8 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
 
 	ncm_opts->bound = true;
 
+	ncm_string_defs[1].s = ecm->ethaddr;
+
 	us = usb_gstrings_attach(cdev, ncm_strings,
 				 ARRAY_SIZE(ncm_string_defs));
 	if (IS_ERR(us)) {
@@ -1771,7 +1773,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);
-- 
2.34.1

Re: [PATCH v3] usb: gadget: f_ncm: Fix MAC assignment NCM ethernet

[kernel test robot]

Hi raub,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus westeri-thunderbolt/next linus/master v6.17-rc1 next-20250814]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/raub-camaioni/usb-gadget-f_ncm-Fix-MAC-assignment-NCM-ethernet/20250815-011301
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20250814171125.635429-1-raubcameo%40gmail.com
patch subject: [PATCH v3] usb: gadget: f_ncm: Fix MAC assignment NCM ethernet
config: i386-randconfig-011-20250815 (https://download.01.org/0day-ci/archive/20250815/202508151101.dzCtG6Wh-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250815/202508151101.dzCtG6Wh-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508151101.dzCtG6Wh-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/usb/gadget/function/f_ncm.c: In function 'ncm_bind':
>> drivers/usb/gadget/function/f_ncm.c:1466:32: error: 'ecm' undeclared (first use in this function); did you mean 'ncm'?
    1466 |         ncm_string_defs[1].s = ecm->ethaddr;
         |                                ^~~
         |                                ncm
   drivers/usb/gadget/function/f_ncm.c:1466:32: note: each undeclared identifier is reported only once for each function it appears in


vim +1466 drivers/usb/gadget/function/f_ncm.c

  1429	
  1430	static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
  1431	{
  1432		struct usb_composite_dev *cdev = c->cdev;
  1433		struct f_ncm		*ncm = func_to_ncm(f);
  1434		struct usb_string	*us;
  1435		int			status = 0;
  1436		struct usb_ep		*ep;
  1437		struct f_ncm_opts	*ncm_opts;
  1438	
  1439		if (!can_support_ecm(cdev->gadget))
  1440			return -EINVAL;
  1441	
  1442		ncm_opts = container_of(f->fi, struct f_ncm_opts, func_inst);
  1443	
  1444		if (cdev->use_os_string) {
  1445			f->os_desc_table = kzalloc(sizeof(*f->os_desc_table),
  1446						   GFP_KERNEL);
  1447			if (!f->os_desc_table)
  1448				return -ENOMEM;
  1449			f->os_desc_n = 1;
  1450			f->os_desc_table[0].os_desc = &ncm_opts->ncm_os_desc;
  1451		}
  1452	
  1453		mutex_lock(&ncm_opts->lock);
  1454		gether_set_gadget(ncm_opts->net, cdev->gadget);
  1455		if (!ncm_opts->bound) {
  1456			ncm_opts->net->mtu = (ncm_opts->max_segment_size - ETH_HLEN);
  1457			status = gether_register_netdev(ncm_opts->net);
  1458		}
  1459		mutex_unlock(&ncm_opts->lock);
  1460	
  1461		if (status)
  1462			goto fail;
  1463	
  1464		ncm_opts->bound = true;
  1465	
> 1466		ncm_string_defs[1].s = ecm->ethaddr;
  1467	
  1468		us = usb_gstrings_attach(cdev, ncm_strings,
  1469					 ARRAY_SIZE(ncm_string_defs));
  1470		if (IS_ERR(us)) {
  1471			status = PTR_ERR(us);
  1472			goto fail;
  1473		}
  1474		ncm_control_intf.iInterface = us[STRING_CTRL_IDX].id;
  1475		ncm_data_nop_intf.iInterface = us[STRING_DATA_IDX].id;
  1476		ncm_data_intf.iInterface = us[STRING_DATA_IDX].id;
  1477		ecm_desc.iMACAddress = us[STRING_MAC_IDX].id;
  1478		ncm_iad_desc.iFunction = us[STRING_IAD_IDX].id;
  1479	
  1480		/* allocate instance-specific interface IDs */
  1481		status = usb_interface_id(c, f);
  1482		if (status < 0)
  1483			goto fail;
  1484		ncm->ctrl_id = status;
  1485		ncm_iad_desc.bFirstInterface = status;
  1486	
  1487		ncm_control_intf.bInterfaceNumber = status;
  1488		ncm_union_desc.bMasterInterface0 = status;
  1489	
  1490		if (cdev->use_os_string)
  1491			f->os_desc_table[0].if_id =
  1492				ncm_iad_desc.bFirstInterface;
  1493	
  1494		status = usb_interface_id(c, f);
  1495		if (status < 0)
  1496			goto fail;
  1497		ncm->data_id = status;
  1498	
  1499		ncm_data_nop_intf.bInterfaceNumber = status;
  1500		ncm_data_intf.bInterfaceNumber = status;
  1501		ncm_union_desc.bSlaveInterface0 = status;
  1502	
  1503		ecm_desc.wMaxSegmentSize = cpu_to_le16(ncm_opts->max_segment_size);
  1504	
  1505		status = -ENODEV;
  1506	
  1507		/* allocate instance-specific endpoints */
  1508		ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_in_desc);
  1509		if (!ep)
  1510			goto fail;
  1511		ncm->port.in_ep = ep;
  1512	
  1513		ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_out_desc);
  1514		if (!ep)
  1515			goto fail;
  1516		ncm->port.out_ep = ep;
  1517	
  1518		ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_notify_desc);
  1519		if (!ep)
  1520			goto fail;
  1521		ncm->notify = ep;
  1522	
  1523		status = -ENOMEM;
  1524	
  1525		/* allocate notification request and buffer */
  1526		ncm->notify_req = usb_ep_alloc_request(ep, GFP_KERNEL);
  1527		if (!ncm->notify_req)
  1528			goto fail;
  1529		ncm->notify_req->buf = kmalloc(NCM_STATUS_BYTECOUNT, GFP_KERNEL);
  1530		if (!ncm->notify_req->buf)
  1531			goto fail;
  1532		ncm->notify_req->context = ncm;
  1533		ncm->notify_req->complete = ncm_notify_complete;
  1534	
  1535		/*
  1536		 * support all relevant hardware speeds... we expect that when
  1537		 * hardware is dual speed, all bulk-capable endpoints work at
  1538		 * both speeds
  1539		 */
  1540		hs_ncm_in_desc.bEndpointAddress = fs_ncm_in_desc.bEndpointAddress;
  1541		hs_ncm_out_desc.bEndpointAddress = fs_ncm_out_desc.bEndpointAddress;
  1542		hs_ncm_notify_desc.bEndpointAddress =
  1543			fs_ncm_notify_desc.bEndpointAddress;
  1544	
  1545		ss_ncm_in_desc.bEndpointAddress = fs_ncm_in_desc.bEndpointAddress;
  1546		ss_ncm_out_desc.bEndpointAddress = fs_ncm_out_desc.bEndpointAddress;
  1547		ss_ncm_notify_desc.bEndpointAddress =
  1548			fs_ncm_notify_desc.bEndpointAddress;
  1549	
  1550		status = usb_assign_descriptors(f, ncm_fs_function, ncm_hs_function,
  1551				ncm_ss_function, ncm_ss_function);
  1552		if (status)
  1553			goto fail;
  1554	
  1555		/*
  1556		 * NOTE:  all that is done without knowing or caring about
  1557		 * the network link ... which is unavailable to this code
  1558		 * until we're activated via set_alt().
  1559		 */
  1560	
  1561		ncm->port.open = ncm_open;
  1562		ncm->port.close = ncm_close;
  1563	
  1564		hrtimer_setup(&ncm->task_timer, ncm_tx_timeout, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
  1565	
  1566		DBG(cdev, "CDC Network: IN/%s OUT/%s NOTIFY/%s\n",
  1567				ncm->port.in_ep->name, ncm->port.out_ep->name,
  1568				ncm->notify->name);
  1569		return 0;
  1570	
  1571	fail:
  1572		kfree(f->os_desc_table);
  1573		f->os_desc_n = 0;
  1574	
  1575		if (ncm->notify_req) {
  1576			kfree(ncm->notify_req->buf);
  1577			usb_ep_free_request(ncm->notify, ncm->notify_req);
  1578		}
  1579	
  1580		ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
  1581	
  1582		return status;
  1583	}
  1584	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki