Skip to content

Commit 610a79f

Browse files
dhowellsbrauner
authored andcommitted
afs: Fix lock recursion
afs_wake_up_async_call() can incur lock recursion. The problem is that it is called from AF_RXRPC whilst holding the ->notify_lock, but it tries to take a ref on the afs_call struct in order to pass it to a work queue - but if the afs_call is already queued, we then have an extraneous ref that must be put... calling afs_put_call() may call back down into AF_RXRPC through rxrpc_kernel_shutdown_call(), however, which might try taking the ->notify_lock again. This case isn't very common, however, so defer it to a workqueue. The oops looks something like: BUG: spinlock recursion on CPU#0, krxrpcio/7001/1646 lock: 0xffff888141399b30, .magic: dead4ead, .owner: krxrpcio/7001/1646, .owner_cpu: 0 CPU: 0 UID: 0 PID: 1646 Comm: krxrpcio/7001 Not tainted 6.12.0-rc2-build3+ #4351 Hardware name: ASUS All Series/H97-PLUS, BIOS 2306 10/09/2014 Call Trace: <TASK> dump_stack_lvl+0x47/0x70 do_raw_spin_lock+0x3c/0x90 rxrpc_kernel_shutdown_call+0x83/0xb0 afs_put_call+0xd7/0x180 rxrpc_notify_socket+0xa0/0x190 rxrpc_input_split_jumbo+0x198/0x1d0 rxrpc_input_data+0x14b/0x1e0 ? rxrpc_input_call_packet+0xc2/0x1f0 rxrpc_input_call_event+0xad/0x6b0 rxrpc_input_packet_on_conn+0x1e1/0x210 rxrpc_input_packet+0x3f2/0x4d0 rxrpc_io_thread+0x243/0x410 ? __pfx_rxrpc_io_thread+0x10/0x10 kthread+0xcf/0xe0 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x24/0x40 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 </TASK> Signed-off-by: David Howells <[email protected]> Link: https://lore.kernel.org/r/[email protected] cc: Marc Dionne <[email protected]> cc: [email protected] cc: [email protected] Signed-off-by: Christian Brauner <[email protected]>
1 parent 15f3434 commit 610a79f

File tree

2 files changed

+61
-24
lines changed

2 files changed

+61
-24
lines changed

fs/afs/internal.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ struct afs_call {
130130
wait_queue_head_t waitq; /* processes awaiting completion */
131131
struct work_struct async_work; /* async I/O processor */
132132
struct work_struct work; /* actual work processor */
133+
struct work_struct free_work; /* Deferred free processor */
133134
struct rxrpc_call *rxcall; /* RxRPC call handle */
134135
struct rxrpc_peer *peer; /* Remote endpoint */
135136
struct key *key; /* security for this call */
@@ -1331,6 +1332,7 @@ extern int __net_init afs_open_socket(struct afs_net *);
13311332
extern void __net_exit afs_close_socket(struct afs_net *);
13321333
extern void afs_charge_preallocation(struct work_struct *);
13331334
extern void afs_put_call(struct afs_call *);
1335+
void afs_deferred_put_call(struct afs_call *call);
13341336
void afs_make_call(struct afs_call *call, gfp_t gfp);
13351337
void afs_wait_for_call_to_complete(struct afs_call *call);
13361338
extern struct afs_call *afs_alloc_flat_call(struct afs_net *,

fs/afs/rxrpc.c

Lines changed: 59 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
struct workqueue_struct *afs_async_calls;
2020

21+
static void afs_deferred_free_worker(struct work_struct *work);
2122
static void afs_wake_up_call_waiter(struct sock *, struct rxrpc_call *, unsigned long);
2223
static void afs_wake_up_async_call(struct sock *, struct rxrpc_call *, unsigned long);
2324
static void afs_process_async_call(struct work_struct *);
@@ -149,6 +150,7 @@ static struct afs_call *afs_alloc_call(struct afs_net *net,
149150
call->debug_id = atomic_inc_return(&rxrpc_debug_id);
150151
refcount_set(&call->ref, 1);
151152
INIT_WORK(&call->async_work, afs_process_async_call);
153+
INIT_WORK(&call->free_work, afs_deferred_free_worker);
152154
init_waitqueue_head(&call->waitq);
153155
spin_lock_init(&call->state_lock);
154156
call->iter = &call->def_iter;
@@ -159,6 +161,36 @@ static struct afs_call *afs_alloc_call(struct afs_net *net,
159161
return call;
160162
}
161163

164+
static void afs_free_call(struct afs_call *call)
165+
{
166+
struct afs_net *net = call->net;
167+
int o;
168+
169+
ASSERT(!work_pending(&call->async_work));
170+
171+
rxrpc_kernel_put_peer(call->peer);
172+
173+
if (call->rxcall) {
174+
rxrpc_kernel_shutdown_call(net->socket, call->rxcall);
175+
rxrpc_kernel_put_call(net->socket, call->rxcall);
176+
call->rxcall = NULL;
177+
}
178+
if (call->type->destructor)
179+
call->type->destructor(call);
180+
181+
afs_unuse_server_notime(call->net, call->server, afs_server_trace_put_call);
182+
kfree(call->request);
183+
184+
o = atomic_read(&net->nr_outstanding_calls);
185+
trace_afs_call(call->debug_id, afs_call_trace_free, 0, o,
186+
__builtin_return_address(0));
187+
kfree(call);
188+
189+
o = atomic_dec_return(&net->nr_outstanding_calls);
190+
if (o == 0)
191+
wake_up_var(&net->nr_outstanding_calls);
192+
}
193+
162194
/*
163195
* Dispose of a reference on a call.
164196
*/
@@ -173,32 +205,34 @@ void afs_put_call(struct afs_call *call)
173205
o = atomic_read(&net->nr_outstanding_calls);
174206
trace_afs_call(debug_id, afs_call_trace_put, r - 1, o,
175207
__builtin_return_address(0));
208+
if (zero)
209+
afs_free_call(call);
210+
}
176211

177-
if (zero) {
178-
ASSERT(!work_pending(&call->async_work));
179-
ASSERT(call->type->name != NULL);
180-
181-
rxrpc_kernel_put_peer(call->peer);
182-
183-
if (call->rxcall) {
184-
rxrpc_kernel_shutdown_call(net->socket, call->rxcall);
185-
rxrpc_kernel_put_call(net->socket, call->rxcall);
186-
call->rxcall = NULL;
187-
}
188-
if (call->type->destructor)
189-
call->type->destructor(call);
212+
static void afs_deferred_free_worker(struct work_struct *work)
213+
{
214+
struct afs_call *call = container_of(work, struct afs_call, free_work);
190215

191-
afs_unuse_server_notime(call->net, call->server, afs_server_trace_put_call);
192-
kfree(call->request);
216+
afs_free_call(call);
217+
}
193218

194-
trace_afs_call(call->debug_id, afs_call_trace_free, 0, o,
195-
__builtin_return_address(0));
196-
kfree(call);
219+
/*
220+
* Dispose of a reference on a call, deferring the cleanup to a workqueue
221+
* to avoid lock recursion.
222+
*/
223+
void afs_deferred_put_call(struct afs_call *call)
224+
{
225+
struct afs_net *net = call->net;
226+
unsigned int debug_id = call->debug_id;
227+
bool zero;
228+
int r, o;
197229

198-
o = atomic_dec_return(&net->nr_outstanding_calls);
199-
if (o == 0)
200-
wake_up_var(&net->nr_outstanding_calls);
201-
}
230+
zero = __refcount_dec_and_test(&call->ref, &r);
231+
o = atomic_read(&net->nr_outstanding_calls);
232+
trace_afs_call(debug_id, afs_call_trace_put, r - 1, o,
233+
__builtin_return_address(0));
234+
if (zero)
235+
schedule_work(&call->free_work);
202236
}
203237

204238
static struct afs_call *afs_get_call(struct afs_call *call,
@@ -640,7 +674,8 @@ static void afs_wake_up_call_waiter(struct sock *sk, struct rxrpc_call *rxcall,
640674
}
641675

642676
/*
643-
* wake up an asynchronous call
677+
* Wake up an asynchronous call. The caller is holding the call notify
678+
* spinlock around this, so we can't call afs_put_call().
644679
*/
645680
static void afs_wake_up_async_call(struct sock *sk, struct rxrpc_call *rxcall,
646681
unsigned long call_user_ID)
@@ -657,7 +692,7 @@ static void afs_wake_up_async_call(struct sock *sk, struct rxrpc_call *rxcall,
657692
__builtin_return_address(0));
658693

659694
if (!queue_work(afs_async_calls, &call->async_work))
660-
afs_put_call(call);
695+
afs_deferred_put_call(call);
661696
}
662697
}
663698

0 commit comments

Comments
 (0)