diff mbox series

[v3] ar5523: check endpoints type and direction in probe()

Message ID 20220827110148.203104-1-mazinalhaddad05@gmail.com
State New
Headers show
Series [v3] ar5523: check endpoints type and direction in probe() | expand

Commit Message

Mazin Al Haddad Aug. 27, 2022, 11:01 a.m. UTC 
Fixes a bug reported by syzbot, where a warning occurs in usb_submit_urb()
due to the wrong endpoint type. There is no check for both the number
of endpoints and the type.

Fix it by adding a check for the number of endpoints and the
direction/type of the endpoints. If the endpoints do not match -ENODEV is
returned.

usb 1-1: BOGUS urb xfer, pipe 3 != type 1
WARNING: CPU: 1 PID: 71 at drivers/usb/core/urb.c:502 usb_submit_urb+0xed2/0x18a0 drivers/usb/core/urb.c:502
Modules linked in:
CPU: 1 PID: 71 Comm: kworker/1:2 Not tainted 5.19.0-rc7-syzkaller-00150-g32f02a211b0a #0
Hardware name: Google Compute Engine/Google Compute Engine, BIOS Google 06/29/2022
Workqueue: usb_hub_wq hub_event
Call Trace:
 <TASK>
 ar5523_cmd+0x420/0x790 drivers/net/wireless/ath/ar5523/ar5523.c:275
 ar5523_cmd_read drivers/net/wireless/ath/ar5523/ar5523.c:302 [inline]
 ar5523_host_available drivers/net/wireless/ath/ar5523/ar5523.c:1376 [inline]
 ar5523_probe+0xc66/0x1da0 drivers/net/wireless/ath/ar5523/ar5523.c:1655

Link: https://syzkaller.appspot.com/bug?extid=1bc2c2afd44f820a669f
Reported-and-tested-by: syzbot+1bc2c2afd44f820a669f@syzkaller.appspotmail.com
Signed-off-by: Mazin Al Haddad <mazinalhaddad05@gmail.com>
---
v2->v3 changes:
 - Make use of helper functions instead of checking for direction
	 and type manually. 

 drivers/net/wireless/ath/ar5523/ar5523.c | 38 ++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Comments

kernel test robot Sept. 3, 2022, 2:22 p.m. UTC | #1 
Hi Mazin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wireless-next/main]
[also build test ERROR on wireless/main linus/master v6.0-rc3 next-20220901]
[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/Mazin-Al-Haddad/ar5523-check-endpoints-type-and-direction-in-probe/20220827-190333
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
config: arm-allyesconfig
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/0c1dd42c9cfa5b58485b12b4f27e1539fcb8b9dd
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Mazin-Al-Haddad/ar5523-check-endpoints-type-and-direction-in-probe/20220827-190333
        git checkout 0c1dd42c9cfa5b58485b12b4f27e1539fcb8b9dd
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/

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

All errors (new ones prefixed by >>):

   drivers/net/wireless/ath/ar5523/ar5523.c: In function 'ar5523_probe':
>> drivers/net/wireless/ath/ar5523/ar5523.c:1620:55: error: macro "dev_warn" requires 3 arguments, but only 1 given
    1620 |                 dev_warn("wrong number of endpoints\n");
         |                                                       ^
   In file included from include/linux/device.h:15,
                    from include/linux/dma-mapping.h:7,
                    from include/linux/skbuff.h:31,
                    from drivers/net/wireless/ath/ar5523/ar5523.c:33:
   include/linux/dev_printk.h:145: note: macro "dev_warn" defined here
     145 | #define dev_warn(dev, fmt, ...) \
         | 
>> drivers/net/wireless/ath/ar5523/ar5523.c:1620:17: error: 'dev_warn' undeclared (first use in this function); did you mean '_dev_warn'?
    1620 |                 dev_warn("wrong number of endpoints\n");
         |                 ^~~~~~~~
         |                 _dev_warn
   drivers/net/wireless/ath/ar5523/ar5523.c:1620:17: note: each undeclared identifier is reported only once for each function it appears in


vim +/dev_warn +1620 drivers/net/wireless/ath/ar5523/ar5523.c

  1577	
  1578	static int ar5523_probe(struct usb_interface *intf,
  1579				const struct usb_device_id *id)
  1580	{
  1581		struct usb_device *dev = interface_to_usbdev(intf);
  1582		struct ieee80211_hw *hw;
  1583		struct ar5523 *ar;
  1584		struct usb_host_interface *host = intf->cur_altsetting;
  1585		struct usb_endpoint_descriptor *cmd_tx, *cmd_rx, *data_tx, *data_rx;
  1586		int error = -ENOMEM;
  1587	
  1588		if (host->desc.bNumEndpoints != 4) {
  1589			dev_err(&dev->dev, "Wrong number of endpoints\n");
  1590			return -ENODEV;
  1591		}
  1592	
  1593		for (int i = 0; i < host->desc.bNumEndpoints; ++i) {
  1594			struct usb_endpoint_descriptor *ep = &host->endpoint[i].desc;
  1595	
  1596			if (usb_endpoint_is_bulk_out(ep)) {
  1597				if (!cmd_tx) {
  1598					if (ep->bEndpointAddress == AR5523_CMD_TX_PIPE)
  1599						cmd_tx = ep;
  1600				}
  1601				if (!data_tx) {
  1602					if (ep->bEndpointAddress == AR5523_DATA_TX_PIPE)
  1603						data_tx = ep;
  1604					}
  1605			}
  1606	
  1607			if (usb_endpoint_is_bulk_in(ep)) {
  1608				if (!cmd_rx) {
  1609					if (ep->bEndpointAddress == AR5523_CMD_RX_PIPE)
  1610						cmd_rx = ep;
  1611				}
  1612				if (!data_rx) {
  1613					if (ep->bEndpointAddress == AR5523_DATA_RX_PIPE)
  1614						data_rx = ep;
  1615				}
  1616			}
  1617		}
  1618	
  1619		if (!cmd_tx || !data_tx || !cmd_rx || !data_rx) {
> 1620			dev_warn("wrong number of endpoints\n");
  1621			return -ENODEV;
  1622		}
  1623	
  1624		/*
  1625		 * Load firmware if the device requires it.  This will return
  1626		 * -ENXIO on success and we'll get called back afer the usb
  1627		 * id changes to indicate that the firmware is present.
  1628		 */
  1629		if (id->driver_info & AR5523_FLAG_PRE_FIRMWARE)
  1630			return ar5523_load_firmware(dev);
  1631	
  1632	
  1633		hw = ieee80211_alloc_hw(sizeof(*ar), &ar5523_ops);
  1634		if (!hw)
  1635			goto out;
  1636		SET_IEEE80211_DEV(hw, &intf->dev);
  1637	
  1638		ar = hw->priv;
  1639		ar->hw = hw;
  1640		ar->dev = dev;
  1641		mutex_init(&ar->mutex);
  1642	
  1643		INIT_DELAYED_WORK(&ar->stat_work, ar5523_stat_work);
  1644		timer_setup(&ar->tx_wd_timer, ar5523_tx_wd_timer, 0);
  1645		INIT_WORK(&ar->tx_wd_work, ar5523_tx_wd_work);
  1646		INIT_WORK(&ar->tx_work, ar5523_tx_work);
  1647		INIT_LIST_HEAD(&ar->tx_queue_pending);
  1648		INIT_LIST_HEAD(&ar->tx_queue_submitted);
  1649		spin_lock_init(&ar->tx_data_list_lock);
  1650		atomic_set(&ar->tx_nr_total, 0);
  1651		atomic_set(&ar->tx_nr_pending, 0);
  1652		init_waitqueue_head(&ar->tx_flush_waitq);
  1653	
  1654		atomic_set(&ar->rx_data_free_cnt, 0);
  1655		INIT_WORK(&ar->rx_refill_work, ar5523_rx_refill_work);
  1656		INIT_LIST_HEAD(&ar->rx_data_free);
  1657		INIT_LIST_HEAD(&ar->rx_data_used);
  1658		spin_lock_init(&ar->rx_data_list_lock);
  1659	
  1660		ar->wq = create_singlethread_workqueue("ar5523");
  1661		if (!ar->wq) {
  1662			ar5523_err(ar, "Could not create wq\n");
  1663			goto out_free_ar;
  1664		}
  1665	
  1666		error = ar5523_alloc_rx_bufs(ar);
  1667		if (error) {
  1668			ar5523_err(ar, "Could not allocate rx buffers\n");
  1669			goto out_free_wq;
  1670		}
  1671	
  1672		error = ar5523_alloc_rx_cmd(ar);
  1673		if (error) {
  1674			ar5523_err(ar, "Could not allocate rx command buffers\n");
  1675			goto out_free_rx_bufs;
  1676		}
  1677	
  1678		error = ar5523_alloc_tx_cmd(ar);
  1679		if (error) {
  1680			ar5523_err(ar, "Could not allocate tx command buffers\n");
  1681			goto out_free_rx_cmd;
  1682		}
  1683	
  1684		error = ar5523_submit_rx_cmd(ar);
  1685		if (error) {
  1686			ar5523_err(ar, "Failed to submit rx cmd\n");
  1687			goto out_free_tx_cmd;
  1688		}
  1689	
  1690		/*
  1691		 * We're now ready to send/receive firmware commands.
  1692		 */
  1693		error = ar5523_host_available(ar);
  1694		if (error) {
  1695			ar5523_err(ar, "could not initialize adapter\n");
  1696			goto out_cancel_rx_cmd;
  1697		}
  1698	
  1699		error = ar5523_get_max_rxsz(ar);
  1700		if (error) {
  1701			ar5523_err(ar, "could not get caps from adapter\n");
  1702			goto out_cancel_rx_cmd;
  1703		}
  1704	
  1705		error = ar5523_get_devcap(ar);
  1706		if (error) {
  1707			ar5523_err(ar, "could not get caps from adapter\n");
  1708			goto out_cancel_rx_cmd;
  1709		}
  1710	
  1711		error = ar5523_get_devstatus(ar);
  1712		if (error != 0) {
  1713			ar5523_err(ar, "could not get device status\n");
  1714			goto out_cancel_rx_cmd;
  1715		}
  1716	
  1717		ar5523_info(ar, "MAC/BBP AR5523, RF AR%c112\n",
  1718				(id->driver_info & AR5523_FLAG_ABG) ? '5' : '2');
  1719	
  1720		ar->vif = NULL;
  1721		ieee80211_hw_set(hw, HAS_RATE_CONTROL);
  1722		ieee80211_hw_set(hw, RX_INCLUDES_FCS);
  1723		ieee80211_hw_set(hw, SIGNAL_DBM);
  1724		hw->extra_tx_headroom = sizeof(struct ar5523_tx_desc) +
  1725					sizeof(struct ar5523_chunk);
  1726		hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
  1727		hw->queues = 1;
  1728	
  1729		error = ar5523_init_modes(ar);
  1730		if (error)
  1731			goto out_cancel_rx_cmd;
  1732	
  1733		wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
  1734	
  1735		usb_set_intfdata(intf, hw);
  1736	
  1737		error = ieee80211_register_hw(hw);
  1738		if (error) {
  1739			ar5523_err(ar, "could not register device\n");
  1740			goto out_cancel_rx_cmd;
  1741		}
  1742	
  1743		ar5523_info(ar, "Found and initialized AR5523 device\n");
  1744		return 0;
  1745	
  1746	out_cancel_rx_cmd:
  1747		ar5523_cancel_rx_cmd(ar);
  1748	out_free_tx_cmd:
  1749		ar5523_free_tx_cmd(ar);
  1750	out_free_rx_cmd:
  1751		ar5523_free_rx_cmd(ar);
  1752	out_free_rx_bufs:
  1753		ar5523_free_rx_bufs(ar);
  1754	out_free_wq:
  1755		destroy_workqueue(ar->wq);
  1756	out_free_ar:
  1757		ieee80211_free_hw(hw);
  1758	out:
  1759		return error;
  1760	}
  1761
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ar5523/ar5523.c b/drivers/net/wireless/ath/ar5523/ar5523.c
index 6f937d2cc126..69979e8f99fd 100644
--- a/drivers/net/wireless/ath/ar5523/ar5523.c
+++ b/drivers/net/wireless/ath/ar5523/ar5523.c
@@ -1581,8 +1581,46 @@  static int ar5523_probe(struct usb_interface *intf,
 	struct usb_device *dev = interface_to_usbdev(intf);
 	struct ieee80211_hw *hw;
 	struct ar5523 *ar;
+	struct usb_host_interface *host = intf->cur_altsetting;
+	struct usb_endpoint_descriptor *cmd_tx, *cmd_rx, *data_tx, *data_rx;
 	int error = -ENOMEM;
 
+	if (host->desc.bNumEndpoints != 4) {
+		dev_err(&dev->dev, "Wrong number of endpoints\n");
+		return -ENODEV;
+	}
+
+	for (int i = 0; i < host->desc.bNumEndpoints; ++i) {
+		struct usb_endpoint_descriptor *ep = &host->endpoint[i].desc;
+
+		if (usb_endpoint_is_bulk_out(ep)) {
+			if (!cmd_tx) {
+				if (ep->bEndpointAddress == AR5523_CMD_TX_PIPE)
+					cmd_tx = ep;
+			}
+			if (!data_tx) {
+				if (ep->bEndpointAddress == AR5523_DATA_TX_PIPE)
+					data_tx = ep;
+				}
+		}
+
+		if (usb_endpoint_is_bulk_in(ep)) {
+			if (!cmd_rx) {
+				if (ep->bEndpointAddress == AR5523_CMD_RX_PIPE)
+					cmd_rx = ep;
+			}
+			if (!data_rx) {
+				if (ep->bEndpointAddress == AR5523_DATA_RX_PIPE)
+					data_rx = ep;
+			}
+		}
+	}
+
+	if (!cmd_tx || !data_tx || !cmd_rx || !data_rx) {
+		dev_warn("wrong number of endpoints\n");
+		return -ENODEV;
+	}
+
 	/*
 	 * Load firmware if the device requires it.  This will return
 	 * -ENXIO on success and we'll get called back afer the usb