Yep, that's correct. We do usually not go through a NETDEV_DOWN/NETDEV_UP cycle,
when, for example, reassigning channels.
Will test your patch and report back.
BR,
Mathias
On Thu, 2022-03-10 at 19:35 +0100, Linus Lüssing wrote:
> From: Linus Lüssing <ll@xxxxxxxxxxxxxxxxxx>
>
> While commit 6a01afcf8468 ("mac80211: mesh: Free ie data when leaving
> mesh") fixed a memory leak on mesh leave / teardown it introduced a
> potential memory corruption caused by a double free when rejoining the
> mesh:
>
> ieee80211_leave_mesh()
> -> kfree(sdata->u.mesh.ie);
> ...
> ieee80211_join_mesh()
> -> copy_mesh_setup()
> -> old_ie = ifmsh->ie;
> -> kfree(old_ie);
>
> This double free / kernel panics can be reproduced by using wpa_supplicant
> with an encrypted mesh (if set up without encryption via "iw" then
> ifmsh->ie is always NULL, which avoids this issue). And then calling:
>
> $ iw dev mesh0 mesh leave
> $ iw dev mesh0 mesh join my-mesh
>
> Note that typically these commands are not used / working when using
> wpa_supplicant. And it seems that wpa_supplicant or wpa_cli are going
> through a NETDEV_DOWN/NETDEV_UP cycle between a mesh leave and mesh join
> where the NETDEV_UP resets the mesh.ie to NULL via a memcpy of
> default_mesh_setup in cfg80211_netdev_notifier_call, which then avoids
> the memory corruption, too.
>
> The issue was first observed in an application which was not using
> wpa_supplicant but "Senf" instead, which implements its own calls to
> nl80211.
>
> Fixing the issue by removing the kfree()'ing of the mesh IE in the mesh
> join function and leaving it solely up to the mesh leave to free the
> mesh IE.
>
> Link: https://gitlab.fit.fraunhofer.de/wiback/senf
> Fixes: 6a01afcf8468 ("mac80211: mesh: Free ie data when leaving mesh")
> Reported-by: Matthias Kretschmer <mathias.kretschmer@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Linus Lüssing <ll@xxxxxxxxxxxxxxxxxx>
> ---
> net/mac80211/cfg.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 87a208089caf..58ff57dc669c 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -2148,14 +2148,12 @@ static int copy_mesh_setup(struct ieee80211_if_mesh
> *ifmsh,
> const struct mesh_setup *setup)
> {
> u8 *new_ie;
> - const u8 *old_ie;
> struct ieee80211_sub_if_data *sdata = container_of(ifmsh,
> struct ieee80211_sub_if_data, u.mesh);
> int i;
>
> /* allocate information elements */
> new_ie = NULL;
> - old_ie = ifmsh->ie;
>
> if (setup->ie_len) {
> new_ie = kmemdup(setup->ie, setup->ie_len,
> @@ -2165,7 +2163,6 @@ static int copy_mesh_setup(struct ieee80211_if_mesh
> *ifmsh,
> }
> ifmsh->ie_len = setup->ie_len;
> ifmsh->ie = new_ie;
> - kfree(old_ie);
>
> /* now copy the rest of the setup parameters */
> ifmsh->mesh_id_len = setup->mesh_id_len;
