| From 4a2d4496e15ea5bb5c8e83b94ca8ca7fb045e7d3 Mon Sep 17 00:00:00 2001 |
| From: =?UTF-8?q?Linus=20L=C3=BCssing?= <ll@simonwunderlich.de> |
| Date: Thu, 10 Mar 2022 19:35:13 +0100 |
| Subject: mac80211: fix potential double free on mesh join |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Linus Lüssing <ll@simonwunderlich.de> |
| |
| commit 4a2d4496e15ea5bb5c8e83b94ca8ca7fb045e7d3 upstream. |
| |
| 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. |
| |
| Cc: stable@vger.kernel.org |
| Fixes: 6a01afcf8468 ("mac80211: mesh: Free ie data when leaving mesh") |
| Reported-by: Matthias Kretschmer <mathias.kretschmer@fit.fraunhofer.de> |
| Signed-off-by: Linus Lüssing <ll@simonwunderlich.de> |
| Tested-by: Mathias Kretschmer <mathias.kretschmer@fit.fraunhofer.de> |
| Link: https://lore.kernel.org/r/20220310183513.28589-1-linus.luessing@c0d3.blue |
| Signed-off-by: Johannes Berg <johannes.berg@intel.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/mac80211/cfg.c | 3 --- |
| 1 file changed, 3 deletions(-) |
| |
| --- a/net/mac80211/cfg.c |
| +++ b/net/mac80211/cfg.c |
| @@ -2148,14 +2148,12 @@ static int copy_mesh_setup(struct ieee80 |
| 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 ieee80 |
| } |
| 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; |