* [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor @ 2019-11-22 5:29 huangwenabc 2019-11-24 7:52 ` kbuild test robot ` (3 more replies) 0 siblings, 4 replies; 18+ 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 related [flat|nested] 18+ 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 ` (2 subsequent siblings) 3 siblings, 2 replies; 18+ 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] 18+ 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; 18+ 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] 18+ messages in thread
[parent not found: <0101016ea290854e-f5721fd1-1ca7-49ab-9c10-85277bc46c64-000000@us-west-2.amazonses.com>]
* 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; 18+ 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] 18+ 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; 18+ 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] 18+ 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 2020-03-24 15:19 ` Kalle Valo 0 siblings, 1 reply; 18+ 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] 18+ messages in thread
* Re: [kbuild-all] Re: [PATCH] libertas: Fix two buffer overflows at parsing bss descriptor 2019-11-28 1:53 ` Rong Chen @ 2020-03-24 15:19 ` Kalle Valo 0 siblings, 0 replies; 18+ messages in thread From: Kalle Valo @ 2020-03-24 15:19 UTC (permalink / raw) To: Rong Chen Cc: Guenter Roeck, Philip Li, kbuild test robot, huangwenabc, kbuild-all, linux-wireless, libertas-dev Rong Chen <rong.a.chen@intel.com> writes: > 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. This is now working perfectly, here's a recent example: https://patchwork.kernel.org/patch/11431301/ I cannot stress enough how much seeing kbuild bot warning in patchwork helps my work as a maintainer. So thank you Guenter for the support and Rong fixing it! -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 18+ 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> 2020-01-09 14:12 ` Nicolai Stange 3 siblings, 0 replies; 18+ 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] 18+ messages in thread
[parent not found: <0101016eb106d678-62ccf480-a650-47f2-87b3-cb5a03deb013-000000@us-west-2.amazonses.com>]
[parent not found: <CADt2dQfbnk5WgDk=oeWjE1tziCEem-3fhhA68Pmr_fo0pZ_V=g@mail.gmail.com>]
* 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; 18+ 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] 18+ 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 ` (2 preceding siblings ...) [not found] ` <0101016eb106d678-62ccf480-a650-47f2-87b3-cb5a03deb013-000000@us-west-2.amazonses.com> @ 2020-01-09 14:12 ` Nicolai Stange 2020-01-14 10:39 ` [PATCH 0/2] libertas: fix rates overflow code path in lbs_ibss_join_existing() Nicolai Stange 3 siblings, 1 reply; 18+ messages in thread From: Nicolai Stange @ 2020-01-09 14:12 UTC (permalink / raw) To: Kalle Valo Cc: huangwenabc, linux-wireless, Takashi Iwai, Nicolai Stange, Miroslav Benes Hi, the patch queued as e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss descriptor") at the wireless tree doesn't look completely correct to me. This hunk here... diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c index 57edfada0665..c9401c121a14 100644 --- a/drivers/net/wireless/marvell/libertas/cfg.c +++ b/drivers/net/wireless/marvell/libertas/cfg.c @@ -1775,9 +1782,12 @@ static int lbs_ibss_join_existing(struct lbs_private *priv, if (!rates_eid) { lbs_add_rates(cmd.bss.rates); } else { - int hw, i; - u8 rates_max = rates_eid[1]; - u8 *rates = cmd.bss.rates; + rates_max = rates_eid[1]; + if (rates_max > MAX_RATES) { + lbs_deb_join("invalid rates"); + goto out; ... makes the error path skip over the rcu_read_unlock() following later in the code and leaves the RCU read section unbalanced. Also, I'm wondering if ret should perhaps get set to some -Exxxx in case of rates_max > MAX_RATES? Thanks, Nicolai + } + rates = cmd.bss.rates; for (hw = 0; hw < ARRAY_SIZE(lbs_rates); hw++) { u8 hw_rate = lbs_rates[hw].bitrate / 5; for (i = 0; i < rates_max; i++) { -- SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg), GF: Felix Imendörffer ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 0/2] libertas: fix rates overflow code path in lbs_ibss_join_existing() 2020-01-09 14:12 ` Nicolai Stange @ 2020-01-14 10:39 ` Nicolai Stange 2020-01-14 10:39 ` [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held Nicolai Stange 2020-01-14 10:39 ` [PATCH 2/2] libertas: make lbs_ibss_join_existing() return error code on rates overflow Nicolai Stange 0 siblings, 2 replies; 18+ messages in thread From: Nicolai Stange @ 2020-01-14 10:39 UTC (permalink / raw) To: Kalle Valo Cc: David S. Miller, Wen Huang, Nicolai Stange, libertas-dev, linux-wireless, netdev, linux-kernel, Takashi Iwai, Miroslav Benes Hi, these two patches here attempt to cleanup two related issues ([1]) introduced with commit e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss descriptor"), currently queued at the wireless-drivers tree. Feel free to squash this into one commit. I don't own the hardware and did some compile-testing on x86_64 only. Thanks, Nicolai [1] https://lkml.kernel.org/r/87woa04t2v.fsf@suse.de Nicolai Stange (2): libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held libertas: make lbs_ibss_join_existing() return error code on rates overflow drivers/net/wireless/marvell/libertas/cfg.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.16.4 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held 2020-01-14 10:39 ` [PATCH 0/2] libertas: fix rates overflow code path in lbs_ibss_join_existing() Nicolai Stange @ 2020-01-14 10:39 ` Nicolai Stange 2020-01-14 13:43 ` Kalle Valo ` (2 more replies) 2020-01-14 10:39 ` [PATCH 2/2] libertas: make lbs_ibss_join_existing() return error code on rates overflow Nicolai Stange 1 sibling, 3 replies; 18+ messages in thread From: Nicolai Stange @ 2020-01-14 10:39 UTC (permalink / raw) To: Kalle Valo Cc: David S. Miller, Wen Huang, Nicolai Stange, libertas-dev, linux-wireless, netdev, linux-kernel, Takashi Iwai, Miroslav Benes Commit e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss descriptor") introduced a bounds check on the number of supplied rates to lbs_ibss_join_existing(). Unfortunately, it introduced a return path from within a RCU read side critical section without a corresponding rcu_read_unlock(). Fix this. Fixes: e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss descriptor") Signed-off-by: Nicolai Stange <nstange@suse.de> --- drivers/net/wireless/marvell/libertas/cfg.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c index c9401c121a14..68985d766349 100644 --- a/drivers/net/wireless/marvell/libertas/cfg.c +++ b/drivers/net/wireless/marvell/libertas/cfg.c @@ -1785,6 +1785,7 @@ static int lbs_ibss_join_existing(struct lbs_private *priv, rates_max = rates_eid[1]; if (rates_max > MAX_RATES) { lbs_deb_join("invalid rates"); + rcu_read_unlock(); goto out; } rates = cmd.bss.rates; -- 2.16.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held 2020-01-14 10:39 ` [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held Nicolai Stange @ 2020-01-14 13:43 ` Kalle Valo 2020-01-15 6:21 ` Nicolai Stange 2020-01-26 15:14 ` Kalle Valo 2020-01-27 14:37 ` Kalle Valo 2 siblings, 1 reply; 18+ messages in thread From: Kalle Valo @ 2020-01-14 13:43 UTC (permalink / raw) To: Nicolai Stange Cc: David S. Miller, Wen Huang, libertas-dev, linux-wireless, netdev, linux-kernel, Takashi Iwai, Miroslav Benes Nicolai Stange <nstange@suse.de> writes: > Commit e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss > descriptor") introduced a bounds check on the number of supplied rates to > lbs_ibss_join_existing(). > > Unfortunately, it introduced a return path from within a RCU read side > critical section without a corresponding rcu_read_unlock(). Fix this. > > Fixes: e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss > descriptor") This should be in one line, I'll fix it during commit. -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held 2020-01-14 13:43 ` Kalle Valo @ 2020-01-15 6:21 ` Nicolai Stange 0 siblings, 0 replies; 18+ messages in thread From: Nicolai Stange @ 2020-01-15 6:21 UTC (permalink / raw) To: Kalle Valo Cc: Nicolai Stange, David S. Miller, Wen Huang, libertas-dev, linux-wireless, netdev, linux-kernel, Takashi Iwai, Miroslav Benes Kalle Valo <kvalo@codeaurora.org> writes: > Nicolai Stange <nstange@suse.de> writes: > >> Commit e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss >> descriptor") introduced a bounds check on the number of supplied rates to >> lbs_ibss_join_existing(). >> >> Unfortunately, it introduced a return path from within a RCU read side >> critical section without a corresponding rcu_read_unlock(). Fix this. >> >> Fixes: e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss >> descriptor") > > This should be in one line, I'll fix it during commit. Thanks! -- SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg), GF: Felix Imendörffer ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held 2020-01-14 10:39 ` [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held Nicolai Stange 2020-01-14 13:43 ` Kalle Valo @ 2020-01-26 15:14 ` Kalle Valo 2020-01-27 14:37 ` Kalle Valo 2 siblings, 0 replies; 18+ messages in thread From: Kalle Valo @ 2020-01-26 15:14 UTC (permalink / raw) To: Nicolai Stange Cc: David S. Miller, Wen Huang, Nicolai Stange, libertas-dev, linux-wireless, netdev, linux-kernel, Takashi Iwai, Miroslav Benes Nicolai Stange <nstange@suse.de> wrote: > Commit e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss > descriptor") introduced a bounds check on the number of supplied rates to > lbs_ibss_join_existing(). > > Unfortunately, it introduced a return path from within a RCU read side > critical section without a corresponding rcu_read_unlock(). Fix this. > > Fixes: e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss > descriptor") > Signed-off-by: Nicolai Stange <nstange@suse.de> I'll queue these to v5.5, unless Linus releases the final today and then they will go to v5.6. -- https://patchwork.kernel.org/patch/11331869/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held 2020-01-14 10:39 ` [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held Nicolai Stange 2020-01-14 13:43 ` Kalle Valo 2020-01-26 15:14 ` Kalle Valo @ 2020-01-27 14:37 ` Kalle Valo 2 siblings, 0 replies; 18+ messages in thread From: Kalle Valo @ 2020-01-27 14:37 UTC (permalink / raw) To: Nicolai Stange Cc: David S. Miller, Wen Huang, Nicolai Stange, libertas-dev, linux-wireless, netdev, linux-kernel, Takashi Iwai, Miroslav Benes Nicolai Stange <nstange@suse.de> wrote: > Commit e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss > descriptor") introduced a bounds check on the number of supplied rates to > lbs_ibss_join_existing(). > > Unfortunately, it introduced a return path from within a RCU read side > critical section without a corresponding rcu_read_unlock(). Fix this. > > Fixes: e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss descriptor") > Signed-off-by: Nicolai Stange <nstange@suse.de> 2 patches applied to wireless-drivers.git, thanks. c7bf1fb7ddca libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held 1754c4f60aaf libertas: make lbs_ibss_join_existing() return error code on rates overflow -- https://patchwork.kernel.org/patch/11331869/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] libertas: make lbs_ibss_join_existing() return error code on rates overflow 2020-01-14 10:39 ` [PATCH 0/2] libertas: fix rates overflow code path in lbs_ibss_join_existing() Nicolai Stange 2020-01-14 10:39 ` [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held Nicolai Stange @ 2020-01-14 10:39 ` Nicolai Stange 2020-01-14 13:44 ` Kalle Valo 1 sibling, 1 reply; 18+ messages in thread From: Nicolai Stange @ 2020-01-14 10:39 UTC (permalink / raw) To: Kalle Valo Cc: David S. Miller, Wen Huang, Nicolai Stange, libertas-dev, linux-wireless, netdev, linux-kernel, Takashi Iwai, Miroslav Benes Commit e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss descriptor") introduced a bounds check on the number of supplied rates to lbs_ibss_join_existing() and made it to return on overflow. However, the aforementioned commit doesn't set the return value accordingly and thus, lbs_ibss_join_existing() would return with zero even though it failed. Make lbs_ibss_join_existing return -EINVAL in case the bounds check on the number of supplied rates fails. Fixes: e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss descriptor") Signed-off-by: Nicolai Stange <nstange@suse.de> --- drivers/net/wireless/marvell/libertas/cfg.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c index 68985d766349..4e3de684928b 100644 --- a/drivers/net/wireless/marvell/libertas/cfg.c +++ b/drivers/net/wireless/marvell/libertas/cfg.c @@ -1786,6 +1786,7 @@ static int lbs_ibss_join_existing(struct lbs_private *priv, if (rates_max > MAX_RATES) { lbs_deb_join("invalid rates"); rcu_read_unlock(); + ret = -EINVAL; goto out; } rates = cmd.bss.rates; -- 2.16.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] libertas: make lbs_ibss_join_existing() return error code on rates overflow 2020-01-14 10:39 ` [PATCH 2/2] libertas: make lbs_ibss_join_existing() return error code on rates overflow Nicolai Stange @ 2020-01-14 13:44 ` Kalle Valo 0 siblings, 0 replies; 18+ messages in thread From: Kalle Valo @ 2020-01-14 13:44 UTC (permalink / raw) To: Nicolai Stange Cc: David S. Miller, Wen Huang, libertas-dev, linux-wireless, netdev, linux-kernel, Takashi Iwai, Miroslav Benes Nicolai Stange <nstange@suse.de> writes: > Commit e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss > descriptor") introduced a bounds check on the number of supplied rates to > lbs_ibss_join_existing() and made it to return on overflow. > > However, the aforementioned commit doesn't set the return value accordingly > and thus, lbs_ibss_join_existing() would return with zero even though it > failed. > > Make lbs_ibss_join_existing return -EINVAL in case the bounds check on the > number of supplied rates fails. > > Fixes: e5e884b42639 ("libertas: Fix two buffer overflows at parsing bss > descriptor") This should be in one line, I'll fix it during commit. -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-03-24 15:20 UTC | newest] Thread overview: 18+ 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 2020-03-24 15:19 ` Kalle Valo 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 2020-01-09 14:12 ` Nicolai Stange 2020-01-14 10:39 ` [PATCH 0/2] libertas: fix rates overflow code path in lbs_ibss_join_existing() Nicolai Stange 2020-01-14 10:39 ` [PATCH 1/2] libertas: don't exit from lbs_ibss_join_existing() with RCU read lock held Nicolai Stange 2020-01-14 13:43 ` Kalle Valo 2020-01-15 6:21 ` Nicolai Stange 2020-01-26 15:14 ` Kalle Valo 2020-01-27 14:37 ` Kalle Valo 2020-01-14 10:39 ` [PATCH 2/2] libertas: make lbs_ibss_join_existing() return error code on rates overflow Nicolai Stange 2020-01-14 13:44 ` Kalle Valo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).