lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZXpjjmD5Se7axJju@google.com> Date: Wed, 13 Dec 2023 18:08:14 -0800 From: Brian Norris <briannorris@...omium.org> To: David Lin <yu-hao.lin@....com> Cc: linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org, kvalo@...nel.org, francesco@...cini.it, tsung-hsien.hsieh@....com, stable@...r.kernel.org Subject: Re: [PATCH v2] wifi: mwifiex: fix STA cannot connect to AP Hi, Nitpick: "fix STA cannot connect to AP" isn't the best commit message; that could describe an enormous number of fixes. Maybe something more like "Configure BSSID consistently when starting AP"? On Sat, Dec 09, 2023 at 07:41:27AM +0800, David Lin wrote: > AP BSSID configuration is missing at AP start. > Without this fix, FW returns STA interface MAC address after first init. > When hostapd restarts, it gets MAC address from netdev before driver > sets STA MAC to netdev again. Now MAC address between hostapd and net > interface are different causes STA cannot connect to AP. > After that MAC address of uap0 mlan0 become the same. And issue > disappears after following hostapd restart (another issue is AP/STA MAC > address become the same). > This patch fixes the issue cleanly. > > Signed-off-by: David Lin <yu-hao.lin@....com> > Fixes: 12190c5d80bd ("mwifiex: add cfg80211 start_ap and stop_ap handlers") > Cc: stable@...r.kernel.org > > --- > > v2: > - v1 was a not finished patch that was send to the LKML by mistake Looks fine to me: Acked-by: Brian Norris <briannorris@...omium.org> > drivers/net/wireless/marvell/mwifiex/cfg80211.c | 2 ++ > drivers/net/wireless/marvell/mwifiex/fw.h | 1 + > drivers/net/wireless/marvell/mwifiex/ioctl.h | 1 + > drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 8 ++++++++ > 4 files changed, 12 insertions(+) > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c > @@ -487,6 +488,13 @@ mwifiex_uap_bss_param_prepare(u8 *tlv, void *cmd_buf, u16 *param_size) > int i; > u16 cmd_size = *param_size; > > + mac_tlv = (struct host_cmd_tlv_mac_addr *)tlv; Not directly related to this patch, but while you're expanding the size of this command buffer: it always felt like a security-hole-in-waiting that none of these command producers do any kinds of bounds checking. We're just "lucky" that these function only generate contents of ~100 bytes at max, while MWIFIEX_SIZE_OF_CMD_BUFFER=2048. But, just add a few more user-space controlled TLV params, and boom, we'll have ourselves a nice little CVE. It probably wouldn't hurt to significantly write much of this driver, but at a minimum, we could probably use a few checks like this: cmd_size += sizeof(struct host_cmd_tlv_mac_addr); if (cmd_size > MWIFIEX_SIZE_OF_CMD_BUFFER) return -1; // Only touch tlv *after* the bounds check. That doesn't need to block this patch, of course. Brian > + mac_tlv->header.type = cpu_to_le16(TLV_TYPE_UAP_MAC_ADDRESS); > + mac_tlv->header.len = cpu_to_le16(ETH_ALEN); > + memcpy(mac_tlv->mac_addr, bss_cfg->mac_addr, ETH_ALEN); > + cmd_size += sizeof(struct host_cmd_tlyyv_mac_addr); > + tlv += sizeof(struct host_cmd_tlv_mac_addr); > + > if (bss_cfg->ssid.ssid_len) { > ssid = (struct host_cmd_tlv_ssid *)tlv; > ssid->header.type = cpu_to_le16(TLV_TYPE_UAP_SSID); > > base-commit: 783004b6dbda2cfe9a552a4cc9c1d168a2068f6c > -- > 2.25.1 >
Powered by blists - more mailing lists