os layer qsort
j at w1.fi
Sun Oct 9 01:17:21 PDT 2016
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?
> 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.
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
More information about the Hostap