Skip to content

Commit

Permalink
rxrpc, afs: Fix peer hash locking vs RCU callback
Browse files Browse the repository at this point in the history 
In its address list, afs now retains pointers to and refs on one or more
rxrpc_peer objects.  The address list is freed under RCU and at this time,
it puts the refs on those peers.

Now, when an rxrpc_peer object runs out of refs, it gets removed from the
peer hash table and, for that, rxrpc has to take a spinlock.  However, it
is now being called from afs's RCU cleanup, which takes place in BH
context - but it is just taking an ordinary spinlock.

The put may also be called from non-BH context, and so there exists the
possibility of deadlock if the BH-based RCU cleanup happens whilst the hash
spinlock is held.  This led to the attached lockdep complaint.

Fix this by changing spinlocks of rxnet->peer_hash_lock back to
BH-disabling locks.

    ================================
    WARNING: inconsistent lock state
    6.13.0-rc5-build2+ #1223 Tainted: G            E
    --------------------------------
    inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
    swapper/1/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
    ffff88810babe228 (&rxnet->peer_hash_lock){+.?.}-{3:3}, at: rxrpc_put_peer+0xcb/0x180
    {SOFTIRQ-ON-W} state was registered at:
      mark_usage+0x164/0x180
      __lock_acquire+0x544/0x990
      lock_acquire.part.0+0x103/0x280
      _raw_spin_lock+0x2f/0x40
      rxrpc_peer_keepalive_worker+0x144/0x440
      process_one_work+0x486/0x7c0
      process_scheduled_works+0x73/0x90
      worker_thread+0x1c8/0x2a0
      kthread+0x19b/0x1b0
      ret_from_fork+0x24/0x40
      ret_from_fork_asm+0x1a/0x30
    irq event stamp: 972402
    hardirqs last  enabled at (972402): [<ffffffff8244360e>] _raw_spin_unlock_irqrestore+0x2e/0x50
    hardirqs last disabled at (972401): [<ffffffff82443328>] _raw_spin_lock_irqsave+0x18/0x60
    softirqs last  enabled at (972300): [<ffffffff810ffbbe>] handle_softirqs+0x3ee/0x430
    softirqs last disabled at (972313): [<ffffffff810ffc54>] __irq_exit_rcu+0x44/0x110

    other info that might help us debug this:
     Possible unsafe locking scenario:
           CPU0
           ----
      lock(&rxnet->peer_hash_lock);
      <Interrupt>
        lock(&rxnet->peer_hash_lock);

     *** DEADLOCK ***
    1 lock held by swapper/1/0:
     #0: ffffffff83576be0 (rcu_callback){....}-{0:0}, at: rcu_lock_acquire+0x7/0x30

    stack backtrace:
    CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Tainted: G            E      6.13.0-rc5-build2+ #1223
    Tainted: [E]=UNSIGNED_MODULE
    Hardware name: ASUS All Series/H97-PLUS, BIOS 2306 10/09/2014
    Call Trace:
     <IRQ>
     dump_stack_lvl+0x57/0x80
     print_usage_bug.part.0+0x227/0x240
     valid_state+0x53/0x70
     mark_lock_irq+0xa5/0x2f0
     mark_lock+0xf7/0x170
     mark_usage+0xe1/0x180
     __lock_acquire+0x544/0x990
     lock_acquire.part.0+0x103/0x280
     _raw_spin_lock+0x2f/0x40
     rxrpc_put_peer+0xcb/0x180
     afs_free_addrlist+0x46/0x90 [kafs]
     rcu_do_batch+0x2d2/0x640
     rcu_core+0x2f7/0x350
     handle_softirqs+0x1ee/0x430
     __irq_exit_rcu+0x44/0x110
     irq_exit_rcu+0xa/0x30
     sysvec_apic_timer_interrupt+0x7f/0xa0
     </IRQ>

Fixes: 72904d7 ("rxrpc, afs: Allow afs to pin rxrpc_peer objects")
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: "David S. Miller" <[email protected]>
cc: Eric Dumazet <[email protected]>
cc: Jakub Kicinski <[email protected]>
cc: Paolo Abeni <[email protected]>
cc: Simon Horman <[email protected]>
cc: [email protected]
cc: [email protected]
Signed-off-by: NipaLocal <nipa@local>
  • Loading branch information
@dhowells
dhowells authored and NipaLocal committed Jan 26, 2025
1 parent 0cdfba8 commit b21c695
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 14 deletions.
16 changes: 8 additions & 8 deletions net/rxrpc/peer_event.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
bool use;
int slot;

spin_lock(&rxnet->peer_hash_lock);
spin_lock_bh(&rxnet->peer_hash_lock);

while (!list_empty(collector)) {
peer = list_entry(collector->next,
Expand All @@ -257,7 +257,7 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
continue;

use = __rxrpc_use_local(peer->local, rxrpc_local_use_peer_keepalive);
spin_unlock(&rxnet->peer_hash_lock);
spin_unlock_bh(&rxnet->peer_hash_lock);

if (use) {
keepalive_at = peer->last_tx_at + RXRPC_KEEPALIVE_TIME;
Expand All @@ -277,17 +277,17 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
*/
slot += cursor;
slot &= mask;
spin_lock(&rxnet->peer_hash_lock);
spin_lock_bh(&rxnet->peer_hash_lock);
list_add_tail(&peer->keepalive_link,
&rxnet->peer_keepalive[slot & mask]);
spin_unlock(&rxnet->peer_hash_lock);
spin_unlock_bh(&rxnet->peer_hash_lock);
rxrpc_unuse_local(peer->local, rxrpc_local_unuse_peer_keepalive);
}
rxrpc_put_peer(peer, rxrpc_peer_put_keepalive);
spin_lock(&rxnet->peer_hash_lock);
spin_lock_bh(&rxnet->peer_hash_lock);
}

spin_unlock(&rxnet->peer_hash_lock);
spin_unlock_bh(&rxnet->peer_hash_lock);
}

/*
Expand Down Expand Up @@ -317,7 +317,7 @@ void rxrpc_peer_keepalive_worker(struct work_struct *work)
* second; the bucket at cursor + 1 goes at now + 1s and so
* on...
*/
spin_lock(&rxnet->peer_hash_lock);
spin_lock_bh(&rxnet->peer_hash_lock);
list_splice_init(&rxnet->peer_keepalive_new, &collector);

stop = cursor + ARRAY_SIZE(rxnet->peer_keepalive);
Expand All @@ -329,7 +329,7 @@ void rxrpc_peer_keepalive_worker(struct work_struct *work)
}

base = now;
spin_unlock(&rxnet->peer_hash_lock);
spin_unlock_bh(&rxnet->peer_hash_lock);

rxnet->peer_keepalive_base = base;
rxnet->peer_keepalive_cursor = cursor;
Expand Down
12 changes: 6 additions & 6 deletions net/rxrpc/peer_object.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,10 @@ void rxrpc_new_incoming_peer(struct rxrpc_local *local, struct rxrpc_peer *peer)
hash_key = rxrpc_peer_hash_key(local, &peer->srx);
rxrpc_init_peer(local, peer, hash_key);

spin_lock(&rxnet->peer_hash_lock);
spin_lock_bh(&rxnet->peer_hash_lock);
hash_add_rcu(rxnet->peer_hash, &peer->hash_link, hash_key);
list_add_tail(&peer->keepalive_link, &rxnet->peer_keepalive_new);
spin_unlock(&rxnet->peer_hash_lock);
spin_unlock_bh(&rxnet->peer_hash_lock);
}

/*
Expand Down Expand Up @@ -360,7 +360,7 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_local *local,
return NULL;
}

spin_lock(&rxnet->peer_hash_lock);
spin_lock_bh(&rxnet->peer_hash_lock);

/* Need to check that we aren't racing with someone else */
peer = __rxrpc_lookup_peer_rcu(local, srx, hash_key);
Expand All @@ -373,7 +373,7 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_local *local,
&rxnet->peer_keepalive_new);
}

spin_unlock(&rxnet->peer_hash_lock);
spin_unlock_bh(&rxnet->peer_hash_lock);

if (peer)
rxrpc_free_peer(candidate);
Expand Down Expand Up @@ -423,10 +423,10 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)

ASSERT(hlist_empty(&peer->error_targets));

spin_lock(&rxnet->peer_hash_lock);
spin_lock_bh(&rxnet->peer_hash_lock);
hash_del_rcu(&peer->hash_link);
list_del_init(&peer->keepalive_link);
spin_unlock(&rxnet->peer_hash_lock);
spin_unlock_bh(&rxnet->peer_hash_lock);

rxrpc_free_peer(peer);
}
Expand Down

0 comments on commit b21c695

Please sign in to comment.