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@nxp.com Fixes: 12190c5d80bd ("mwifiex: add cfg80211 start_ap and stop_ap handlers") Cc: stable@vger.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@chromium.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