| Message ID | 20211029064012.7619-1-wangxinpeng@uniontech.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scan: Solve the problem of garbled characters in the scanned ssid | expand |
On Fri, 2021-10-29 at 14:40 +0800, xinpeng.wang wrote: > Using Netgear WN11V2 usb wireless network card, it is easy to have garbled characters i > n the scanned ssid. This is because the driver sends the problem packets to wpa through > netlink. These packets are only partly seen through wireshark, but the missing parts when > sent to wpa are some random values, which may cause the read ssid to be garbled. > In the update scan res, check whether the sum of the length of each ie in ies is the same > as ie_len. If it is not the same, it is considered to be abnormal packet and discard it. Seems like something the *driver* should do? johannes
>> On Fri, 2021-10-29 at 14:40 +0800, xinpeng.wang wrote: > > Using Netgear WN11V2 usb wireless network card, it is easy to have garbled characters i > > n the scanned ssid. This is because the driver sends the problem packets to wpa through > > netlink. These packets are only partly seen through wireshark, but the missing parts when > > sent to wpa are some random values, which may cause the read ssid to be garbled. > > In the update scan res, check whether the sum of the length of each ie in ies is the same > > as ie_len. If it is not the same, it is considered to be abnormal packet and discard it. >> Seems like something the *driver* should do? thank you for your reply. I think the driver should be able to ensure that what is passed to wpa is correct, but wpa should also be able to filter these packets and not be affected by these abnormal packets. -- Thanks! xinpeng.wang
On Fri, 2021-10-29 at 15:07 +0800, 王鑫鹏 wrote: > > > On Fri, 2021-10-29 at 14:40 +0800, xinpeng.wang wrote: > > > Using Netgear WN11V2 usb wireless network card, it is easy to have > > > garbled characters i > > > n the scanned ssid. This is because the driver sends the problem > > > packets to wpa through > > > netlink. These packets are only partly seen through wireshark, but > > > the missing parts when > > > sent to wpa are some random values, which may cause the read ssid > > > to be garbled. > > > In the update scan res, check whether the sum of the length of > > > each ie in ies is the same > > > as ie_len. If it is not the same, it is considered to be abnormal > > > packet and discard it. > > > > Seems like something the *driver* should do? > > thank you for your reply. > > I think the driver should be able to ensure that what is passed to wpa > is correct, but wpa > should also be able to filter these packets and not be affected by > these abnormal packets. I'm not sure I agree. If we wanted to put workarounds for all hardware bugs into wpa_supplicant, that'd probably end up quite a mess. Such things are better handled by the specific driver. Clearly wpa_supplicant needs to ensure its own consistency isn't damaged, e.g. that it doesn't crash on malformed (perhaps maliciously so) frames, but as long as the frame is just garbage, IMHO the driver should do such things. Note also that in this particular instance, I believe you're now dropping beacon frames that don't have completely well-formed elements, which is very well known to cause problems. johannes
> > > On Fri, 2021-10-29 at 14:40 +0800, xinpeng.wang wrote: > > > Using Netgear WN11V2 usb wireless network card, it is easy to have > > > garbled characters i > > > n the scanned ssid. This is because the driver sends the problem > > > packets to wpa through > > > netlink. These packets are only partly seen through wireshark, but > > > the missing parts when > > > sent to wpa are some random values, which may cause the read ssid > > > to be garbled. > > > In the update scan res, check whether the sum of the length of > > > each ie in ies is the same > > > as ie_len. If it is not the same, it is considered to be abnormal > > > packet and discard it. > > > > Seems like something the *driver* should do? > > > thank you for your reply. > > > I think the driver should be able to ensure that what is passed to wpa > > is correct, but wpa > > should also be able to filter these packets and not be affected by > > these abnormal packets. > I'm not sure I agree. If we wanted to put workarounds for all hardware > bugs into wpa_supplicant, that'd probably end up quite a mess. Such > things are better handled by the specific driver. > Clearly wpa_supplicant needs to ensure its own consistency isn't > damaged, e.g. that it doesn't crash on malformed (perhaps maliciously > so) frames, but as long as the frame is just garbage, IMHO the driver > should do such things. > Note also that in this particular instance, I believe you're now > dropping beacon frames that don't have completely well-formed elements, > which is very well known to cause problems. thank you for your reply. I will find a way to solve this problem from the driver. Thanks. ------ xinpeng.wang
diff --git a/wpa_supplicant/bss.c b/wpa_supplicant/bss.c index e13783ce1..1a546fd38 100644 --- a/wpa_supplicant/bss.c +++ b/wpa_supplicant/bss.c @@ -779,6 +779,12 @@ void wpa_bss_update_scan_res(struct wpa_supplicant *wpa_s, MACSTR, MAC2STR(res->bssid)); return; } + if (wpa_scan_check_ie(res)) + { + wpa_dbg(wpa_s, MSG_DEBUG, "BSS: IE check error ssid %s for " + MACSTR, wpa_ssid_txt(ssid+2, ssid[1]),MAC2STR(res->bssid)); + return; + } p2p = wpa_scan_get_vendor_ie(res, P2P_IE_VENDOR_TYPE); #ifdef CONFIG_P2P diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c index 97a8d9a63..676c177a3 100644 --- a/wpa_supplicant/scan.c +++ b/wpa_supplicant/scan.c @@ -1866,6 +1866,32 @@ static int wpa_scan_get_max_rate(const struct wpa_scan_res *res) return rate; } +/** + * wpa_scan_check_ie - Check whether the ies in the scan result is correct + * @res: Scan result entry * + * Returns: 0 means correct,-1 means error + * + * This function checks that the content in ies is legal ie, the + * sum of the length of all ie is equal to ie_len. + */ +int wpa_scan_check_ie(const struct wpa_scan_res *res) +{ + size_t ie_len = res->ie_len; + const struct element *elem; + const u8 *end, *pos; + + /* Use the Beacon frame IEs if res->ie_len is not available */ + if (!ie_len) + ie_len = res->beacon_ie_len; + pos = (const u8 *) (res + 1); + end = pos + res->ie_len; + + for_each_element(elem,pos,ie_len); + + if ((const u8 *)elem == end) + return 0; + return -1; +} /** * wpa_scan_get_ie - Fetch a specified information element from a scan result diff --git a/wpa_supplicant/scan.h b/wpa_supplicant/scan.h index d1780eb09..117dd6e02 100644 --- a/wpa_supplicant/scan.h +++ b/wpa_supplicant/scan.h @@ -51,6 +51,7 @@ wpa_supplicant_get_scan_results(struct wpa_supplicant *wpa_s, struct scan_info *info, int new_scan); int wpa_supplicant_update_scan_results(struct wpa_supplicant *wpa_s); const u8 * wpa_scan_get_ie(const struct wpa_scan_res *res, u8 ie); +int wpa_scan_check_ie(const struct wpa_scan_res *res); const u8 * wpa_scan_get_vendor_ie(const struct wpa_scan_res *res, u32 vendor_type); const u8 * wpa_scan_get_vendor_ie_beacon(const struct wpa_scan_res *res,
Using Netgear WN11V2 usb wireless network card, it is easy to have garbled characters i n the scanned ssid. This is because the driver sends the problem packets to wpa through netlink. These packets are only partly seen through wireshark, but the missing parts when sent to wpa are some random values, which may cause the read ssid to be garbled. In the update scan res, check whether the sum of the length of each ie in ies is the same as ie_len. If it is not the same, it is considered to be abnormal packet and discard it. Signed-off-by: xinpeng.wang <wangxinpeng@uniontech.com> --- wpa_supplicant/bss.c | 6 ++++++ wpa_supplicant/scan.c | 26 ++++++++++++++++++++++++++ wpa_supplicant/scan.h | 1 + 3 files changed, 33 insertions(+)