Commit 37893bc5 authored by Kuen-Han Tsai's avatar Kuen-Han Tsai Committed by Greg Kroah-Hartman
Browse files

Revert "usb: gadget: f_ncm: align net_device lifecycle with bind/unbind"



This reverts commit 56a512a9.

This commit is being reverted as part of a series-wide revert.

By deferring the net_device allocation to the bind() phase, a single
function instance will spawn multiple network devices if it is symlinked
to multiple USB configurations.

This causes regressions for userspace tools (like the postmarketOS DHCP
daemon) that rely on reading the interface name (e.g., "usb0") from
configfs. Currently, configfs returns the template "usb%d", causing the
userspace network setup to fail.

Crucially, because this patch breaks the 1:1 mapping between the
function instance and the network device, this naming issue cannot
simply be patched. Configfs only exposes a single 'ifname' attribute per
instance, making it impossible to accurately report the actual interface
name when multiple underlying network devices can exist for that single
instance.

All configurations tied to the same function instance are meant to share
a single network device. Revert this change to restore the 1:1 mapping
by allocating the network device at the instance level (alloc_inst).

Reported-by: default avatarDavid Heidelberg <david@ixit.cz>
Closes: https://lore.kernel.org/linux-usb/70b558ea-a12e-4170-9b8e-c951131249af@ixit.cz/


Fixes: 56a512a9 ("usb: gadget: f_ncm: align net_device lifecycle with bind/unbind")
Cc: stable <stable@kernel.org>
Signed-off-by: default avatarKuen-Han Tsai <khtsai@google.com>
Link: https://patch.msgid.link/20260309-f-ncm-revert-v2-3-ea2afbc7d9b2@google.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent f2524c0e
Loading
Loading
Loading
Loading
+65 −63
Original line number Diff line number Diff line
@@ -83,11 +83,6 @@ static inline struct f_ncm *func_to_ncm(struct usb_function *f)
	return container_of(f, struct f_ncm, port.func);
}

static inline struct f_ncm_opts *func_to_ncm_opts(struct usb_function *f)
{
	return container_of(f->fi, struct f_ncm_opts, func_inst);
}

/*-------------------------------------------------------------------------*/

/*
@@ -864,7 +859,6 @@ static int ncm_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
{
	struct f_ncm		*ncm = func_to_ncm(f);
	struct f_ncm_opts	*opts = func_to_ncm_opts(f);
	struct usb_composite_dev *cdev = f->config->cdev;

	/* Control interface has only altsetting 0 */
@@ -887,10 +881,9 @@ static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
		if (alt > 1)
			goto fail;

		scoped_guard(mutex, &opts->lock)
			if (opts->net) {
		if (ncm->netdev) {
			DBG(cdev, "reset ncm\n");
				opts->net = NULL;
			ncm->netdev = NULL;
			gether_disconnect(&ncm->port);
			ncm_reset_values(ncm);
		}
@@ -926,8 +919,7 @@ static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
			net = gether_connect(&ncm->port);
			if (IS_ERR(net))
				return PTR_ERR(net);
			scoped_guard(mutex, &opts->lock)
				opts->net = net;
			ncm->netdev = net;
		}

		spin_lock(&ncm->lock);
@@ -1374,14 +1366,12 @@ static int ncm_unwrap_ntb(struct gether *port,
static void ncm_disable(struct usb_function *f)
{
	struct f_ncm		*ncm = func_to_ncm(f);
	struct f_ncm_opts	*opts = func_to_ncm_opts(f);
	struct usb_composite_dev *cdev = f->config->cdev;

	DBG(cdev, "ncm deactivated\n");

	scoped_guard(mutex, &opts->lock)
		if (opts->net) {
			opts->net = NULL;
	if (ncm->netdev) {
		ncm->netdev = NULL;
		gether_disconnect(&ncm->port);
	}

@@ -1443,44 +1433,39 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
{
	struct usb_composite_dev *cdev = c->cdev;
	struct f_ncm		*ncm = func_to_ncm(f);
	struct f_ncm_opts	*ncm_opts = func_to_ncm_opts(f);
	struct usb_string	*us;
	int			status = 0;
	struct usb_ep		*ep;
	struct f_ncm_opts	*ncm_opts;

	struct usb_os_desc_table	*os_desc_table __free(kfree) = NULL;
	struct net_device		*netdev __free(free_gether_netdev) = NULL;
	struct usb_request		*request __free(free_usb_request) = NULL;

	if (!can_support_ecm(cdev->gadget))
		return -EINVAL;

	ncm_opts = container_of(f->fi, struct f_ncm_opts, func_inst);

	if (cdev->use_os_string) {
		os_desc_table = kzalloc(sizeof(*os_desc_table), GFP_KERNEL);
		if (!os_desc_table)
			return -ENOMEM;
	}

	netdev = gether_setup_default();
	if (IS_ERR(netdev))
		return -ENOMEM;

	scoped_guard(mutex, &ncm_opts->lock) {
		gether_apply_opts(netdev, &ncm_opts->net_opts);
		netdev->mtu = ncm_opts->max_segment_size - ETH_HLEN;
	mutex_lock(&ncm_opts->lock);
	gether_set_gadget(ncm_opts->net, cdev->gadget);
	if (!ncm_opts->bound) {
		ncm_opts->net->mtu = (ncm_opts->max_segment_size - ETH_HLEN);
		status = gether_register_netdev(ncm_opts->net);
	}
	mutex_unlock(&ncm_opts->lock);

	gether_set_gadget(netdev, cdev->gadget);
	status = gether_register_netdev(netdev);
	if (status)
		return status;

	/* export host's Ethernet address in CDC format */
	status = gether_get_host_addr_cdc(netdev, ncm->ethaddr,
					  sizeof(ncm->ethaddr));
	if (status < 12)
		return -EINVAL;
	ncm_string_defs[STRING_MAC_IDX].s = ncm->ethaddr;
	ncm_opts->bound = true;

	ncm_string_defs[1].s = ncm->ethaddr;

	us = usb_gstrings_attach(cdev, ncm_strings,
				 ARRAY_SIZE(ncm_string_defs));
@@ -1578,8 +1563,6 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
		f->os_desc_n = 1;
	}
	ncm->notify_req = no_free_ptr(request);
	ncm->netdev = no_free_ptr(netdev);
	ncm->port.ioport = netdev_priv(ncm->netdev);

	DBG(cdev, "CDC Network: IN/%s OUT/%s NOTIFY/%s\n",
			ncm->port.in_ep->name, ncm->port.out_ep->name,
@@ -1594,19 +1577,19 @@ static inline struct f_ncm_opts *to_f_ncm_opts(struct config_item *item)
}

/* f_ncm_item_ops */
USB_ETHER_OPTS_ITEM(ncm);
USB_ETHERNET_CONFIGFS_ITEM(ncm);

/* f_ncm_opts_dev_addr */
USB_ETHER_OPTS_ATTR_DEV_ADDR(ncm);
USB_ETHERNET_CONFIGFS_ITEM_ATTR_DEV_ADDR(ncm);

/* f_ncm_opts_host_addr */
USB_ETHER_OPTS_ATTR_HOST_ADDR(ncm);
USB_ETHERNET_CONFIGFS_ITEM_ATTR_HOST_ADDR(ncm);

/* f_ncm_opts_qmult */
USB_ETHER_OPTS_ATTR_QMULT(ncm);
USB_ETHERNET_CONFIGFS_ITEM_ATTR_QMULT(ncm);

/* f_ncm_opts_ifname */
USB_ETHER_OPTS_ATTR_IFNAME(ncm);
USB_ETHERNET_CONFIGFS_ITEM_ATTR_IFNAME(ncm);

static ssize_t ncm_opts_max_segment_size_show(struct config_item *item,
					      char *page)
@@ -1672,27 +1655,34 @@ static void ncm_free_inst(struct usb_function_instance *f)
	struct f_ncm_opts *opts;

	opts = container_of(f, struct f_ncm_opts, func_inst);
	if (opts->bound)
		gether_cleanup(netdev_priv(opts->net));
	else
		free_netdev(opts->net);
	kfree(opts->ncm_interf_group);
	kfree(opts);
}

static struct usb_function_instance *ncm_alloc_inst(void)
{
	struct usb_function_instance *ret;
	struct f_ncm_opts *opts;
	struct usb_os_desc *descs[1];
	char *names[1];
	struct config_group *ncm_interf_group;

	struct f_ncm_opts *opts __free(kfree) = kzalloc_obj(*opts);
	opts = kzalloc_obj(*opts);
	if (!opts)
		return ERR_PTR(-ENOMEM);

	opts->net = NULL;
	opts->ncm_os_desc.ext_compat_id = opts->ncm_ext_compat_id;
	gether_setup_opts_default(&opts->net_opts, "usb");

	mutex_init(&opts->lock);
	opts->func_inst.free_func_inst = ncm_free_inst;
	opts->net = gether_setup_default();
	if (IS_ERR(opts->net)) {
		struct net_device *net = opts->net;
		kfree(opts);
		return ERR_CAST(net);
	}
	opts->max_segment_size = ETH_FRAME_LEN;
	INIT_LIST_HEAD(&opts->ncm_os_desc.ext_prop);

@@ -1703,22 +1693,26 @@ static struct usb_function_instance *ncm_alloc_inst(void)
	ncm_interf_group =
		usb_os_desc_prepare_interf_dir(&opts->func_inst.group, 1, descs,
					       names, THIS_MODULE);
	if (IS_ERR(ncm_interf_group))
	if (IS_ERR(ncm_interf_group)) {
		ncm_free_inst(&opts->func_inst);
		return ERR_CAST(ncm_interf_group);
	}
	opts->ncm_interf_group = ncm_interf_group;

	ret = &opts->func_inst;
	retain_and_null_ptr(opts);
	return ret;
	return &opts->func_inst;
}

static void ncm_free(struct usb_function *f)
{
	struct f_ncm_opts *opts = func_to_ncm_opts(f);
	struct f_ncm *ncm;
	struct f_ncm_opts *opts;

	scoped_guard(mutex, &opts->lock)
	ncm = func_to_ncm(f);
	opts = container_of(f->fi, struct f_ncm_opts, func_inst);
	kfree(ncm);
	mutex_lock(&opts->lock);
	opts->refcnt--;
	kfree(func_to_ncm(f));
	mutex_unlock(&opts->lock);
}

static void ncm_unbind(struct usb_configuration *c, struct usb_function *f)
@@ -1742,15 +1736,13 @@ static void ncm_unbind(struct usb_configuration *c, struct usb_function *f)

	kfree(ncm->notify_req->buf);
	usb_ep_free_request(ncm->notify, ncm->notify_req);

	ncm->port.ioport = NULL;
	gether_cleanup(netdev_priv(ncm->netdev));
}

static struct usb_function *ncm_alloc(struct usb_function_instance *fi)
{
	struct f_ncm		*ncm;
	struct f_ncm_opts	*opts;
	int status;

	/* allocate and initialize one new instance */
	ncm = kzalloc(sizeof(*ncm), GFP_KERNEL);
@@ -1758,12 +1750,22 @@ static struct usb_function *ncm_alloc(struct usb_function_instance *fi)
		return ERR_PTR(-ENOMEM);

	opts = container_of(fi, struct f_ncm_opts, func_inst);

	scoped_guard(mutex, &opts->lock)
	mutex_lock(&opts->lock);
	opts->refcnt++;

	/* export host's Ethernet address in CDC format */
	status = gether_get_host_addr_cdc(opts->net, ncm->ethaddr,
				      sizeof(ncm->ethaddr));
	if (status < 12) { /* strlen("01234567890a") */
		kfree(ncm);
		mutex_unlock(&opts->lock);
		return ERR_PTR(-EINVAL);
	}

	spin_lock_init(&ncm->lock);
	ncm_reset_values(ncm);
	ncm->port.ioport = netdev_priv(opts->net);
	mutex_unlock(&opts->lock);
	ncm->port.is_fixed = true;
	ncm->port.supports_multi_frame = true;

+1 −3
Original line number Diff line number Diff line
@@ -15,13 +15,11 @@

#include <linux/usb/composite.h>

#include "u_ether.h"

struct f_ncm_opts {
	struct usb_function_instance	func_inst;
	struct net_device		*net;
	bool				bound;

	struct gether_opts		net_opts;
	struct config_group		*ncm_interf_group;
	struct usb_os_desc		ncm_os_desc;
	char				ncm_ext_compat_id[16];