recent_proc_net_exit() removes the proc entries of all tables with
recent_lock held. remove_proc_entry() waits until nobody calls into the
entry anymore, and reading one of them takes recent_lock in
recent_seq_start() while procfs has the entry pinned. So when a reader
gets there while the network namespace goes away, the exit path sleeps
with a spinlock held and bh disabled, waiting for a reader that spins
on that very lock:

  BUG: sleeping function called from invalid context at 
kernel/sched/completion.c:116
  in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 12, name: kworker/u16:0
  locks held by kworker/u16:0/12: 4, last CPU#2:
   #2: (pernet_ops_rwsem){++++}-{4:4}, at: cleanup_net+0xbf/0x860
   #3: (recent_lock){+...}-{3:3}, at: recent_net_exit+0x3d/0x170 [xt_recent]
  Workqueue: netns cleanup_net
  Call Trace:
   wait_for_completion+0x8e/0x400
   proc_entry_rundown+0x1ab/0x1f0
   remove_proc_entry+0x1ab/0x530
   recent_net_exit+0x9f/0x170 [xt_recent]
   ops_undo_list+0x271/0x8d0
   cleanup_net+0x3f3/0x860
   process_one_work+0x946/0x1820
  BUG: scheduling while atomic: kworker/u16:0/12/0x00000202

  watchdog: BUG: soft lockup - CPU#0 stuck for 26s! [rloop:252]
  Call Trace:
   _raw_spin_lock_bh+0x7a/0x80
   recent_seq_start+0x86/0x1e0 [xt_recent]
   seq_read_iter+0x26d/0x1230
   seq_read+0x2a9/0x560
   proc_reg_read+0x1c2/0x3c0
   vfs_read+0x176/0xb40
   __x64_sys_pread64+0x191/0x250

Tables are added and removed with recent_mutex held and that's also
what recent_mt_destroy() looks at ->xt_recent under. Nothing that runs
with a proc entry pinned takes it. So take recent_mutex instead.

Fixes: 665e205c16c1 ("netfilter: xt_recent: fix namespace destroy path")
Cc: [email protected]
Signed-off-by: Christian Brauner (Amutable) <[email protected]>
---
 net/netfilter/xt_recent.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
index 55f1f2d89952..11acb9a50d4b 100644
--- a/net/netfilter/xt_recent.c
+++ b/net/netfilter/xt_recent.c
@@ -652,12 +652,12 @@ static void __net_exit recent_proc_net_exit(struct net 
*net)
         * that the parent xt_recent proc entry is empty before trying to
         * remove it.
         */
-       spin_lock_bh(&recent_lock);
+       mutex_lock(&recent_mutex);
        list_for_each_entry(t, &recent_net->tables, list)
                remove_proc_entry(t->name, recent_net->xt_recent);
 
        recent_net->xt_recent = NULL;
-       spin_unlock_bh(&recent_lock);
+       mutex_unlock(&recent_mutex);
 
        remove_proc_entry("xt_recent", net->proc_net);
 }

-- 
2.53.0


Reply via email to