On Mon, Jun 03, 2019 at 09:21:17PM +0200, John Crispin wrote: > The ppet field inside ieee80211_he_capabilities is of size [0]. The code > currently copies up to 12 additional bytes into the buffer, thus overwriting > memory. Fix this by verifying the size properly and using the passed length > value for allocation and the following memcpy() call. What is the relationship of this patch to "[V2] HE: fix hostapd_get_he_capab()"? http://patchwork.ozlabs.org/patch/1116968/ That patch proposes changes to the optional[] array in the end of struct ieee80211_he_capabilities to make it fixed length 37.. > +static inline u8 > +ieee80211_he_mcs_set_size(const u8 *phy_cap_info) Please no "inline" in helper functions. The compiler should be clever enough to handle this for static functions. > @@ -331,13 +364,13 @@ u16 copy_sta_he_capab(struct hostapd_data *hapd, struct sta_info *sta, > > if (!sta->he_capab) { > sta->he_capab = > - os_zalloc(sizeof(struct ieee80211_he_capabilities)); > + os_zalloc(he_capab_len); > if (!sta->he_capab) > return WLAN_STATUS_UNSPECIFIED_FAILURE; > } > > sta->flags |= WLAN_STA_HE; > - os_memset(sta->he_capab, 0, sizeof(struct ieee80211_he_capabilities)); > + os_memset(sta->he_capab, 0, he_capab_len); > os_memcpy(sta->he_capab, he_capab, he_capab_len); What guarantees that the allocated sta->he_capab buffer is large enough if it can now be of variable size and it is not reallocated in case sta->he_capab was non-NULL when getting here? That part would work with fixed size allocations, but this one does not seem safe without this function modified to check that there is sufficient room or alternatively, always allocate the maximum size or always allocate the buffer again here regardless of whether it was previously allocated. -- Jouni Malinen PGP id EFC895FA _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
