Re: [PATCH v3 4/4] iwlwifi: mvm: add vendor commands needed for iwlmei

"Grumbach, Emmanuel" [off-list ref] writes:

quoted

quoted

On Thu, Jun 24, 2021 at 8:13 PM Kalle Valo [off-list ref] wrote:

quoted

Emmanuel Grumbach [off-list ref] writes:

quoted

Add the vendor commands that must be used by the network manager

to

quoted

quoted

quoted

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 <redacted>
Signed-off-by: Emmanuel Grumbach

[off-list ref]

quoted

quoted

quoted

---
 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

++++++++++++++++++

quoted

quoted

quoted

 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

quoted

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