[PATCH] nl80211: Zero num_modes if nl80211_get_hw_feature_data() fails

[Otcheretianski, Andrei]

> -----Original Message-----
> From: Jouni Malinen [mailto:j at w1.fi]
> Sent: Friday, December 30, 2016 23:45
> To: Otcheretianski, Andrei <andrei.otcheretianski at intel.com>
> Cc: hostap at lists.infradead.org
> Subject: Re: [PATCH] nl80211: Zero num_modes if
> nl80211_get_hw_feature_data() fails
> 
> On Wed, Dec 28, 2016 at 03:47:07PM +0200, Andrei Otcheretianski wrote:
> > It was possible that nl80211_get_hw_feature_data() function would
> > return NULL when num_modes is not set to zero. This might result in a
> > later crash when accessing hw.modes. This may be reproduced with hwsim
> > oom tests, for example, dbus_connect_oom.
> > Fix that by zeroing num_modes if NULL is returned.
> 
> I haven't been able to reproduce this.. Would you be able to identify the
> caller that does not check the returned pointer? There should be no places
> where *num_modes is used if NULL is returned..

Here is my log from dbus_connect_oom:

1483359087.370642: WPA_TRACE: eloop SIGSEGV - START
1483359087.375423: [1]: /home/tester/maintain/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant() [0x44c56e]
1483359087.375895:      eloop_sigsegv_handler() ../src/utils/eloop.c:123
1483359087.389225: [2]: /lib/x86_64-linux-gnu/libc.so.6(+0x36d40) [0x7efeb2282d40]
1483359087.389904: [3]: /home/tester/maintain/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(get_mode+0x5) [0x586925]
1483359087.390466:      get_mode() wpa_supplicant.c:6858
1483359087.391223: [4]: /home/tester/maintain/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(wpas_supp_op_class_ie+0xe5) [0x448ed5]
1483359087.391750:      wpas_op_class_supported() op_classes.c:203
1483359087.392341:      wpas_supp_op_class_ie() op_classes.c:291
1483359087.393186: [5]: /home/tester/maintain/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant() [0x569ed3]
1483359087.393553:      sme_send_authentication() sme.c:445
1483359087.394147: [6]: /home/tester/maintain/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant() [0x57f9f3]
1483359087.394583:      radio_start_next_work() wpa_supplicant.c:4462
1483359087.395122: [7]: /home/tester/maintain/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(eloop_run+0x1bb) [0x44d4cb]
1483359087.395470:      eloop_run() ../src/utils/eloop.c:1196
1483359087.396030: [8]: /home/tester/maintain/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(wpa_supplicant_run+0x77) [0x580ec7]
1483359087.396502:      wpa_supplicant_run() wpa_supplicant.c:5585
1483359087.396891: [9]: /home/tester/maintain/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(main+0x3a8) [0x43b7b8]
1483359087.397441:      main() main.c:392
1483359087.397824: WPA_TRACE: eloop SIGSEGV - END
> 
> > diff --git a/src/drivers/driver_nl80211_capa.c
> > b/src/drivers/driver_nl80211_capa.c
> > @@ -1771,6 +1771,7 @@ nl80211_get_hw_feature_data(void *priv, u16
> *num_modes, u16 *flags)
> >  				os_free(result.modes[i].rates);
> >  			}
> >  			os_free(result.modes);
> > +			*num_modes = 0;
> >  			return NULL;
> >  		}
> >  		return
> wpa_driver_nl80211_postprocess_modes(result.modes,
> 
> This does not look like a complete fix since the function can return NULL also
> if processing of NL80211_CMD_GET_WIPHY response fails. I'd assume this
> could potentially happen after having already incremented *num_modes.

I don't see how it can happen without having result.failed == 1.

> In any case, if this can really be hit with the current hostap.git snapshot, more
> appropriate fix would be to modify the caller that uses *num_modes if NULL
> is returned from get_hw_feature_data().

IMHO get_hw_feature_data() must be consistent and should set num_modes = 0 when it returns NULL,
at least this is what can be understood from it's documentation "@num_modes: Variable for returning the number of returned modes"

> 
> --
> Jouni Malinen                                            PGP id EFC895FA