diff mbox series

[v3,07/11] usb: gadget: configfs: Attach arbitrary strings to cdev

Message ID 20230130093443.25644-8-dan.scally@ideasonboard.com (mailing list archive)
State Superseded
Headers show
Series Add XU support to UVC Gadget | expand

Commit Message

Dan Scally Jan. 30, 2023, 9:34 a.m. UTC 
Attach any arbitrary strings that are defined to the composite dev.
We handle the old-style manufacturer, product and serialnumbers
strings in the same function for simplicity.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v3:

	- Was 7/9 in version 2, moved the same functionality from the UVC
	function to usb gadget core.

Changes in v2:

	- New patch

 drivers/usb/gadget/configfs.c | 95 ++++++++++++++++++++++++++++-------
 include/linux/usb/composite.h |  1 +
 2 files changed, 79 insertions(+), 17 deletions(-)

Comments

Dan Carpenter Jan. 31, 2023, 7:35 a.m. UTC | #1 
Hi Daniel,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
patch link:    https://lore.kernel.org/r/20230130093443.25644-8-dan.scally%40ideasonboard.com
patch subject: [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230131/202301311446.AqdvqXkI-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>

New smatch warnings:
drivers/usb/gadget/configfs.c:1563 configfs_composite_bind() warn: passing zero to 'PTR_ERR'

Old smatch warnings:
drivers/usb/gadget/configfs.c:985 os_desc_b_vendor_code_show() warn: argument 3 to %02x specifier has type 'char'

vim +/PTR_ERR +1563 drivers/usb/gadget/configfs.c

88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1518  static int configfs_composite_bind(struct usb_gadget *gadget,
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1519  		struct usb_gadget_driver *gdriver)
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1520  {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1521  	struct usb_composite_driver     *composite = to_cdriver(gdriver);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1522  	struct gadget_info		*gi = container_of(composite,
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1523  						struct gadget_info, composite);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1524  	struct usb_composite_dev	*cdev = &gi->cdev;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1525  	struct usb_configuration	*c;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1526  	struct usb_string		*s;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1527  	unsigned			i;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1528  	int				ret;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1529  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1530  	/* the gi->lock is hold by the caller */
1a1c851bbd706e Peter Chen                2019-08-26  1531  	gi->unbind = 0;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1532  	cdev->gadget = gadget;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1533  	set_gadget_data(gadget, cdev);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1534  	ret = composite_dev_prepare(composite, cdev);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1535  	if (ret)
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1536  		return ret;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1537  	/* and now the gadget bind */
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1538  	ret = -EINVAL;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1539  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1540  	if (list_empty(&gi->cdev.configs)) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1541  		pr_err("Need at least one configuration in %s.\n",
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1542  				gi->composite.name);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1543  		goto err_comp_cleanup;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1544  	}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1545  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1546  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1547  	list_for_each_entry(c, &gi->cdev.configs, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1548  		struct config_usb_cfg *cfg;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1549  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1550  		cfg = container_of(c, struct config_usb_cfg, c);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1551  		if (list_empty(&cfg->func_list)) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1552  			pr_err("Config %s/%d of %s needs at least one function.\n",
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1553  			      c->label, c->bConfigurationValue,
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1554  			      gi->composite.name);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1555  			goto err_comp_cleanup;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1556  		}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1557  	}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1558  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1559  	/* init all strings */
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1560  	if (!list_empty(&gi->string_list)) {
0c252735443756 Daniel Scally             2023-01-30  1561  		s = configfs_attach_gadget_strings(gi);
0c252735443756 Daniel Scally             2023-01-30  1562  		if (IS_ERR_OR_NULL(s)) {


Passing NULL to PTR_ERR(s) is not a bug, but this check has a probably
under 10% false positive rate because 90% of the time when people do
that it is wrong.

In this case configfs_attach_gadget_strings() cannot actually return
NULL so this could just be changed to if (IS_ERR(s)) {.

I would say that probably if it *did* return NULL then we should return
-EPROBE_DEFER.  It's an interesting philosophical debate how to handle
impossible things...

fea77077d1623c Wei Yongjun               2013-05-07 @1563  			ret = PTR_ERR(s);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1564  			goto err_comp_cleanup;
fea77077d1623c Wei Yongjun               2013-05-07  1565  		}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1566  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1567  		gi->cdev.desc.iManufacturer = s[USB_GADGET_MANUFACTURER_IDX].id;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1568  		gi->cdev.desc.iProduct = s[USB_GADGET_PRODUCT_IDX].id;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1569  		gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
0c252735443756 Daniel Scally             2023-01-30  1570  
0c252735443756 Daniel Scally             2023-01-30  1571  		gi->cdev.usb_strings = s;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1572  	}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1573  
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1574  	if (gi->use_os_desc) {
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1575  		cdev->use_os_string = true;
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1576  		cdev->b_vendor_code = gi->b_vendor_code;
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1577  		memcpy(cdev->qw_sign, gi->qw_sign, OS_STRING_QW_SIGN_LEN);
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1578  	}
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1579  
41ce84c86d0a04 Li Jun                    2015-07-09  1580  	if (gadget_is_otg(gadget) && !otg_desc[0]) {
41ce84c86d0a04 Li Jun                    2015-07-09  1581  		struct usb_descriptor_header *usb_desc;
41ce84c86d0a04 Li Jun                    2015-07-09  1582  
41ce84c86d0a04 Li Jun                    2015-07-09  1583  		usb_desc = usb_otg_descriptor_alloc(gadget);
41ce84c86d0a04 Li Jun                    2015-07-09  1584  		if (!usb_desc) {
41ce84c86d0a04 Li Jun                    2015-07-09  1585  			ret = -ENOMEM;
41ce84c86d0a04 Li Jun                    2015-07-09  1586  			goto err_comp_cleanup;
41ce84c86d0a04 Li Jun                    2015-07-09  1587  		}
41ce84c86d0a04 Li Jun                    2015-07-09  1588  		usb_otg_descriptor_init(gadget, usb_desc);
41ce84c86d0a04 Li Jun                    2015-07-09  1589  		otg_desc[0] = usb_desc;
41ce84c86d0a04 Li Jun                    2015-07-09  1590  		otg_desc[1] = NULL;
41ce84c86d0a04 Li Jun                    2015-07-09  1591  	}
41ce84c86d0a04 Li Jun                    2015-07-09  1592  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1593  	/* Go through all configs, attach all functions */
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1594  	list_for_each_entry(c, &gi->cdev.configs, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1595  		struct config_usb_cfg *cfg;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1596  		struct usb_function *f;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1597  		struct usb_function *tmp;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1598  		struct gadget_config_name *cn;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1599  
41ce84c86d0a04 Li Jun                    2015-07-09  1600  		if (gadget_is_otg(gadget))
41ce84c86d0a04 Li Jun                    2015-07-09  1601  			c->descriptors = otg_desc;
41ce84c86d0a04 Li Jun                    2015-07-09  1602  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1603  		cfg = container_of(c, struct config_usb_cfg, c);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1604  		if (!list_empty(&cfg->string_list)) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1605  			i = 0;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1606  			list_for_each_entry(cn, &cfg->string_list, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1607  				cfg->gstrings[i] = &cn->stringtab_dev;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1608  				cn->stringtab_dev.strings = &cn->strings;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1609  				cn->strings.s = cn->configuration;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1610  				i++;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1611  			}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1612  			cfg->gstrings[i] = NULL;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1613  			s = usb_gstrings_attach(&gi->cdev, cfg->gstrings, 1);
fea77077d1623c Wei Yongjun               2013-05-07  1614  			if (IS_ERR(s)) {
fea77077d1623c Wei Yongjun               2013-05-07  1615  				ret = PTR_ERR(s);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1616  				goto err_comp_cleanup;
fea77077d1623c Wei Yongjun               2013-05-07  1617  			}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1618  			c->iConfiguration = s[0].id;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1619  		}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1620  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1621  		list_for_each_entry_safe(f, tmp, &cfg->func_list, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1622  			list_del(&f->list);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1623  			ret = usb_add_function(c, f);
5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1624  			if (ret) {
5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1625  				list_add(&f->list, &cfg->func_list);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1626  				goto err_purge_funcs;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1627  			}
5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1628  		}
7adf9e3adc398e Wesley Cheng              2021-07-10  1629  		ret = usb_gadget_check_config(cdev->gadget);
7adf9e3adc398e Wesley Cheng              2021-07-10  1630  		if (ret)
7adf9e3adc398e Wesley Cheng              2021-07-10  1631  			goto err_purge_funcs;
7adf9e3adc398e Wesley Cheng              2021-07-10  1632  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1633  		usb_ep_autoconfig_reset(cdev->gadget);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1634  	}
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1635  	if (cdev->use_os_string) {
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1636  		ret = composite_os_desc_req_prepare(cdev, gadget->ep0);
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1637  		if (ret)
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1638  			goto err_purge_funcs;
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1639  	}
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1640  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1641  	usb_ep_autoconfig_reset(cdev->gadget);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1642  	return 0;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1643  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1644  err_purge_funcs:
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1645  	purge_configs_funcs(gi);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1646  err_comp_cleanup:
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1647  	composite_dev_cleanup(cdev);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1648  	return ret;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1649  }
Dan Scally Feb. 1, 2023, 9:50 a.m. UTC | #2 
Morning Dan

On 31/01/2023 07:35, Dan Carpenter wrote:
> Hi Daniel,
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
> patch link:    https://lore.kernel.org/r/20230130093443.25644-8-dan.scally%40ideasonboard.com
> patch subject: [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev
> config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230131/202301311446.AqdvqXkI-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
>
> New smatch warnings:
> drivers/usb/gadget/configfs.c:1563 configfs_composite_bind() warn: passing zero to 'PTR_ERR'
>
> Old smatch warnings:
> drivers/usb/gadget/configfs.c:985 os_desc_b_vendor_code_show() warn: argument 3 to %02x specifier has type 'char'


Really must make time to look at Smatch...

>
> vim +/PTR_ERR +1563 drivers/usb/gadget/configfs.c
>
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1518  static int configfs_composite_bind(struct usb_gadget *gadget,
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1519  		struct usb_gadget_driver *gdriver)
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1520  {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1521  	struct usb_composite_driver     *composite = to_cdriver(gdriver);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1522  	struct gadget_info		*gi = container_of(composite,
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1523  						struct gadget_info, composite);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1524  	struct usb_composite_dev	*cdev = &gi->cdev;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1525  	struct usb_configuration	*c;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1526  	struct usb_string		*s;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1527  	unsigned			i;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1528  	int				ret;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1529
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1530  	/* the gi->lock is hold by the caller */
> 1a1c851bbd706e Peter Chen                2019-08-26  1531  	gi->unbind = 0;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1532  	cdev->gadget = gadget;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1533  	set_gadget_data(gadget, cdev);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1534  	ret = composite_dev_prepare(composite, cdev);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1535  	if (ret)
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1536  		return ret;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1537  	/* and now the gadget bind */
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1538  	ret = -EINVAL;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1539
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1540  	if (list_empty(&gi->cdev.configs)) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1541  		pr_err("Need at least one configuration in %s.\n",
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1542  				gi->composite.name);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1543  		goto err_comp_cleanup;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1544  	}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1545
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1546
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1547  	list_for_each_entry(c, &gi->cdev.configs, list) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1548  		struct config_usb_cfg *cfg;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1549
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1550  		cfg = container_of(c, struct config_usb_cfg, c);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1551  		if (list_empty(&cfg->func_list)) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1552  			pr_err("Config %s/%d of %s needs at least one function.\n",
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1553  			      c->label, c->bConfigurationValue,
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1554  			      gi->composite.name);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1555  			goto err_comp_cleanup;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1556  		}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1557  	}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1558
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1559  	/* init all strings */
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1560  	if (!list_empty(&gi->string_list)) {
> 0c252735443756 Daniel Scally             2023-01-30  1561  		s = configfs_attach_gadget_strings(gi);
> 0c252735443756 Daniel Scally             2023-01-30  1562  		if (IS_ERR_OR_NULL(s)) {
>
>
> Passing NULL to PTR_ERR(s) is not a bug, but this check has a probably
> under 10% false positive rate because 90% of the time when people do
> that it is wrong.
>
> In this case configfs_attach_gadget_strings() cannot actually return
> NULL so this could just be changed to if (IS_ERR(s)) {.


Yes good spot, thank you

>
> I would say that probably if it *did* return NULL then we should return
> -EPROBE_DEFER.  It's an interesting philosophical debate how to handle
> impossible things...


Well in this case it returns null when there's no languages defined, 
which isn't possible here since it's behind the if(!list_empty(...)) 
check but in principle I think is allowed - usb devices don't have to 
support any string descriptors as far as I know.

> fea77077d1623c Wei Yongjun               2013-05-07 @1563  			ret = PTR_ERR(s);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1564  			goto err_comp_cleanup;
> fea77077d1623c Wei Yongjun               2013-05-07  1565  		}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1566
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1567  		gi->cdev.desc.iManufacturer = s[USB_GADGET_MANUFACTURER_IDX].id;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1568  		gi->cdev.desc.iProduct = s[USB_GADGET_PRODUCT_IDX].id;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1569  		gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
> 0c252735443756 Daniel Scally             2023-01-30  1570
> 0c252735443756 Daniel Scally             2023-01-30  1571  		gi->cdev.usb_strings = s;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1572  	}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1573
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1574  	if (gi->use_os_desc) {
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1575  		cdev->use_os_string = true;
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1576  		cdev->b_vendor_code = gi->b_vendor_code;
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1577  		memcpy(cdev->qw_sign, gi->qw_sign, OS_STRING_QW_SIGN_LEN);
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1578  	}
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1579
> 41ce84c86d0a04 Li Jun                    2015-07-09  1580  	if (gadget_is_otg(gadget) && !otg_desc[0]) {
> 41ce84c86d0a04 Li Jun                    2015-07-09  1581  		struct usb_descriptor_header *usb_desc;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1582
> 41ce84c86d0a04 Li Jun                    2015-07-09  1583  		usb_desc = usb_otg_descriptor_alloc(gadget);
> 41ce84c86d0a04 Li Jun                    2015-07-09  1584  		if (!usb_desc) {
> 41ce84c86d0a04 Li Jun                    2015-07-09  1585  			ret = -ENOMEM;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1586  			goto err_comp_cleanup;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1587  		}
> 41ce84c86d0a04 Li Jun                    2015-07-09  1588  		usb_otg_descriptor_init(gadget, usb_desc);
> 41ce84c86d0a04 Li Jun                    2015-07-09  1589  		otg_desc[0] = usb_desc;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1590  		otg_desc[1] = NULL;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1591  	}
> 41ce84c86d0a04 Li Jun                    2015-07-09  1592
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1593  	/* Go through all configs, attach all functions */
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1594  	list_for_each_entry(c, &gi->cdev.configs, list) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1595  		struct config_usb_cfg *cfg;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1596  		struct usb_function *f;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1597  		struct usb_function *tmp;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1598  		struct gadget_config_name *cn;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1599
> 41ce84c86d0a04 Li Jun                    2015-07-09  1600  		if (gadget_is_otg(gadget))
> 41ce84c86d0a04 Li Jun                    2015-07-09  1601  			c->descriptors = otg_desc;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1602
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1603  		cfg = container_of(c, struct config_usb_cfg, c);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1604  		if (!list_empty(&cfg->string_list)) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1605  			i = 0;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1606  			list_for_each_entry(cn, &cfg->string_list, list) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1607  				cfg->gstrings[i] = &cn->stringtab_dev;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1608  				cn->stringtab_dev.strings = &cn->strings;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1609  				cn->strings.s = cn->configuration;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1610  				i++;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1611  			}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1612  			cfg->gstrings[i] = NULL;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1613  			s = usb_gstrings_attach(&gi->cdev, cfg->gstrings, 1);
> fea77077d1623c Wei Yongjun               2013-05-07  1614  			if (IS_ERR(s)) {
> fea77077d1623c Wei Yongjun               2013-05-07  1615  				ret = PTR_ERR(s);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1616  				goto err_comp_cleanup;
> fea77077d1623c Wei Yongjun               2013-05-07  1617  			}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1618  			c->iConfiguration = s[0].id;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1619  		}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1620
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1621  		list_for_each_entry_safe(f, tmp, &cfg->func_list, list) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1622  			list_del(&f->list);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1623  			ret = usb_add_function(c, f);
> 5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1624  			if (ret) {
> 5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1625  				list_add(&f->list, &cfg->func_list);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1626  				goto err_purge_funcs;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1627  			}
> 5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1628  		}
> 7adf9e3adc398e Wesley Cheng              2021-07-10  1629  		ret = usb_gadget_check_config(cdev->gadget);
> 7adf9e3adc398e Wesley Cheng              2021-07-10  1630  		if (ret)
> 7adf9e3adc398e Wesley Cheng              2021-07-10  1631  			goto err_purge_funcs;
> 7adf9e3adc398e Wesley Cheng              2021-07-10  1632
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1633  		usb_ep_autoconfig_reset(cdev->gadget);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1634  	}
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1635  	if (cdev->use_os_string) {
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1636  		ret = composite_os_desc_req_prepare(cdev, gadget->ep0);
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1637  		if (ret)
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1638  			goto err_purge_funcs;
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1639  	}
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1640
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1641  	usb_ep_autoconfig_reset(cdev->gadget);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1642  	return 0;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1643
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1644  err_purge_funcs:
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1645  	purge_configs_funcs(gi);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1646  err_comp_cleanup:
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1647  	composite_dev_cleanup(cdev);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1648  	return ret;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1649  }
>
diff mbox series

Patch

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 7c8b8ab5dfa3..c2f23a63ab10 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1441,6 +1441,80 @@  static void purge_configs_funcs(struct gadget_info *gi)
 	}
 }
 
+static struct usb_string *
+configfs_attach_gadget_strings(struct gadget_info *gi)
+{
+	struct usb_gadget_strings **gadget_strings;
+	struct gadget_language *language;
+	struct gadget_string *string;
+	unsigned int nlangs = 0;
+	struct list_head *iter;
+	struct usb_string *us;
+	unsigned int i = 0;
+	int nstrings = -1;
+	unsigned int j;
+
+	list_for_each(iter, &gi->string_list)
+		nlangs++;
+
+	/* Bail out early if no languages are configured */
+	if (!nlangs)
+		return NULL;
+
+	gadget_strings = kcalloc(nlangs + 1, /* including NULL terminator */
+				 sizeof(struct usb_gadget_strings *), GFP_KERNEL);
+	if (!gadget_strings)
+		return ERR_PTR(-ENOMEM);
+
+	list_for_each_entry(language, &gi->string_list, list) {
+		struct usb_string *stringtab;
+
+		if (nstrings == -1) {
+			nstrings = language->nstrings;
+		} else if (nstrings != language->nstrings) {
+			pr_err("languages must contain the same number of strings\n");
+			us = ERR_PTR(-EINVAL);
+			goto cleanup;
+		}
+
+		stringtab = kcalloc(language->nstrings + 1, sizeof(struct usb_string),
+				    GFP_KERNEL);
+		if (!stringtab) {
+			us = ERR_PTR(-ENOMEM);
+			goto cleanup;
+		}
+
+		stringtab[USB_GADGET_MANUFACTURER_IDX].id = USB_GADGET_MANUFACTURER_IDX;
+		stringtab[USB_GADGET_MANUFACTURER_IDX].s = language->manufacturer;
+		stringtab[USB_GADGET_PRODUCT_IDX].id = USB_GADGET_PRODUCT_IDX;
+		stringtab[USB_GADGET_PRODUCT_IDX].s = language->product;
+		stringtab[USB_GADGET_SERIAL_IDX].id = USB_GADGET_SERIAL_IDX;
+		stringtab[USB_GADGET_SERIAL_IDX].s = language->serialnumber;
+
+		j = USB_GADGET_FIRST_AVAIL_IDX;
+		list_for_each_entry(string, &language->gadget_strings, list) {
+			memcpy(&stringtab[j], &string->usb_string, sizeof(struct usb_string));
+			j++;
+		}
+
+		language->stringtab_dev.strings = stringtab;
+		gadget_strings[i] = &language->stringtab_dev;
+		i++;
+	}
+
+	us = usb_gstrings_attach(&gi->cdev, gadget_strings, nstrings);
+
+cleanup:
+	list_for_each_entry(language, &gi->string_list, list) {
+		kfree(language->stringtab_dev.strings);
+		language->stringtab_dev.strings = NULL;
+	}
+
+	kfree(gadget_strings);
+
+	return us;
+}
+
 static int configfs_composite_bind(struct usb_gadget *gadget,
 		struct usb_gadget_driver *gdriver)
 {
@@ -1484,23 +1558,8 @@  static int configfs_composite_bind(struct usb_gadget *gadget,
 
 	/* init all strings */
 	if (!list_empty(&gi->string_list)) {
-		struct gadget_language *gs;
-
-		i = 0;
-		list_for_each_entry(gs, &gi->string_list, list) {
-
-			gi->gstrings[i] = &gs->stringtab_dev;
-			gs->stringtab_dev.strings = gs->strings;
-			gs->strings[USB_GADGET_MANUFACTURER_IDX].s =
-				gs->manufacturer;
-			gs->strings[USB_GADGET_PRODUCT_IDX].s = gs->product;
-			gs->strings[USB_GADGET_SERIAL_IDX].s = gs->serialnumber;
-			i++;
-		}
-		gi->gstrings[i] = NULL;
-		s = usb_gstrings_attach(&gi->cdev, gi->gstrings,
-				USB_GADGET_FIRST_AVAIL_IDX);
-		if (IS_ERR(s)) {
+		s = configfs_attach_gadget_strings(gi);
+		if (IS_ERR_OR_NULL(s)) {
 			ret = PTR_ERR(s);
 			goto err_comp_cleanup;
 		}
@@ -1508,6 +1567,8 @@  static int configfs_composite_bind(struct usb_gadget *gadget,
 		gi->cdev.desc.iManufacturer = s[USB_GADGET_MANUFACTURER_IDX].id;
 		gi->cdev.desc.iProduct = s[USB_GADGET_PRODUCT_IDX].id;
 		gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
+
+		gi->cdev.usb_strings = s;
 	}
 
 	if (gi->use_os_desc) {
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 43ac3fa760db..85c7f6036933 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -483,6 +483,7 @@  struct usb_composite_dev {
 	struct usb_composite_driver	*driver;
 	u8				next_string_id;
 	char				*def_manufacturer;
+	struct usb_string		*usb_strings;
 
 	/* the gadget driver won't enable the data pullup
 	 * while the deactivation count is nonzero.