linux-kernel - Re: [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160628122747.GA7757@localhost>
Date:	Tue, 28 Jun 2016 08:27:47 -0400
From:	Bob Copeland <me@...copeland.com>
To:	Yaniv Machani <yanivma@...com>
Cc:	linux-kernel@...r.kernel.org,
	Johannes Berg <johannes@...solutions.net>,
	"David S . Miller" <davem@...emloft.net>,
	linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
	Maital Hahn <maitalm@...com>
Subject: Re: [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh
 peer is disconnecting

On Tue, Jun 28, 2016 at 02:13:05PM +0300, Yaniv Machani wrote:
> From: Maital Hahn <maitalm@...com>
> 
> Once receiving a CLOSE action frame from the disconnecting peer,
> flush all entries in the path table which has this peer as the
> next hop.

Please address the user-visible behavior in your commit messages.
Does it crash?  Does it send frames to an invalid peer?  Do
frames get dropped?

> In addition, upon receiving a packet, if next hop is not found,
> trigger PERQ immidiatly, instead of just putting it in the queue.

"PREQ"

Please split this into a separate patch that we can review
separately (and also give the "why" in the commit log).

> Signed-off-by: Maital Hahn <maitalm@...com>
> Acked-by: Yaniv Machani <yanivma@...com>
> ---
>  net/mac80211/cfg.c       |  1 +
>  net/mac80211/mesh.c      |  3 ++-
>  net/mac80211/mesh_hwmp.c | 42 +++++++++++++++++++++++++-----------------
>  3 files changed, 28 insertions(+), 18 deletions(-)
> 
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 0c12e40..f876ef7 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1011,6 +1011,7 @@ static void sta_apply_mesh_params(struct ieee80211_local *local,
>  			if (sta->mesh->plink_state == NL80211_PLINK_ESTAB)
>  				changed = mesh_plink_dec_estab_count(sdata);
>  			sta->mesh->plink_state = params->plink_state;
> +			mesh_path_flush_by_nexthop(sta);

This isn't necessary, caller should already be doing
mesh_path_flush_by_nexthop() in every case I could see.  Besides it
cannot be done under plink lock.

> +++ b/net/mac80211/mesh.c
> @@ -159,7 +159,8 @@ void mesh_sta_cleanup(struct sta_info *sta)
>  	if (!sdata->u.mesh.user_mpm) {
>  		changed |= mesh_plink_deactivate(sta);
>  		del_timer_sync(&sta->mesh->plink_timer);
> -	}
> +	} else
> +		mesh_path_flush_by_nexthop(sta);

And this is already fixed in mac80211-next.

-- 
Bob Copeland %% http://bobcopeland.com/