On Tue, 2020-06-23 at 11:00 +0000, Ajay.Kathat@xxxxxxxxxxxxx wrote:
>
> +struct wilc_p2p_pub_act_frame {
> + u8 category;
> + u8 action;
> + u8 oui[3];
> + u8 oui_type;
> + u8 oui_subtype;
> + u8 dialog_token;
> + u8 elem[];
> +} __packed;
> +
> +struct wilc_vendor_specific_ie {
> + u8 tag_number;
> + u8 tag_len;
> + u8 oui[3];
> + u8 oui_type;
> + u8 attr[];
> +} __packed;
> +
> +struct wilc_attr_entry {
> + u8 attr_type;
> + __le16 attr_len;
> + u8 val[];
> +} __packed;
> +
> +struct wilc_attr_oper_ch {
> + u8 attr_type;
> + __le16 attr_len;
> + u8 country_code[IEEE80211_COUNTRY_STRING_LEN];
> + u8 op_class;
> + u8 op_channel;
> +} __packed;
> +
> +struct wilc_attr_ch_list {
> + u8 attr_type;
> + __le16 attr_len;
> + u8 country_code[IEEE80211_COUNTRY_STRING_LEN];
> + u8 elem[];
> +} __packed;
> +
> +struct wilc_ch_list_elem {
> + u8 op_class;
> + u8 no_of_channels;
> + u8 ch_list[];
> +} __packed;
It seems like these should be used from ieee80211.h, and/or added there
if they don't already exist?
> +static int wilc_wfi_cfg_copy_wpa_info(struct wilc_wfi_key *key_info,
> + struct key_params *params)
> +{
> + kfree(key_info->key);
> +
> + key_info->key = kmemdup(params->key, params->key_len, GFP_KERNEL);
> + if (!key_info->key)
> + return -ENOMEM;
> +
> + kfree(key_info->seq);
> +
> + if (params->seq_len > 0) {
> + key_info->seq = kmemdup(params->seq, params->seq_len,
> + GFP_KERNEL);
> + if (!key_info->seq)
> + return -ENOMEM;
you may leak key_info->key here?
> +static inline void wilc_wfi_cfg_parse_ch_attr(u8 *buf, u32 len, u8 sta_ch)
That's a bit big to be forced inline, imho. if it's used only once the
compiler will inline it anyway.
> + d = (struct wilc_p2p_pub_act_frame *)(&mgmt->u.action);
> + if (d->oui_subtype != GO_NEG_REQ && d->oui_subtype != GO_NEG_RSP &&
> + d->oui_subtype != P2P_INV_REQ && d->oui_subtype != P2P_INV_RSP)
> + goto out_rx_mgmt;
> +
> + vendor_ie = cfg80211_find_vendor_ie(WLAN_OUI_WFA, WLAN_OUI_TYPE_WFA_P2P,
> + buff + ie_offset, size - ie_offset);
> + if (!vendor_ie)
> + goto out_rx_mgmt;
> +
> + p = (struct wilc_vendor_specific_ie *)vendor_ie;
> + wilc_wfi_cfg_parse_ch_attr(p->attr, p->tag_len - 4, vif->wilc->sta_ch);
but overall, why do you even need this? I don't think this is normally
handled in the driver, but wpa_s?
Anyway, I'm not convinced that we should really keep kicking this back
over minor issues like this ... better to merge it and fix later, imho.
johannes
