[Buildroot] [PATCH 0/2] package/wpa_supplicant: follow-up fixes for v2.11

[Buildroot] [PATCH 0/2] package/wpa_supplicant: follow-up fixes for v2.11

[Sergey Matyukevich]

Hi all,

Some issues have been discovered after wpa_supplicant update to v2.11:

- wired driver fails to build with kernel headers less than v5.7
  -- detected by Buildroot autobuilder
- wpa_supplicant fails to complete wpa2-psk connection on brcmfmac hardware

This patch series provides fixes for them:
- bump kernel headers restrictions for wired driver
- upstream patch revert to fix brcmfmac

Sergey Matyukevich (2):
  package/wpa_supplicant: wired driver needs headers >= 5.7
  package/wpa_supplicant: revert upstream patch to fix brcmfmac

 ...orization-completed-on-driver-indica.patch | 55 +++++++++++++++++++
 package/wpa_supplicant/Config.in              |  6 +-
 2 files changed, 58 insertions(+), 3 deletions(-)
 create mode 100644 package/wpa_supplicant/0001-Revert-Mark-authorization-completed-on-driver-indica.patch

-- 
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/2] package/wpa_supplicant: wired driver needs headers >= 5.7

[Sergey Matyukevich]

In wpa_supplicant v2.11 macsec support in wired driver requires enum
'macsec_offload' if libnl version is >= v3.6, see the commit:
- https://w1.fi/cgit/hostap/commit/?id=40c139664439b2576e1506fbca14a7b79425a9dd

Buildroot provides libnl version v3.9.0, so enum 'macsec_offload' shall
be available in order to successfully build wpa_supplicant wired driver
with macsec support. However that enum has only been added to Linux header
if_link.h since kernel v5.7, see commits:
- https://github.com/torvalds/linux/commit/21114b7feec29e4425a3ac48a037569c016a46c8
- https://github.com/torvalds/linux/commit/76564261a7db80c5f5c624e0122a28787f266bdf

That is why we have to tighten restriction on Linux headers for wired
driver from v4.6 to v5.7.

Fixes:
- http://autobuild.buildroot.net/results/b59d5bc5bd17683a3a1e3577c40c802e81911f84/

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 package/wpa_supplicant/Config.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/package/wpa_supplicant/Config.in b/package/wpa_supplicant/Config.in
index 92953f69f0..e013f5a9c1 100644
--- a/package/wpa_supplicant/Config.in
+++ b/package/wpa_supplicant/Config.in
@@ -42,7 +42,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_WEXT
 config BR2_PACKAGE_WPA_SUPPLICANT_WIRED
 	bool "Enable wired support"
 	depends on BR2_TOOLCHAIN_HAS_THREADS # libnl
-	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_6
+	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_7
 	select BR2_PACKAGE_LIBNL
 	select BR2_PACKAGE_WPA_SUPPLICANT_EAP
 	help
@@ -50,8 +50,8 @@ config BR2_PACKAGE_WPA_SUPPLICANT_WIRED
 	  supplicant can be used with Ethernet.  This also enables
 	  support for MACSEC.
 
-comment "wired macsec support needs a toolchain w/ headers >= 4.6"
-	depends on !BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_6
+comment "wired macsec support needs a toolchain w/ headers >= 5.7"
+	depends on !BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_7
 
 comment "wired support needs a toolchain w/ threads"
 	depends on !BR2_TOOLCHAIN_HAS_THREADS
-- 
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/2] package/wpa_supplicant: revert upstream commit to fix brcmfmac

[Sergey Matyukevich]

Revert the upstream wpa_supplicant commit 41638606054 ("Mark
authorization completed on driver indication during 4-way HS offload").
That commit breaks WPA2-PSK and WPA3-SAE for brcfmac driver in upstream
Linux kernels. For details refer to the hostapd mailing list discussion
and redhat bugzilla:
- http://lists.infradead.org/pipermail/hostap/2024-August/042893.html
- https://bugzilla.redhat.com/show_bug.cgi?id=2302577

Alternative option would be to disable offload in brcmfmac driver
adding "brcmfmac.feature_disable=0x82000" to Linux command line.

Final decision is not yet agreed upon on the hostapd mailing list,
but reporter and maintainer are inclined to revert the patch. So this
change adds revert to Buildroot to improve user experience for such
boards as Orange Pi Zero Plus2, BananaPro, and others. Later on the
patch will be updated according to hostapd upstream changes.

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 ...orization-completed-on-driver-indica.patch | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 package/wpa_supplicant/0001-Revert-Mark-authorization-completed-on-driver-indica.patch

diff --git a/package/wpa_supplicant/0001-Revert-Mark-authorization-completed-on-driver-indica.patch b/package/wpa_supplicant/0001-Revert-Mark-authorization-completed-on-driver-indica.patch
new file mode 100644
index 0000000000..89954eb7c5
--- /dev/null
+++ b/package/wpa_supplicant/0001-Revert-Mark-authorization-completed-on-driver-indica.patch
@@ -0,0 +1,55 @@
+From 170d8a433308ee39f87af94acaef461f8e80b67b Mon Sep 17 00:00:00 2001
+From: Janne Grunau <janne-fdr@jannau.net>
+Date: Sun, 4 Aug 2024 13:24:42 +0200
+Subject: [PATCH] Revert "Mark authorization completed on driver indication during 4-way HS offload"
+
+This reverts commit 41638606054a09867fe3f9a2b5523aa4678cbfa5.
+
+Signed-off-by: Janne Grunau <janne-fdr@jannau.net>
+Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
+
+Upstream: not yet sent, ongoing discussion http://lists.infradead.org/pipermail/hostap/2024-August/042893.html
+
+---
+ wpa_supplicant/events.c | 25 ++++++++-----------------
+ 1 file changed, 8 insertions(+), 17 deletions(-)
+
+diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
+index 49917f7aa..bbb3a3eda 100644
+--- a/wpa_supplicant/events.c
++++ b/wpa_supplicant/events.c
+@@ -4327,23 +4327,14 @@ static void wpa_supplicant_event_assoc(struct wpa_supplicant *wpa_s,
+ 		eapol_sm_notify_eap_success(wpa_s->eapol, true);
+ 	} else if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_PSK) &&
+ 		   wpa_key_mgmt_wpa_psk(wpa_s->key_mgmt)) {
+-		if (already_authorized) {
+-			/*
+-			 * We are done; the driver will take care of RSN 4-way
+-			 * handshake.
+-			 */
+-			wpa_supplicant_cancel_auth_timeout(wpa_s);
+-			wpa_supplicant_set_state(wpa_s, WPA_COMPLETED);
+-			eapol_sm_notify_portValid(wpa_s->eapol, true);
+-			eapol_sm_notify_eap_success(wpa_s->eapol, true);
+-		} else {
+-			/* Update port, WPA_COMPLETED state from the
+-			 * EVENT_PORT_AUTHORIZED handler when the driver is done
+-			 * with the 4-way handshake.
+-			 */
+-			wpa_msg(wpa_s, MSG_DEBUG,
+-				"ASSOC INFO: wait for driver port authorized indication");
+-		}
++		/*
++		 * We are done; the driver will take care of RSN 4-way
++		 * handshake.
++		 */
++		wpa_supplicant_cancel_auth_timeout(wpa_s);
++		wpa_supplicant_set_state(wpa_s, WPA_COMPLETED);
++		eapol_sm_notify_portValid(wpa_s->eapol, true);
++		eapol_sm_notify_eap_success(wpa_s->eapol, true);
+ 	} else if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_8021X) &&
+ 		   wpa_key_mgmt_wpa_ieee8021x(wpa_s->key_mgmt)) {
+ 		/*
+-- 
+2.45.2
+
-- 
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/2] package/wpa_supplicant: wired driver needs headers >= 5.7

[Thomas Petazzoni via buildroot]

Hello Sergey,

On Sun, 18 Aug 2024 23:31:29 +0300
Sergey Matyukevich <geomatsi@gmail.com> wrote:

> diff --git a/package/wpa_supplicant/Config.in b/package/wpa_supplicant/Config.in
> index 92953f69f0..e013f5a9c1 100644
> --- a/package/wpa_supplicant/Config.in
> +++ b/package/wpa_supplicant/Config.in
> @@ -42,7 +42,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_WEXT
>  config BR2_PACKAGE_WPA_SUPPLICANT_WIRED
>  	bool "Enable wired support"
>  	depends on BR2_TOOLCHAIN_HAS_THREADS # libnl
> -	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_6
> +	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_7

Thanks, but it seems a bit drastic.

If I looked at the code:

ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_WIRED),y)
WPA_SUPPLICANT_DEPENDENCIES += host-pkgconf libnl
WPA_SUPPLICANT_CONFIG_ENABLE += \
        CONFIG_LIBNL32 \
        CONFIG_DRIVER_WIRED \
        CONFIG_MACSEC \
        CONFIG_DRIVER_MACSEC_LINUX

so it forces CONFIG_DRIVER_MACSEC_LINUX, what about making that
dependent on kernel headers >= 5.7, rather than preventing the whole
wired driver from being compiled?

Or, in fact even better, in ./src/drivers/driver_macsec_linux.c, change:

#if LIBNL_VER_NUM >= LIBNL_VER(3, 6)
#define LIBNL_HAS_OFFLOAD
#endif

to:

#if LIBNL_VER_NUM >= LIBNL_VER(3, 6) && LINUX_VERSION_CODE >= KERNEL_VERSION(5,7,0)
#define LIBNL_HAS_OFFLOAD
#endif

And that should only enable the offload support... when it can really
be supported. Could you check this?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 2/2] package/wpa_supplicant: revert upstream commit to fix brcmfmac

[Thomas Petazzoni via buildroot]

On Sun, 18 Aug 2024 23:31:30 +0300
Sergey Matyukevich <geomatsi@gmail.com> wrote:

> Revert the upstream wpa_supplicant commit 41638606054 ("Mark
> authorization completed on driver indication during 4-way HS offload").
> That commit breaks WPA2-PSK and WPA3-SAE for brcfmac driver in upstream
> Linux kernels. For details refer to the hostapd mailing list discussion
> and redhat bugzilla:
> - http://lists.infradead.org/pipermail/hostap/2024-August/042893.html
> - https://bugzilla.redhat.com/show_bug.cgi?id=2302577
> 
> Alternative option would be to disable offload in brcmfmac driver
> adding "brcmfmac.feature_disable=0x82000" to Linux command line.
> 
> Final decision is not yet agreed upon on the hostapd mailing list,
> but reporter and maintainer are inclined to revert the patch. So this
> change adds revert to Buildroot to improve user experience for such
> boards as Orange Pi Zero Plus2, BananaPro, and others. Later on the
> patch will be updated according to hostapd upstream changes.
> 
> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> ---
>  ...orization-completed-on-driver-indica.patch | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 package/wpa_supplicant/0001-Revert-Mark-authorization-completed-on-driver-indica.patch

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/2] package/wpa_supplicant: wired driver needs headers >= 5.7

[Sergey Matyukevich]

Hello Thomas,

On Mon, Aug 19, 2024 at 12:59:36PM +0200, Thomas Petazzoni wrote:
> Hello Sergey,
> 
> On Sun, 18 Aug 2024 23:31:29 +0300
> Sergey Matyukevich <geomatsi@gmail.com> wrote:
> 
> > diff --git a/package/wpa_supplicant/Config.in b/package/wpa_supplicant/Config.in
> > index 92953f69f0..e013f5a9c1 100644
> > --- a/package/wpa_supplicant/Config.in
> > +++ b/package/wpa_supplicant/Config.in
> > @@ -42,7 +42,7 @@ config BR2_PACKAGE_WPA_SUPPLICANT_WEXT
> >  config BR2_PACKAGE_WPA_SUPPLICANT_WIRED
> >  	bool "Enable wired support"
> >  	depends on BR2_TOOLCHAIN_HAS_THREADS # libnl
> > -	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_6
> > +	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_7
> 
> Thanks, but it seems a bit drastic.
> 
> If I looked at the code:
> 
> ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_WIRED),y)
> WPA_SUPPLICANT_DEPENDENCIES += host-pkgconf libnl
> WPA_SUPPLICANT_CONFIG_ENABLE += \
>         CONFIG_LIBNL32 \
>         CONFIG_DRIVER_WIRED \
>         CONFIG_MACSEC \
>         CONFIG_DRIVER_MACSEC_LINUX
> 
> so it forces CONFIG_DRIVER_MACSEC_LINUX, what about making that
> dependent on kernel headers >= 5.7, rather than preventing the whole
> wired driver from being compiled?
> 
> Or, in fact even better, in ./src/drivers/driver_macsec_linux.c, change:
> 
> #if LIBNL_VER_NUM >= LIBNL_VER(3, 6)
> #define LIBNL_HAS_OFFLOAD
> #endif
> 
> to:
> 
> #if LIBNL_VER_NUM >= LIBNL_VER(3, 6) && LINUX_VERSION_CODE >= KERNEL_VERSION(5,7,0)
> #define LIBNL_HAS_OFFLOAD
> #endif
> 
> And that should only enable the offload support... when it can really
> be supported. Could you check this?

I like the second option with explicit kernel and libnl version check.
I build-tested several configurations and it works just fine:

: diff --git a/src/drivers/driver_macsec_linux.c b/src/drivers/driver_macsec_linux.c
: index c86715498..9ad24183e 100644
: --- a/src/drivers/driver_macsec_linux.c
: +++ b/src/drivers/driver_macsec_linux.c
: @@ -19,6 +19,7 @@
:  #include <netlink/route/link.h>
:  #include <netlink/route/link/macsec.h>
:  #include <linux/if_macsec.h>
: +#include <linux/version.h>
:  #include <inttypes.h>
:  
:  #include "utils/common.h"
: @@ -32,7 +33,7 @@
:  
:  #define UNUSED_SCI 0xffffffffffffffff
:  
: -#if LIBNL_VER_NUM >= LIBNL_VER(3, 6)
: +#if (LIBNL_VER_NUM >= LIBNL_VER(3, 6) && LINUX_VERSION_CODE >= KERNEL_VERSION(5,7,0))
:  #define LIBNL_HAS_OFFLOAD
:  #endif

I will send a fix to hostapd mailing list. If it is not acceptable for
whatever reason, we can fallback to the separate Buildroot option for
MACSEC.

BTW, the idea and draft is yours, so let me know if I should keep
your authorship of the hostapd patch.

Regards,
Sergey
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/2] package/wpa_supplicant: wired driver needs headers >= 5.7

[Thomas Petazzoni via buildroot]

Hello Sergey,

On Mon, 19 Aug 2024 23:07:38 +0300
Sergey Matyukevich <geomatsi@gmail.com> wrote:

> > And that should only enable the offload support... when it can really
> > be supported. Could you check this?  
> 
> I like the second option with explicit kernel and libnl version check.
> I build-tested several configurations and it works just fine:

Excellent!

> I will send a fix to hostapd mailing list. If it is not acceptable for
> whatever reason, we can fallback to the separate Buildroot option for
> MACSEC.

Sounds like a good plan.

> BTW, the idea and draft is yours, so let me know if I should keep
> your authorship of the hostapd patch.

I have written/contributed enough patches that I no longer really care
about authorship. Feel free to stupid the hostapd patch under your
name, it's going to make things easier (and in fact you did all the
testing, I just threw some random untested one-liner code in an e-mail).

Thanks for all your work!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot