os layer qsort

[Jouni Malinen]

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