Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor
@ 2019-11-22  5:29 huangwenabc
  2019-11-24  7:52 ` kbuild test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: huangwenabc @ 2019-11-22  5:29 UTC (permalink / raw)
  To: linux-wireless; +Cc: linux-distros, security, libertas-dev

From: Wen Huang <huangwenabc@gmail.com>

add_ie_rates() copys rates without checking the length 
in bss descriptor from remote AP.when victim connects to 
remote attacker, this may trigger buffer overflow.
lbs_ibss_join_existing() copys rates without checking the length 
in bss descriptor from remote IBSS node.when victim connects to 
remote attacker, this may trigger buffer overflow.
Fix them by putting the length check before performing copy.

This fix addresses CVE-2019-14896 and CVE-2019-14897.

Signed-off-by: Wen Huang <huangwenabc@gmail.com>
---
 drivers/net/wireless/marvell/libertas/cfg.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c
index 57edfada0..290280764 100644
--- a/drivers/net/wireless/marvell/libertas/cfg.c
+++ b/drivers/net/wireless/marvell/libertas/cfg.c
@@ -273,6 +273,10 @@ add_ie_rates(u8 *tlv, const u8 *ie, int *nrates)
 	int hw, ap, ap_max = ie[1];
 	u8 hw_rate;
 
+	if (ap_max > MAX_RATES) {
+		lbs_deb_assoc("invalid rates\n");
+		return tlv;
+	}
 	/* Advance past IE header */
 	ie += 2;
 
@@ -1777,6 +1781,10 @@ static int lbs_ibss_join_existing(struct lbs_private *priv,
 	} else {
 		int hw, i;
 		u8 rates_max = rates_eid[1];
+		if (rates_max > MAX_RATES) {
+			lbs_deb_join("invalid rates");
+			goto out;
+		}
 		u8 *rates = cmd.bss.rates;
 		for (hw = 0; hw < ARRAY_SIZE(lbs_rates); hw++) {
 			u8 hw_rate = lbs_rates[hw].bitrate / 5;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor
  2019-11-22  5:29 [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor huangwenabc
@ 2019-11-24  7:52 ` kbuild test robot
  2019-11-25 12:36   ` Kalle Valo
       [not found]   ` <0101016ea290854e-f5721fd1-1ca7-49ab-9c10-85277bc46c64-000000@us-west-2.amazonses.com>
  2019-11-28  8:00 ` Kalle Valo
       [not found] ` <0101016eb106d678-62ccf480-a650-47f2-87b3-cb5a03deb013-000000@us-west-2.amazonses.com>
  2 siblings, 2 replies; 8+ messages in thread
From: kbuild test robot @ 2019-11-24  7:52 UTC (permalink / raw)
  To: huangwenabc
  Cc: kbuild-all, linux-wireless, linux-distros, security, libertas-dev

[-- Attachment #1: Type: text/plain, Size: 15322 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on v5.4-rc8 next-20191122]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/huangwenabc-gmail-com/libertas-Fix-two-buffer-overflows-at-parsing-bss-descriptor/20191124-142236
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sh 

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

All warnings (new ones prefixed by >>):

   drivers/net/wireless/marvell/libertas/cfg.c: In function 'lbs_ibss_join_existing':
>> drivers/net/wireless/marvell/libertas/cfg.c:1788:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      u8 *rates = cmd.bss.rates;
      ^~

vim +1788 drivers/net/wireless/marvell/libertas/cfg.c

e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1715  
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1716  static int lbs_ibss_join_existing(struct lbs_private *priv,
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1717  	struct cfg80211_ibss_params *params,
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1718  	struct cfg80211_bss *bss)
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1719  {
9caf03640279e6 drivers/net/wireless/libertas/cfg.c         Johannes Berg 2012-11-29  1720  	const u8 *rates_eid;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1721  	struct cmd_ds_802_11_ad_hoc_join cmd;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1722  	u8 preamble = RADIO_PREAMBLE_SHORT;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1723  	int ret = 0;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1724  
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1725  	/* TODO: set preamble based on scan result */
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1726  	ret = lbs_set_radio(priv, preamble, 1);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1727  	if (ret)
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1728  		goto out;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1729  
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1730  	/*
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1731  	 * Example CMD_802_11_AD_HOC_JOIN command:
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1732  	 *
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1733  	 * command         2c 00         CMD_802_11_AD_HOC_JOIN
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1734  	 * size            65 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1735  	 * sequence        xx xx
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1736  	 * result          00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1737  	 * bssid           02 27 27 97 2f 96
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1738  	 * ssid            49 42 53 53 00 00 00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1739  	 *                 00 00 00 00 00 00 00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1740  	 *                 00 00 00 00 00 00 00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1741  	 *                 00 00 00 00 00 00 00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1742  	 * type            02            CMD_BSS_TYPE_IBSS
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1743  	 * beacon period   64 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1744  	 * dtim period     00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1745  	 * timestamp       00 00 00 00 00 00 00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1746  	 * localtime       00 00 00 00 00 00 00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1747  	 * IE DS           03
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1748  	 * IE DS len       01
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1749  	 * IE DS channel   01
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1750  	 * reserveed       00 00 00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1751  	 * IE IBSS         06
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1752  	 * IE IBSS len     02
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1753  	 * IE IBSS atim    00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1754  	 * reserved        00 00 00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1755  	 * capability      02 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1756  	 * rates           82 84 8b 96 0c 12 18 24 30 48 60 6c 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1757  	 * fail timeout    ff 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1758  	 * probe delay     00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1759  	 */
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1760  	memset(&cmd, 0, sizeof(cmd));
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1761  	cmd.hdr.size = cpu_to_le16(sizeof(cmd));
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1762  
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1763  	memcpy(cmd.bss.bssid, bss->bssid, ETH_ALEN);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1764  	memcpy(cmd.bss.ssid, params->ssid, params->ssid_len);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1765  	cmd.bss.type = CMD_BSS_TYPE_IBSS;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1766  	cmd.bss.beaconperiod = cpu_to_le16(params->beacon_interval);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1767  	cmd.bss.ds.header.id = WLAN_EID_DS_PARAMS;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1768  	cmd.bss.ds.header.len = 1;
683b6d3b31a519 drivers/net/wireless/libertas/cfg.c         Johannes Berg 2012-11-08  1769  	cmd.bss.ds.channel = params->chandef.chan->hw_value;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1770  	cmd.bss.ibss.header.id = WLAN_EID_IBSS_PARAMS;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1771  	cmd.bss.ibss.header.len = 2;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1772  	cmd.bss.ibss.atimwindow = 0;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1773  	cmd.bss.capability = cpu_to_le16(bss->capability & CAPINFO_MASK);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1774  
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1775  	/* set rates to the intersection of our rates and the rates in the
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1776  	   bss */
9caf03640279e6 drivers/net/wireless/libertas/cfg.c         Johannes Berg 2012-11-29  1777  	rcu_read_lock();
9caf03640279e6 drivers/net/wireless/libertas/cfg.c         Johannes Berg 2012-11-29  1778  	rates_eid = ieee80211_bss_get_ie(bss, WLAN_EID_SUPP_RATES);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1779  	if (!rates_eid) {
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1780  		lbs_add_rates(cmd.bss.rates);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1781  	} else {
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1782  		int hw, i;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1783  		u8 rates_max = rates_eid[1];
bb7da3c8c1a225 drivers/net/wireless/marvell/libertas/cfg.c Wen Huang     2019-11-22  1784  		if (rates_max > MAX_RATES) {
bb7da3c8c1a225 drivers/net/wireless/marvell/libertas/cfg.c Wen Huang     2019-11-22  1785  			lbs_deb_join("invalid rates");
bb7da3c8c1a225 drivers/net/wireless/marvell/libertas/cfg.c Wen Huang     2019-11-22  1786  			goto out;
bb7da3c8c1a225 drivers/net/wireless/marvell/libertas/cfg.c Wen Huang     2019-11-22  1787  		}
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14 @1788  		u8 *rates = cmd.bss.rates;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1789  		for (hw = 0; hw < ARRAY_SIZE(lbs_rates); hw++) {
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1790  			u8 hw_rate = lbs_rates[hw].bitrate / 5;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1791  			for (i = 0; i < rates_max; i++) {
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1792  				if (hw_rate == (rates_eid[i+2] & 0x7f)) {
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1793  					u8 rate = rates_eid[i+2];
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1794  					if (rate == 0x02 || rate == 0x04 ||
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1795  					    rate == 0x0b || rate == 0x16)
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1796  						rate |= 0x80;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1797  					*rates++ = rate;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1798  				}
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1799  			}
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1800  		}
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1801  	}
9caf03640279e6 drivers/net/wireless/libertas/cfg.c         Johannes Berg 2012-11-29  1802  	rcu_read_unlock();
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1803  
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1804  	/* Only v8 and below support setting this */
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1805  	if (MRVL_FW_MAJOR_REV(priv->fwrelease) <= 8) {
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1806  		cmd.failtimeout = cpu_to_le16(MRVDRV_ASSOCIATION_TIME_OUT);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1807  		cmd.probedelay = cpu_to_le16(CMD_SCAN_PROBE_DELAY_TIME);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1808  	}
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1809  	ret = lbs_cmd_with_response(priv, CMD_802_11_AD_HOC_JOIN, &cmd);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1810  	if (ret)
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1811  		goto out;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1812  
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1813  	/*
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1814  	 * This is a sample response to CMD_802_11_AD_HOC_JOIN:
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1815  	 *
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1816  	 * response        2c 80
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1817  	 * size            09 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1818  	 * sequence        xx xx
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1819  	 * result          00 00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1820  	 * reserved        00
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1821  	 */
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1822  	lbs_join_post(priv, params, bss->bssid, bss->capability);
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1823  
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1824   out:
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1825  	return ret;
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1826  }
e86dc1ca467644 drivers/net/wireless/libertas/cfg.c         Kiran Divekar 2010-06-14  1827  

:::::: The code at line 1788 was first introduced by commit
:::::: e86dc1ca4676445d9f0dfe35104efe0eb8a2f566 Libertas: cfg80211 support

:::::: TO: Kiran Divekar <dkiran@marvell.com>
:::::: CC: John W. Linville <linville@tuxdriver.com>

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52274 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor
  2019-11-24  7:52 ` kbuild test robot
@ 2019-11-25 12:36   ` Kalle Valo
       [not found]   ` <0101016ea290854e-f5721fd1-1ca7-49ab-9c10-85277bc46c64-000000@us-west-2.amazonses.com>
  1 sibling, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2019-11-25 12:36 UTC (permalink / raw)
  To: kbuild test robot
  Cc: huangwenabc, kbuild-all, linux-wireless, linux-distros, security,
	libertas-dev

kbuild test robot <lkp@intel.com> writes:

> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on wireless-drivers-next/master]
> [also build test WARNING on v5.4-rc8 next-20191122]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url:    https://github.com/0day-ci/linux/commits/huangwenabc-gmail-com/libertas-Fix-two-buffer-overflows-at-parsing-bss-descriptor/20191124-142236
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
> config: sh-allmodconfig (attached as .config)
> compiler: sh4-linux-gcc (GCC) 7.4.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.4.0 make.cross ARCH=sh 
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>    drivers/net/wireless/marvell/libertas/cfg.c: In function 'lbs_ibss_join_existing':
>>> drivers/net/wireless/marvell/libertas/cfg.c:1788:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]

I was wondering why I didn't see this mail in patchwork:

https://patchwork.kernel.org/patch/11257187/

And then I noticed this:

X-Patchwork-Hint: ignore

kbuild team, why are you adding that header? It's really bad for a
maintainer like me who uses patchwork actively, it means that all these
important warnings are not visible in patchwork and can be easily missed
by the maintainers.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [kbuild-all] Re: [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor
       [not found]   ` <0101016ea290854e-f5721fd1-1ca7-49ab-9c10-85277bc46c64-000000@us-west-2.amazonses.com>
@ 2019-11-25 14:29     ` " Philip Li
  2019-11-27 18:23       ` Guenter Roeck
  0 siblings, 1 reply; 8+ messages in thread
From: Philip Li @ 2019-11-25 14:29 UTC (permalink / raw)
  To: Kalle Valo
  Cc: kbuild test robot, huangwenabc, kbuild-all, linux-wireless,
	linux-distros, security, libertas-dev

On Mon, Nov 25, 2019 at 12:36:50PM +0000, Kalle Valo wrote:
> kbuild test robot <lkp@intel.com> writes:
> 
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on wireless-drivers-next/master]
> > [also build test WARNING on v5.4-rc8 next-20191122]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >
> > url:    https://github.com/0day-ci/linux/commits/huangwenabc-gmail-com/libertas-Fix-two-buffer-overflows-at-parsing-bss-descriptor/20191124-142236
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
> > config: sh-allmodconfig (attached as .config)
> > compiler: sh4-linux-gcc (GCC) 7.4.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         GCC_VERSION=7.4.0 make.cross ARCH=sh 
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> >    drivers/net/wireless/marvell/libertas/cfg.c: In function 'lbs_ibss_join_existing':
> >>> drivers/net/wireless/marvell/libertas/cfg.c:1788:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
> 
> I was wondering why I didn't see this mail in patchwork:
> 
> https://patchwork.kernel.org/patch/11257187/
> 
> And then I noticed this:
> 
> X-Patchwork-Hint: ignore
> 
> kbuild team, why are you adding that header? It's really bad for a
thanks for the feedback, early on we received another feedback to suggest
for adding this, refer to https://gitlab.freedesktop.org/patchwork-fdo/patchwork-fdo/issues/21
for detail. Since there's no further input regarding this usage, we keep
that flag. If this is not suitable, we can investigate other way to fullfill
both requirements.

> maintainer like me who uses patchwork actively, it means that all these
> important warnings are not visible in patchwork and can be easily missed
> by the maintainers.
> 
> -- 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> _______________________________________________
> kbuild-all mailing list -- kbuild-all@lists.01.org
> To unsubscribe send an email to kbuild-all-leave@lists.01.org

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [kbuild-all] Re: [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor
  2019-11-25 14:29     ` [kbuild-all] " Philip Li
@ 2019-11-27 18:23       ` Guenter Roeck
  2019-11-28  1:53         ` Rong Chen
  0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2019-11-27 18:23 UTC (permalink / raw)
  To: Philip Li
  Cc: Kalle Valo, kbuild test robot, huangwenabc, kbuild-all,
	linux-wireless, linux-distros, security, libertas-dev

On Mon, Nov 25, 2019 at 10:29:52PM +0800, Philip Li wrote:
> On Mon, Nov 25, 2019 at 12:36:50PM +0000, Kalle Valo wrote:
> > kbuild test robot <lkp@intel.com> writes:
> > 
> > > Thank you for the patch! Perhaps something to improve:
> > >
> > > [auto build test WARNING on wireless-drivers-next/master]
> > > [also build test WARNING on v5.4-rc8 next-20191122]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help
> > > improve the system. BTW, we also suggest to use '--base' option to specify the
> > > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > >
> > > url:    https://github.com/0day-ci/linux/commits/huangwenabc-gmail-com/libertas-Fix-two-buffer-overflows-at-parsing-bss-descriptor/20191124-142236
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
> > > config: sh-allmodconfig (attached as .config)
> > > compiler: sh4-linux-gcc (GCC) 7.4.0
> > > reproduce:
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # save the attached .config to linux build tree
> > >         GCC_VERSION=7.4.0 make.cross ARCH=sh 
> > >
> > > If you fix the issue, kindly add following tag
> > > Reported-by: kbuild test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > >    drivers/net/wireless/marvell/libertas/cfg.c: In function 'lbs_ibss_join_existing':
> > >>> drivers/net/wireless/marvell/libertas/cfg.c:1788:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
> > 
> > I was wondering why I didn't see this mail in patchwork:
> > 
> > https://patchwork.kernel.org/patch/11257187/
> > 
> > And then I noticed this:
> > 
> > X-Patchwork-Hint: ignore
> > 
> > kbuild team, why are you adding that header? It's really bad for a
> thanks for the feedback, early on we received another feedback to suggest
> for adding this, refer to https://gitlab.freedesktop.org/patchwork-fdo/patchwork-fdo/issues/21
> for detail. Since there's no further input regarding this usage, we keep
> that flag. If this is not suitable, we can investigate other way to fullfill
> both requirements.
> 

I second Kalle's comment; this is really bad.

Note that the above referenced link suggested to add
	X-Patchwork-Hint: comment
to e-mail headers. Instead, you added:
	X-Patchwork-Hint: ignore
which is substantially different. Also, the problem was with a _patch_
sent by the robot, not with direct feedback. On top of that, the
suggestion was really to add "X-Patchwork-Hint: comment" to _patches_
sent by the robot, not to everything. It should be fine to add
"X-Patchwork-Hint: ignore" to patches only as long as other feedback
is still provided and added to patchwork. That should meet all
requirements.

Thanks,
Guenter

> > maintainer like me who uses patchwork actively, it means that all these
> > important warnings are not visible in patchwork and can be easily missed
> > by the maintainers.
> > 
> > -- 
> > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> > _______________________________________________
> > kbuild-all mailing list -- kbuild-all@lists.01.org
> > To unsubscribe send an email to kbuild-all-leave@lists.01.org

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [kbuild-all] Re: [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor
  2019-11-27 18:23       ` Guenter Roeck
@ 2019-11-28  1:53         ` Rong Chen
  0 siblings, 0 replies; 8+ messages in thread
From: Rong Chen @ 2019-11-28  1:53 UTC (permalink / raw)
  To: Guenter Roeck, Philip Li, Kalle Valo
  Cc: kbuild test robot, huangwenabc, kbuild-all, linux-wireless,
	linux-distros, security, libertas-dev



On 11/28/19 2:23 AM, Guenter Roeck wrote:
> On Mon, Nov 25, 2019 at 10:29:52PM +0800, Philip Li wrote:
>> On Mon, Nov 25, 2019 at 12:36:50PM +0000, Kalle Valo wrote:
>>> kbuild test robot <lkp@intel.com> writes:
>>>
>>>> Thank you for the patch! Perhaps something to improve:
>>>>
>>>> [auto build test WARNING on wireless-drivers-next/master]
>>>> [also build test WARNING on v5.4-rc8 next-20191122]
>>>> [if your patch is applied to the wrong git tree, please drop us a note to help
>>>> improve the system. BTW, we also suggest to use '--base' option to specify the
>>>> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>>>>
>>>> url:    https://github.com/0day-ci/linux/commits/huangwenabc-gmail-com/libertas-Fix-two-buffer-overflows-at-parsing-bss-descriptor/20191124-142236
>>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
>>>> config: sh-allmodconfig (attached as .config)
>>>> compiler: sh4-linux-gcc (GCC) 7.4.0
>>>> reproduce:
>>>>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>>          chmod +x ~/bin/make.cross
>>>>          # save the attached .config to linux build tree
>>>>          GCC_VERSION=7.4.0 make.cross ARCH=sh
>>>>
>>>> If you fix the issue, kindly add following tag
>>>> Reported-by: kbuild test robot <lkp@intel.com>
>>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>>     drivers/net/wireless/marvell/libertas/cfg.c: In function 'lbs_ibss_join_existing':
>>>>>> drivers/net/wireless/marvell/libertas/cfg.c:1788:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
>>> I was wondering why I didn't see this mail in patchwork:
>>>
>>> https://patchwork.kernel.org/patch/11257187/
>>>
>>> And then I noticed this:
>>>
>>> X-Patchwork-Hint: ignore
>>>
>>> kbuild team, why are you adding that header? It's really bad for a
>> thanks for the feedback, early on we received another feedback to suggest
>> for adding this, refer to https://gitlab.freedesktop.org/patchwork-fdo/patchwork-fdo/issues/21
>> for detail. Since there's no further input regarding this usage, we keep
>> that flag. If this is not suitable, we can investigate other way to fullfill
>> both requirements.
>>
> I second Kalle's comment; this is really bad.
>
> Note that the above referenced link suggested to add
> 	X-Patchwork-Hint: comment
> to e-mail headers. Instead, you added:
> 	X-Patchwork-Hint: ignore
> which is substantially different. Also, the problem was with a _patch_
> sent by the robot, not with direct feedback. On top of that, the
> suggestion was really to add "X-Patchwork-Hint: comment" to _patches_
> sent by the robot, not to everything. It should be fine to add
> "X-Patchwork-Hint: ignore" to patches only as long as other feedback
> is still provided and added to patchwork. That should meet all
> requirements.
>
> Thanks,
> Guenter

Hi Kalle, Guenter

Thanks so much for your help, we have removed the hint header in build 
report mails and
still keep it in patch mails.

Best Regards,
Rong Chen

>
>>> maintainer like me who uses patchwork actively, it means that all these
>>> important warnings are not visible in patchwork and can be easily missed
>>> by the maintainers.
>>>
>>> -- 
>>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>>> _______________________________________________
>>> kbuild-all mailing list -- kbuild-all@lists.01.org
>>> To unsubscribe send an email to kbuild-all-leave@lists.01.org
> _______________________________________________
> kbuild-all mailing list -- kbuild-all@lists.01.org
> To unsubscribe send an email to kbuild-all-leave@lists.01.org


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor
  2019-11-22  5:29 [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor huangwenabc
  2019-11-24  7:52 ` kbuild test robot
@ 2019-11-28  8:00 ` Kalle Valo
       [not found] ` <0101016eb106d678-62ccf480-a650-47f2-87b3-cb5a03deb013-000000@us-west-2.amazonses.com>
  2 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2019-11-28  8:00 UTC (permalink / raw)
  To: huangwenabc; +Cc: linux-wireless, linux-distros, security, libertas-dev

huangwenabc@gmail.com wrote:

> From: Wen Huang <huangwenabc@gmail.com>
> 
> add_ie_rates() copys rates without checking the length 
> in bss descriptor from remote AP.when victim connects to 
> remote attacker, this may trigger buffer overflow.
> lbs_ibss_join_existing() copys rates without checking the length 
> in bss descriptor from remote IBSS node.when victim connects to 
> remote attacker, this may trigger buffer overflow.
> Fix them by putting the length check before performing copy.
> 
> This fix addresses CVE-2019-14896 and CVE-2019-14897.
> 
> Signed-off-by: Wen Huang <huangwenabc@gmail.com>

Please fix the warning reported by kbuild bot.

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/patch/11257187/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor
       [not found]   ` <CADt2dQfbnk5WgDk=oeWjE1tziCEem-3fhhA68Pmr_fo0pZ_V=g@mail.gmail.com>
@ 2019-11-28 11:54     ` Kalle Valo
  0 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2019-11-28 11:54 UTC (permalink / raw)
  To: Wen Huang; +Cc: linux-wireless, libertas-dev

Wen Huang <huangwenabc@gmail.com> writes:

> I have modified the patch and submmit:
> https://patchwork.kernel.org/patch/11265751/ 

Thanks, but few tips for the future (no need to resend because of
these):

* don't use HTML in email

* use v2, v3 and so on to identify the version of the patch

* do not top post

More info in the link below, I suggest to carefully study that. Better
chances of getting your patches accepted that way.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-22  5:29 [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor huangwenabc
2019-11-24  7:52 ` kbuild test robot
2019-11-25 12:36   ` Kalle Valo
     [not found]   ` <0101016ea290854e-f5721fd1-1ca7-49ab-9c10-85277bc46c64-000000@us-west-2.amazonses.com>
2019-11-25 14:29     ` [kbuild-all] " Philip Li
2019-11-27 18:23       ` Guenter Roeck
2019-11-28  1:53         ` Rong Chen
2019-11-28  8:00 ` Kalle Valo
     [not found] ` <0101016eb106d678-62ccf480-a650-47f2-87b3-cb5a03deb013-000000@us-west-2.amazonses.com>
     [not found]   ` <CADt2dQfbnk5WgDk=oeWjE1tziCEem-3fhhA68Pmr_fo0pZ_V=g@mail.gmail.com>
2019-11-28 11:54     ` Kalle Valo

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git