From: Johannes Berg <johannes@sipsolutions.net> To: Ajay.Kathat@microchip.com, linux-wireless@vger.kernel.org Cc: devel@driverdev.osuosl.org, gregkh@linuxfoundation.org, Nicolas.Ferre@microchip.com, Claudiu.Beznea@microchip.com, Sripad.Balwadgi@microchip.com, Venkateswara.Kaja@microchip.com Subject: Re: [PATCH v7 06/17] wilc1000: add cfg80211.c Date: Wed, 24 Jun 2020 11:00:52 +0200 [thread overview] Message-ID: <11b8d5a5631cc472fce17638ff9e1dfef8750628.camel@sipsolutions.net> (raw) In-Reply-To: <20200623110000.31559-7-ajay.kathat@microchip.com> On Tue, 2020-06-23 at 11:00 +0000, Ajay.Kathat@microchip.com 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
WARNING: multiple messages have this Message-ID (diff)
From: Johannes Berg <johannes@sipsolutions.net> To: Ajay.Kathat@microchip.com, linux-wireless@vger.kernel.org Cc: devel@driverdev.osuosl.org, Venkateswara.Kaja@microchip.com, gregkh@linuxfoundation.org, Nicolas.Ferre@microchip.com, Sripad.Balwadgi@microchip.com Subject: Re: [PATCH v7 06/17] wilc1000: add cfg80211.c Date: Wed, 24 Jun 2020 11:00:52 +0200 [thread overview] Message-ID: <11b8d5a5631cc472fce17638ff9e1dfef8750628.camel@sipsolutions.net> (raw) In-Reply-To: <20200623110000.31559-7-ajay.kathat@microchip.com> On Tue, 2020-06-23 at 11:00 +0000, Ajay.Kathat@microchip.com 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 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
next prev parent reply other threads:[~2020-06-24 9:01 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-23 11:00 [PATCH v7 00/17] wilc1000: move out of staging Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 01/17] wilc1000: add hif.h Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 02/17] wilc1000: add hif.c Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 03/17] wilc1000: add wlan_if.h Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 04/17] wilc1000: add wlan_cfg.h Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 05/17] wilc1000: add wlan_cfg.c Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 06/17] wilc1000: add cfg80211.c Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-24 9:00 ` Johannes Berg [this message] 2020-06-24 9:00 ` Johannes Berg 2020-06-23 11:00 ` [PATCH v7 07/17] wilc1000: add cfg80211.h Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 08/17] wilc1000: add netdev.h Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 09/17] wilc1000: add netdev.c Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 10/17] wilc1000: add mon.c Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 12/17] wilc1000: add wlan.h Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 11/17] wilc1000: add spi.c Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 13/17] wilc1000: add wlan.c Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 15/17] wilc1000: add fw.h Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 14/17] wilc1000: add sdio.c Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 17/17] wilc1000: add Makefile and Kconfig files for wilc1000 compilation Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-23 14:52 ` kernel test robot 2020-06-23 14:52 ` kernel test robot 2020-06-23 14:52 ` kernel test robot 2020-06-24 5:26 ` Ajay.Kathat 2020-06-24 5:26 ` Ajay.Kathat 2020-06-24 5:26 ` Ajay.Kathat 2020-06-23 11:00 ` [PATCH v7 16/17] dt: bindings: net: add microchip,wilc1000.yaml Ajay.Kathat 2020-06-23 11:00 ` Ajay.Kathat 2020-06-24 8:50 ` [PATCH v7 00/17] wilc1000: move out of staging Kalle Valo 2020-06-24 8:50 ` Kalle Valo 2020-06-24 9:10 ` Greg KH 2020-06-24 9:10 ` Greg KH 2020-06-24 9:49 ` Kalle Valo 2020-06-24 9:49 ` Kalle Valo 2020-06-24 14:52 ` Greg KH 2020-06-24 14:52 ` Greg KH 2020-06-26 5:34 ` Kalle Valo 2020-06-26 5:34 ` Kalle Valo 2020-06-26 13:46 ` Greg KH 2020-06-26 13:46 ` Greg KH 2020-07-02 7:05 ` Kalle Valo 2020-07-02 7:05 ` Kalle Valo 2020-06-29 13:26 ` Pali Rohár 2020-06-29 13:26 ` Pali Rohár 2020-06-30 3:17 ` Ajay.Kathat 2020-06-30 3:17 ` Ajay.Kathat 2020-07-01 7:55 ` Pali Rohár 2020-07-01 7:55 ` Pali Rohár 2020-07-01 7:56 ` Pali Rohár 2020-07-01 7:56 ` Pali Rohár 2020-07-06 8:12 ` Ulf Hansson 2020-07-06 8:12 ` Ulf Hansson
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=11b8d5a5631cc472fce17638ff9e1dfef8750628.camel@sipsolutions.net \ --to=johannes@sipsolutions.net \ --cc=Ajay.Kathat@microchip.com \ --cc=Claudiu.Beznea@microchip.com \ --cc=Nicolas.Ferre@microchip.com \ --cc=Sripad.Balwadgi@microchip.com \ --cc=Venkateswara.Kaja@microchip.com \ --cc=devel@driverdev.osuosl.org \ --cc=gregkh@linuxfoundation.org \ --cc=linux-wireless@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.