On Oct 09, 2016, at 03:20 AM, Jouni Malinen <j@xxxxx> wrote: > On Sat, Oct 08, 2016 at 12:39:24PM -0500, Joel Cunningham wrote: >> I forgot to mention another detail, in the case of wpa_supplicant_get_scan_results, the base pointer into qsort is NULL in addition to num being 0. > > Are you sure the C library asserts on the array length 0 instead of that > NULL base pointer? You're correct, the abort happens because of the NULL base pointer. Sorry for not clearly communicating that to begin with. We ended up using a check against the size instead of base pointer to also skip the qsort function call when num is 1. > > >> This can be seen in the driver_nl80211.c (possibly other drivers too, I’m not sure) where nl80211_get_scan_results() calls os_zalloc() on struct wpa_scan_results and if no results are added to the res array in bss_info_handler, res->res and res->num will end up being NULL and 0. >> >> According to at least this reference, passing in a NULL pointer results in undefined behavior: http://www.cplusplus.com/reference/cstdlib/qsort/ > > I'd be more supportive of checking that base != NULL before the qsort() > calls.. In fact, that's already done in almost all the other locations.. > >> In our local port of WPA supplicant, we added a check if res->num > 1 before calling qsort in wpa_supplicant_get_scan_results() and I’m fine with this approach as well (attached is a patch). I was leaning towards the os layer abstraction because it seemed that was the standard approach for interacting with standard C lib calls and it wasn’t clear why qsort wasn’t already in that abstraction. Plus using the abstraction saves adding a conditional to each location that uses qsort to ensure no other NULL pointers are being passed. > > Only the calls in wpa_supplicant_get_scan_results() and > hostapd_config_read_maclist() seem to be able to pass in base == NULL to > qsort(), so it is not really that many locations that would need > changes.. I'd be fine with a patch adding such explicit checks if that > fixes the issue you are seeing. I have attached a patch containing NULL base pointer checks for these cases > > The os_*() wrappers are for cases where there is no consistent behavior > in C compilers/libraries. I'd rather not add more of them unless there > is clear justification for the need. > > -- > Jouni Malinen PGP id EFC895FA > > _______________________________________________ > Hostap mailing list > Hostap@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/hostap
Attachment:
0001-Check-for-NULL-qsort-base-pointers.patch
Description: Binary data
_______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
