Add the vendor commands that must be used by the network manager
to allow proper operation of iwlmei.
* Send information on the AP CSME is connected to
* Notify the userspace when roaming is forbidden
* Allow the userspace to require ownership
Co-Developed-by: Ayala Beker <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
drivers/net/wireless/intel/iwlwifi/Kconfig | 11 ++
.../net/wireless/intel/iwlwifi/mvm/Makefile | 1 +
.../net/wireless/intel/iwlwifi/mvm/mac80211.c | 2 +
drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 9 +-
.../wireless/intel/iwlwifi/mvm/vendor-cmd.c | 186 ++++++++++++++++++
5 files changed, 203 insertions(+), 6 deletions(-)
create mode 100644 drivers/net/wireless/intel/iwlwifi/mvm/vendor-cmd.c
diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig b/drivers/net/wireless/intel/iwlwifi/Kconfig
index 629aaa26a230..f91516d08b28 100644
--- a/drivers/net/wireless/intel/iwlwifi/Kconfig
+++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
@@ -92,11 +92,22 @@ config IWLWIFI_BCAST_FILTERING
If unsure, don't enable this option, as some programs might
expect incoming broadcasts for their normal operations.
+config IWLMVM_VENDOR_CMDS
+ bool "Enable vendor commands"
+ depends on IWLMVM
+ help
+ This option enables support for vendor commands, including some
+ that don't have their own Kconfig option. Other Kconfig options
+ depend on this one as well.
+
+ This is not enabled by default, if unsure, say N.
+
config IWLMEI
tristate "Intel Management Engine communication over WLAN"
depends on INTEL_MEI
depends on PM
depends on IWLMVM
+ select IWLMVM_VENDOR_CMDS
help
Enables the iwlmei kernel module. This allows to communicate with
the Intel Management Engine over Wifi. This is supported starting
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/Makefile b/drivers/net/wireless/intel/iwlwifi/mvm/Makefile
index 75fc2d935e5d..02078069d33d 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/Makefile
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/Makefile
@@ -10,5 +10,6 @@ iwlmvm-y += rfi.o
iwlmvm-$(CONFIG_IWLWIFI_DEBUGFS) += debugfs.o debugfs-vif.o
iwlmvm-$(CONFIG_IWLWIFI_LEDS) += led.o
iwlmvm-$(CONFIG_PM) += d3.o
+iwlmvm-$(CONFIG_IWLMVM_VENDOR_CMDS) += vendor-cmd.o
ccflags-y += -I $(srctree)/$(src)/../
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index ec6916ba4982..80ab14cacc3c 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -719,6 +719,8 @@ int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm)
wiphy_ext_feature_set(hw->wiphy,
NL80211_EXT_FEATURE_PROTECTED_TWT);
+ iwl_mvm_vendor_cmds_register(mvm);
+
hw->wiphy->available_antennas_tx = iwl_mvm_get_valid_tx_ant(mvm);
hw->wiphy->available_antennas_rx = iwl_mvm_get_valid_rx_ant(mvm);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index 322e9dce7bb0..505cdfb4fcbd 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -2212,11 +2212,8 @@ static inline void iwl_mvm_mei_set_sw_rfkill_state(struct iwl_mvm *mvm, bool sw_
iwl_mei_set_rfkill_state(iwl_mvm_is_radio_killed(mvm), sw_rfkill);
}
-static inline void iwl_mvm_send_roaming_forbidden_event(struct iwl_mvm *mvm,
- struct ieee80211_vif *vif,
- bool forbidden)
-{
- /* TODO */
-}
+void iwl_mvm_send_roaming_forbidden_event(struct iwl_mvm *mvm,
+ struct ieee80211_vif *vif,
+ bool forbidden);
#endif /* __IWL_MVM_H__ */
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/vendor-cmd.c b/drivers/net/wireless/intel/iwlwifi/mvm/vendor-cmd.c
new file mode 100644
index 000000000000..97e8885f1483
--- /dev/null
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/vendor-cmd.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2012-2014, 2018-2021 Intel Corporation
+ * Copyright (C) 2013-2015 Intel Mobile Communications GmbH
+ * Copyright (C) 2016-2017 Intel Deutschland GmbH
+ */
+#include "mvm.h"
+#include <linux/nl80211-vnd-intel.h>
+
+static const struct nla_policy
+iwl_mvm_vendor_attr_policy[NUM_IWL_MVM_VENDOR_ATTR] = {
+ [IWL_MVM_VENDOR_ATTR_ROAMING_FORBIDDEN] = { .type = NLA_U8 },
+ [IWL_MVM_VENDOR_ATTR_AUTH_MODE] = { .type = NLA_U32 },
+ [IWL_MVM_VENDOR_ATTR_CHANNEL_NUM] = { .type = NLA_U8 },
+ [IWL_MVM_VENDOR_ATTR_SSID] = { .type = NLA_BINARY,
+ .len = IEEE80211_MAX_SSID_LEN },
+ [IWL_MVM_VENDOR_ATTR_BAND] = { .type = NLA_U8 },
+ [IWL_MVM_VENDOR_ATTR_COLLOC_CHANNEL] = { .type = NLA_U8 },
+ [IWL_MVM_VENDOR_ATTR_COLLOC_ADDR] = { .type = NLA_BINARY, .len = ETH_ALEN },
+};
+
+__maybe_unused static struct nlattr **
+iwl_mvm_parse_vendor_data(const void *data, int data_len)
+{
+ struct nlattr **tb;
+ int err;
+
+ if (!data)
+ return ERR_PTR(-EINVAL);
+
+ tb = kcalloc(MAX_IWL_MVM_VENDOR_ATTR + 1, sizeof(*tb), GFP_KERNEL);
+ if (!tb)
+ return ERR_PTR(-ENOMEM);
+
+ err = nla_parse(tb, MAX_IWL_MVM_VENDOR_ATTR, data, data_len,
+ iwl_mvm_vendor_attr_policy, NULL);
+ if (err) {
+ kfree(tb);
+ return ERR_PTR(err);
+ }
+
+ return tb;
+}
+
+#if IS_ENABLED(CONFIG_IWLMEI)
+static int iwl_mvm_vendor_get_csme_conn_info(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ const void *data, int data_len)
+{
+ struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
+ struct iwl_mvm *mvm = IWL_MAC80211_GET_MVM(hw);
+ struct iwl_mvm_csme_conn_info *csme_conn_info;
+ struct sk_buff *skb;
+ int err = 0;
+
+ mutex_lock(&mvm->mutex);
+ csme_conn_info = iwl_mvm_get_csme_conn_info(mvm);
+
+ if (!csme_conn_info) {
+ err = -EINVAL;
+ goto out_unlock;
+ }
+
+ skb = cfg80211_vendor_cmd_alloc_reply_skb(wiphy, 200);
+ if (!skb) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ if (nla_put_u32(skb, IWL_MVM_VENDOR_ATTR_AUTH_MODE,
+ csme_conn_info->conn_info.auth_mode) ||
+ nla_put(skb, IWL_MVM_VENDOR_ATTR_SSID,
+ csme_conn_info->conn_info.ssid_len,
+ csme_conn_info->conn_info.ssid) ||
+ nla_put_u32(skb, IWL_MVM_VENDOR_ATTR_STA_CIPHER,
+ csme_conn_info->conn_info.pairwise_cipher) ||
+ nla_put_u8(skb, IWL_MVM_VENDOR_ATTR_CHANNEL_NUM,
+ csme_conn_info->conn_info.channel) ||
+ nla_put(skb, IWL_MVM_VENDOR_ATTR_ADDR, ETH_ALEN,
+ csme_conn_info->conn_info.bssid)) {
+ kfree_skb(skb);
+ err = -ENOBUFS;
+ }
+
+out_unlock:
+ mutex_unlock(&mvm->mutex);
+ if (err)
+ return err;
+
+ return cfg80211_vendor_cmd_reply(skb);
+}
+
+static int iwl_mvm_vendor_host_get_ownership(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ const void *data, int data_len)
+{
+ struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
+ struct iwl_mvm *mvm = IWL_MAC80211_GET_MVM(hw);
+
+ mutex_lock(&mvm->mutex);
+ iwl_mvm_mei_get_ownership(mvm);
+ mutex_unlock(&mvm->mutex);
+
+ return 0;
+}
+#endif
+
+static const struct wiphy_vendor_command iwl_mvm_vendor_commands[] = {
+#if IS_ENABLED(CONFIG_IWLMEI)
+ {
+ .info = {
+ .vendor_id = INTEL_OUI,
+ .subcmd = IWL_MVM_VENDOR_CMD_GET_CSME_CONN_INFO,
+ },
+ .doit = iwl_mvm_vendor_get_csme_conn_info,
+ .flags = WIPHY_VENDOR_CMD_NEED_WDEV,
+ .policy = iwl_mvm_vendor_attr_policy,
+ .maxattr = MAX_IWL_MVM_VENDOR_ATTR,
+ },
+ {
+ .info = {
+ .vendor_id = INTEL_OUI,
+ .subcmd = IWL_MVM_VENDOR_CMD_HOST_GET_OWNERSHIP,
+ },
+ .doit = iwl_mvm_vendor_host_get_ownership,
+ .flags = WIPHY_VENDOR_CMD_NEED_WDEV,
+ .policy = iwl_mvm_vendor_attr_policy,
+ .maxattr = MAX_IWL_MVM_VENDOR_ATTR,
+ },
+#endif
+};
+
+enum iwl_mvm_vendor_events_idx {
+ /* 0x0 - 0x3 are deprecated */
+#if IS_ENABLED(CONFIG_IWLMEI)
+ IWL_MVM_VENDOR_EVENT_IDX_ROAMING_FORBIDDEN = 4,
+#endif
+ NUM_IWL_MVM_VENDOR_EVENT_IDX
+};
+
+static const struct nl80211_vendor_cmd_info
+iwl_mvm_vendor_events[NUM_IWL_MVM_VENDOR_EVENT_IDX] = {
+#if IS_ENABLED(CONFIG_IWLMEI)
+ [IWL_MVM_VENDOR_EVENT_IDX_ROAMING_FORBIDDEN] = {
+ .vendor_id = INTEL_OUI,
+ .subcmd = IWL_MVM_VENDOR_CMD_ROAMING_FORBIDDEN_EVENT,
+ },
+#endif
+};
+
+void iwl_mvm_vendor_cmds_register(struct iwl_mvm *mvm)
+{
+ mvm->hw->wiphy->vendor_commands = iwl_mvm_vendor_commands;
+ mvm->hw->wiphy->n_vendor_commands = ARRAY_SIZE(iwl_mvm_vendor_commands);
+ mvm->hw->wiphy->vendor_events = iwl_mvm_vendor_events;
+ mvm->hw->wiphy->n_vendor_events = ARRAY_SIZE(iwl_mvm_vendor_events);
+}
+
+#if IS_ENABLED(CONFIG_IWLMEI)
+void iwl_mvm_send_roaming_forbidden_event(struct iwl_mvm *mvm,
+ struct ieee80211_vif *vif,
+ bool forbidden)
+{
+ struct sk_buff *msg =
+ cfg80211_vendor_event_alloc(mvm->hw->wiphy,
+ ieee80211_vif_to_wdev(vif),
+ 200, IWL_MVM_VENDOR_EVENT_IDX_ROAMING_FORBIDDEN,
+ GFP_ATOMIC);
+ if (!msg)
+ return;
+
+ if (WARN_ON(!vif))
+ return;
+
+ if (nla_put(msg, IWL_MVM_VENDOR_ATTR_VIF_ADDR,
+ ETH_ALEN, vif->addr) ||
+ nla_put_u8(msg, IWL_MVM_VENDOR_ATTR_ROAMING_FORBIDDEN, forbidden))
+ goto nla_put_failure;
+
+ cfg80211_vendor_event(msg, GFP_ATOMIC);
+ return;
+
+ nla_put_failure:
+ kfree_skb(msg);
+}
+#endif
--
2.25.1
Emmanuel Grumbach <[email protected]> writes:
> Add the vendor commands that must be used by the network manager
> to allow proper operation of iwlmei.
>
> * Send information on the AP CSME is connected to
> * Notify the userspace when roaming is forbidden
> * Allow the userspace to require ownership
>
> Co-Developed-by: Ayala Beker <[email protected]>
> Signed-off-by: Emmanuel Grumbach <[email protected]>
> ---
> drivers/net/wireless/intel/iwlwifi/Kconfig | 11 ++
> .../net/wireless/intel/iwlwifi/mvm/Makefile | 1 +
> .../net/wireless/intel/iwlwifi/mvm/mac80211.c | 2 +
> drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 9 +-
> .../wireless/intel/iwlwifi/mvm/vendor-cmd.c | 186 ++++++++++++++++++
> 5 files changed, 203 insertions(+), 6 deletions(-)
> create mode 100644 drivers/net/wireless/intel/iwlwifi/mvm/vendor-cmd.c
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig b/drivers/net/wireless/intel/iwlwifi/Kconfig
> index 629aaa26a230..f91516d08b28 100644
> --- a/drivers/net/wireless/intel/iwlwifi/Kconfig
> +++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
> @@ -92,11 +92,22 @@ config IWLWIFI_BCAST_FILTERING
> If unsure, don't enable this option, as some programs might
> expect incoming broadcasts for their normal operations.
>
> +config IWLMVM_VENDOR_CMDS
> + bool "Enable vendor commands"
> + depends on IWLMVM
> + help
> + This option enables support for vendor commands, including some
> + that don't have their own Kconfig option. Other Kconfig options
> + depend on this one as well.
> +
> + This is not enabled by default, if unsure, say N.
Why do we need a new Kconfig option? Why not always include it in the
compilation?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Thu, Jun 24, 2021 at 8:13 PM Kalle Valo <[email protected]> wrote:
>
> Emmanuel Grumbach <[email protected]> writes:
>
> > Add the vendor commands that must be used by the network manager
> > to allow proper operation of iwlmei.
> >
> > * Send information on the AP CSME is connected to
> > * Notify the userspace when roaming is forbidden
> > * Allow the userspace to require ownership
> >
> > Co-Developed-by: Ayala Beker <[email protected]>
> > Signed-off-by: Emmanuel Grumbach <[email protected]>
> > ---
> > drivers/net/wireless/intel/iwlwifi/Kconfig | 11 ++
> > .../net/wireless/intel/iwlwifi/mvm/Makefile | 1 +
> > .../net/wireless/intel/iwlwifi/mvm/mac80211.c | 2 +
> > drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 9 +-
> > .../wireless/intel/iwlwifi/mvm/vendor-cmd.c | 186 ++++++++++++++++++
> > 5 files changed, 203 insertions(+), 6 deletions(-)
> > create mode 100644 drivers/net/wireless/intel/iwlwifi/mvm/vendor-cmd.c
> >
> > diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig b/drivers/net/wireless/intel/iwlwifi/Kconfig
> > index 629aaa26a230..f91516d08b28 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/Kconfig
> > +++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
> > @@ -92,11 +92,22 @@ config IWLWIFI_BCAST_FILTERING
> > If unsure, don't enable this option, as some programs might
> > expect incoming broadcasts for their normal operations.
> >
> > +config IWLMVM_VENDOR_CMDS
> > + bool "Enable vendor commands"
> > + depends on IWLMVM
> > + help
> > + This option enables support for vendor commands, including some
> > + that don't have their own Kconfig option. Other Kconfig options
> > + depend on this one as well.
> > +
> > + This is not enabled by default, if unsure, say N.
>
> Why do we need a new Kconfig option? Why not always include it in the
> compilation?
I expect 99.9% of the users to want to disable this.VENDOR_CMDS adds a
user space API and in a sense, it increases the attack surface. You
can claim that I can reuse the IWLMEI Kconfig option, which is true,
but we have other features that need VENDOR_CMDS that are not (yet)
upstream. So the idea here is that any feature that needs the
VENDOR_CMDS will select it and if none of them are enabled (for 99.9%
of the use cases), then, we would disable VENDOR_CMDS and decrease the
attack surface.
Makes sense?
Emmanuel Grumbach <[email protected]> writes:
> On Thu, Jun 24, 2021 at 8:13 PM Kalle Valo <[email protected]> wrote:
>>
>> Emmanuel Grumbach <[email protected]> writes:
>>
>> > Add the vendor commands that must be used by the network manager
>> > to allow proper operation of iwlmei.
>> >
>> > * Send information on the AP CSME is connected to
>> > * Notify the userspace when roaming is forbidden
>> > * Allow the userspace to require ownership
>> >
>> > Co-Developed-by: Ayala Beker <[email protected]>
>> > Signed-off-by: Emmanuel Grumbach <[email protected]>
>> > ---
>> > drivers/net/wireless/intel/iwlwifi/Kconfig | 11 ++
>> > .../net/wireless/intel/iwlwifi/mvm/Makefile | 1 +
>> > .../net/wireless/intel/iwlwifi/mvm/mac80211.c | 2 +
>> > drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 9 +-
>> > .../wireless/intel/iwlwifi/mvm/vendor-cmd.c | 186 ++++++++++++++++++
>> > 5 files changed, 203 insertions(+), 6 deletions(-)
>> > create mode 100644 drivers/net/wireless/intel/iwlwifi/mvm/vendor-cmd.c
>> >
>> > diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig b/drivers/net/wireless/intel/iwlwifi/Kconfig
>> > index 629aaa26a230..f91516d08b28 100644
>> > --- a/drivers/net/wireless/intel/iwlwifi/Kconfig
>> > +++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
>> > @@ -92,11 +92,22 @@ config IWLWIFI_BCAST_FILTERING
>> > If unsure, don't enable this option, as some programs might
>> > expect incoming broadcasts for their normal operations.
>> >
>> > +config IWLMVM_VENDOR_CMDS
>> > + bool "Enable vendor commands"
>> > + depends on IWLMVM
>> > + help
>> > + This option enables support for vendor commands, including some
>> > + that don't have their own Kconfig option. Other Kconfig options
>> > + depend on this one as well.
>> > +
>> > + This is not enabled by default, if unsure, say N.
>>
>> Why do we need a new Kconfig option? Why not always include it in the
>> compilation?
>
> I expect 99.9% of the users to want to disable this.VENDOR_CMDS adds a
> user space API and in a sense, it increases the attack surface. You
> can claim that I can reuse the IWLMEI Kconfig option, which is true,
> but we have other features that need VENDOR_CMDS that are not (yet)
> upstream. So the idea here is that any feature that needs the
> VENDOR_CMDS will select it and if none of them are enabled (for 99.9%
> of the use cases), then, we would disable VENDOR_CMDS and decrease the
> attack surface.
>
> Makes sense?
How do you prevent users or distros from enabling the feature? They can
be in a hurry, lazy or not caring and enable the feature anyway. So no,
I'm not really buying this. If the interface is not secure it should not
be in upstream, I think only exception to this is the nl80211 testmode
interface which is for lab or similar use.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> > On Thu, Jun 24, 2021 at 8:13 PM Kalle Valo <[email protected]> wrote:
> >>
> >> Emmanuel Grumbach <[email protected]> writes:
> >>
> >> > Add the vendor commands that must be used by the network manager
> to
> >> > allow proper operation of iwlmei.
> >> >
> >> > * Send information on the AP CSME is connected to
> >> > * Notify the userspace when roaming is forbidden
> >> > * Allow the userspace to require ownership
> >> >
> >> > Co-Developed-by: Ayala Beker <[email protected]>
> >> > Signed-off-by: Emmanuel Grumbach
> <[email protected]>
> >> > ---
> >> > drivers/net/wireless/intel/iwlwifi/Kconfig | 11 ++
> >> > .../net/wireless/intel/iwlwifi/mvm/Makefile | 1 +
> >> > .../net/wireless/intel/iwlwifi/mvm/mac80211.c | 2 +
> >> > drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 9 +-
> >> > .../wireless/intel/iwlwifi/mvm/vendor-cmd.c | 186
> ++++++++++++++++++
> >> > 5 files changed, 203 insertions(+), 6 deletions(-) create mode
> >> > 100644 drivers/net/wireless/intel/iwlwifi/mvm/vendor-cmd.c
> >> >
> >> > diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig
> >> > b/drivers/net/wireless/intel/iwlwifi/Kconfig
> >> > index 629aaa26a230..f91516d08b28 100644
> >> > --- a/drivers/net/wireless/intel/iwlwifi/Kconfig
> >> > +++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
> >> > @@ -92,11 +92,22 @@ config IWLWIFI_BCAST_FILTERING
> >> > If unsure, don't enable this option, as some programs might
> >> > expect incoming broadcasts for their normal operations.
> >> >
> >> > +config IWLMVM_VENDOR_CMDS
> >> > + bool "Enable vendor commands"
> >> > + depends on IWLMVM
> >> > + help
> >> > + This option enables support for vendor commands, including some
> >> > + that don't have their own Kconfig option. Other Kconfig options
> >> > + depend on this one as well.
> >> > +
> >> > + This is not enabled by default, if unsure, say N.
> >>
> >> Why do we need a new Kconfig option? Why not always include it in the
> >> compilation?
> >
> > I expect 99.9% of the users to want to disable this.VENDOR_CMDS adds a
> > user space API and in a sense, it increases the attack surface. You
> > can claim that I can reuse the IWLMEI Kconfig option, which is true,
> > but we have other features that need VENDOR_CMDS that are not (yet)
> > upstream. So the idea here is that any feature that needs the
> > VENDOR_CMDS will select it and if none of them are enabled (for 99.9%
> > of the use cases), then, we would disable VENDOR_CMDS and decrease
> the
> > attack surface.
> >
> > Makes sense?
>
> How do you prevent users or distros from enabling the feature? They can be
> in a hurry, lazy or not caring and enable the feature anyway. So no, I'm not
> really buying this. If the interface is not secure it should not be in upstream, I
> think only exception to this is the nl80211 testmode interface which is for lab
> or similar use.
>
So what do you want?
To make it depend on IWLMEI Kconfig knob and not add the VENDOR_CMDS one?
Fine.
"Grumbach, Emmanuel" <[email protected]> writes:
>
>> > On Thu, Jun 24, 2021 at 8:13 PM Kalle Valo <[email protected]> wrote:
>> >>
>> >> Emmanuel Grumbach <[email protected]> writes:
>> >>
>> >> > Add the vendor commands that must be used by the network manager
>> to
>> >> > allow proper operation of iwlmei.
>> >> >
>> >> > * Send information on the AP CSME is connected to
>> >> > * Notify the userspace when roaming is forbidden
>> >> > * Allow the userspace to require ownership
>> >> >
>> >> > Co-Developed-by: Ayala Beker <[email protected]>
>> >> > Signed-off-by: Emmanuel Grumbach
>> <[email protected]>
>> >> > ---
>> >> > drivers/net/wireless/intel/iwlwifi/Kconfig | 11 ++
>> >> > .../net/wireless/intel/iwlwifi/mvm/Makefile | 1 +
>> >> > .../net/wireless/intel/iwlwifi/mvm/mac80211.c | 2 +
>> >> > drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 9 +-
>> >> > .../wireless/intel/iwlwifi/mvm/vendor-cmd.c | 186
>> ++++++++++++++++++
>> >> > 5 files changed, 203 insertions(+), 6 deletions(-) create mode
>> >> > 100644 drivers/net/wireless/intel/iwlwifi/mvm/vendor-cmd.c
>> >> >
>> >> > diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig
>> >> > b/drivers/net/wireless/intel/iwlwifi/Kconfig
>> >> > index 629aaa26a230..f91516d08b28 100644
>> >> > --- a/drivers/net/wireless/intel/iwlwifi/Kconfig
>> >> > +++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
>> >> > @@ -92,11 +92,22 @@ config IWLWIFI_BCAST_FILTERING
>> >> > If unsure, don't enable this option, as some programs might
>> >> > expect incoming broadcasts for their normal operations.
>> >> >
>> >> > +config IWLMVM_VENDOR_CMDS
>> >> > + bool "Enable vendor commands"
>> >> > + depends on IWLMVM
>> >> > + help
>> >> > + This option enables support for vendor commands, including some
>> >> > + that don't have their own Kconfig option. Other Kconfig options
>> >> > + depend on this one as well.
>> >> > +
>> >> > + This is not enabled by default, if unsure, say N.
>> >>
>> >> Why do we need a new Kconfig option? Why not always include it in the
>> >> compilation?
>> >
>> > I expect 99.9% of the users to want to disable this.VENDOR_CMDS adds a
>> > user space API and in a sense, it increases the attack surface. You
>> > can claim that I can reuse the IWLMEI Kconfig option, which is true,
>> > but we have other features that need VENDOR_CMDS that are not (yet)
>> > upstream. So the idea here is that any feature that needs the
>> > VENDOR_CMDS will select it and if none of them are enabled (for 99.9%
>> > of the use cases), then, we would disable VENDOR_CMDS and decrease
>> the
>> > attack surface.
>> >
>> > Makes sense?
>>
>> How do you prevent users or distros from enabling the feature? They can be
>> in a hurry, lazy or not caring and enable the feature anyway. So no, I'm not
>> really buying this. If the interface is not secure it should not be in upstream, I
>> think only exception to this is the nl80211 testmode interface which is for lab
>> or similar use.
>>
>
> So what do you want? To make it depend on IWLMEI Kconfig knob and not
> add the VENDOR_CMDS one? Fine.
Yes, that sounds like a good idea. And I saw you did that already in v6.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches