SIGSEGV in Supplicant

[Jouni Malinen]

On Mon, Jun 01, 2015 at 10:44:38AM +0000, Peer, Ilan wrote:
> It is possible that in this flow, there was a pending query that was not started yet, so when gas_query_deinit() flow was executed and gas_query_free() was called, the 'query' object was freed, but without having the radio work removed. Thus, later, when all the radio works of the interface were removed, gas_query_start_cb() is called, which in turn called gas_query_free() which tried to access query->list which resulted with a segfault.
> 
> Can you please check if the attached patch fixes things? (I did not test it ...)

This is not really the cleanest way of handling this, but I could not
find any better way to do it without significant extra complexity which
is not really justifiable on the interface cleanup path.. The issue here
is in gas_query_free() not being able to remove the pending radio work
that has not yet been started since it does not have a pointer to that
radio work. Clearing all gas-query radio works just before calling
gas_query_deinit() is a simple way of avoiding this and that's fine for
wpa_supplicant_cleanup() to do, so I applied this. However, this needed
a small change to avoid a NULL pointer dereference if the interface in
question had not completed initialization (wpa_s->radio could be NULL
here). I added that and a comment explaining why this is here.

The fix is here:
http://w1.fi/cgit/hostap/commit/?id=57e832de37ea0a82e650d8230457e0868a01b72e

And these hwsim test cases reproduce the issue:
http://w1.fi/cgit/hostap/commit/?id=c518fecc826a5c7101ed6c7708eb618cf2ffbfe2

ap_hs20_connect_deinit should be practically identical to the problem
reported in this thread. gas_query_deinit is a bit simpler test case
which is also hitting the same issue, but through a bit different code
path.

-- 
Jouni Malinen                                            PGP id EFC895FA