neighbor: Fix locking order for gc_list changes
authorDavid Ahern <dsahern@gmail.com>
Wed, 12 Dec 2018 01:57:21 +0000 (18:57 -0700)
committerDavid S. Miller <davem@davemloft.net>
Fri, 14 Dec 2018 23:44:47 +0000 (15:44 -0800)
commit9c29a2f55ec05cc8b525ee3b2d75d3cd37911123
tree39283a1e264ed4d46da436a5ec4b6c704392c5e8
parentaeb3fecde811d5392ed481d8558f5751ac542e77
neighbor: Fix locking order for gc_list changes

Lock checker noted an inverted lock order between neigh_change_state
(neighbor lock then table lock) and neigh_periodic_work (table lock and
then neighbor lock) resulting in:

[  121.057652] ======================================================
[  121.058740] WARNING: possible circular locking dependency detected
[  121.059861] 4.20.0-rc6+ #43 Not tainted
[  121.060546] ------------------------------------------------------
[  121.061630] kworker/0:2/65 is trying to acquire lock:
[  121.062519] (____ptrval____) (&n->lock){++--}, at: neigh_periodic_work+0x237/0x324
[  121.063894]
[  121.063894] but task is already holding lock:
[  121.064920] (____ptrval____) (&tbl->lock){+.-.}, at: neigh_periodic_work+0x194/0x324
[  121.066274]
[  121.066274] which lock already depends on the new lock.
[  121.066274]
[  121.067693]
[  121.067693] the existing dependency chain (in reverse order) is:
...

Fix by renaming neigh_change_state to neigh_update_gc_list, changing
it to only manage whether an entry should be on the gc_list and taking
locks in the same order as neigh_periodic_work. Invoke at the end of
neigh_update only if diff between old or new states has the PERMANENT
flag set.

Fixes: 8cc196d6ef86 ("neighbor: gc_list changes should be protected by table lock")
Signed-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/core/neighbour.c