netdev - Re: [PATCH v2] rhashtable: make walk safe from softirq context

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: <20190212215437.GA18182@localhost>
Date:   Tue, 12 Feb 2019 16:54:37 -0500
From:   Bob Copeland <me@...copeland.com>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     David Miller <davem@...emloft.net>, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, j@...fi, tgraf@...g.ch,
        herbert@...dor.apana.org.au
Subject: Re: [PATCH v2] rhashtable: make walk safe from softirq context

On Tue, Feb 12, 2019 at 08:03:17PM +0100, Johannes Berg wrote:
> commit 60854fd94573f0d3b80b55b40cf0140a0430f3ab
> Author: Bob Copeland <me@...copeland.com>
> Date:   Wed Mar 2 10:09:20 2016 -0500
> 
>     mac80211: mesh: convert path table to rhashtable
> 
> which is kinda old. Not sure why this didn't surface before, because the
> spinlock was introduced *before*, otherwise certainly the mutex would've
> caused us to not be able to do this code to start with (commit
> c6ff5268293 - rhashtable: Fix walker list corruption).
> 
> That commit also just converted an existing hashtable walk to
> rhashtable, so not sure that counts as having introduced the problem :-)

Yeah, I didn't change any of the contexts in which the iterations were
called, except making it possible to replace the previous mesh-specific
RCU hashtable with rhashtable by introducing this.  That said, it would
certainly be nice to not have to walk the table to find paths that
use a specific station as nexthop.

> But I guess we should also ask Bob first:
> 1) do you think it'd be easy to maintain a separate list or avoid the
>    iteration in some otherway, and make that a small enough patch to be
>    applicable for stable?

I'm not sure, it's been a while.  I guess most of the difficulties would
be around station removal?

> 2) or do you think maybe the mesh_plink_broken() call could just be
>    lifted into a workqueue instead?

As far as I know, no reason this can't wait until later; we might send
a few frames to the wrong destination but that can happen anyway.  But
there are a couple more walks in there like mesh_path_flush_by_nexthop().

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