Skip to content

Commit 98bf40c

Browse files
committed
afs: Protect call->state changes against signals
Protect call->state changes against the call being prematurely terminated due to a signal. What can happen is that a signal causes afs_wait_for_call_to_complete() to abort an afs_call because it's not yet complete whilst afs_deliver_to_call() is delivering data to that call. If the data delivery causes the state to change, this may overwrite the state of the afs_call, making it not-yet-complete again - but no further notifications will be forthcoming from AF_RXRPC as the rxrpc call has been aborted and completed, so kAFS will just hang in various places waiting for that call or on page bits that need clearing by that call. A tracepoint to monitor call state changes is also provided. Signed-off-by: David Howells <[email protected]>
1 parent 13524ab commit 98bf40c

File tree

4 files changed

+150
-69
lines changed

4 files changed

+150
-69
lines changed

fs/afs/cmservice.c

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@ static int afs_deliver_cb_callback(struct afs_call *call)
188188

189189
switch (call->unmarshall) {
190190
case 0:
191-
rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx);
192191
call->offset = 0;
193192
call->unmarshall++;
194193

@@ -281,10 +280,12 @@ static int afs_deliver_cb_callback(struct afs_call *call)
281280
break;
282281
}
283282

284-
call->state = AFS_CALL_REPLYING;
283+
if (!afs_check_call_state(call, AFS_CALL_SV_REPLYING))
284+
return -EIO;
285285

286286
/* we'll need the file server record as that tells us which set of
287287
* vnodes to operate upon */
288+
rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx);
288289
server = afs_find_server(call->net, &srx);
289290
if (!server)
290291
return -ENOTCONN;
@@ -325,9 +326,6 @@ static int afs_deliver_cb_init_call_back_state(struct afs_call *call)
325326
if (ret < 0)
326327
return ret;
327328

328-
/* no unmarshalling required */
329-
call->state = AFS_CALL_REPLYING;
330-
331329
/* we'll need the file server record as that tells us which set of
332330
* vnodes to operate upon */
333331
server = afs_find_server(call->net, &srx);
@@ -352,8 +350,6 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
352350

353351
_enter("");
354352

355-
rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx);
356-
357353
_enter("{%u}", call->unmarshall);
358354

359355
switch (call->unmarshall) {
@@ -397,11 +393,12 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
397393
break;
398394
}
399395

400-
/* no unmarshalling required */
401-
call->state = AFS_CALL_REPLYING;
396+
if (!afs_check_call_state(call, AFS_CALL_SV_REPLYING))
397+
return -EIO;
402398

403399
/* we'll need the file server record as that tells us which set of
404400
* vnodes to operate upon */
401+
rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx);
405402
server = afs_find_server(call->net, &srx);
406403
if (!server)
407404
return -ENOTCONN;
@@ -436,8 +433,8 @@ static int afs_deliver_cb_probe(struct afs_call *call)
436433
if (ret < 0)
437434
return ret;
438435

439-
/* no unmarshalling required */
440-
call->state = AFS_CALL_REPLYING;
436+
if (!afs_check_call_state(call, AFS_CALL_SV_REPLYING))
437+
return -EIO;
441438

442439
return afs_queue_call_work(call);
443440
}
@@ -519,7 +516,8 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call)
519516
break;
520517
}
521518

522-
call->state = AFS_CALL_REPLYING;
519+
if (!afs_check_call_state(call, AFS_CALL_SV_REPLYING))
520+
return -EIO;
523521

524522
return afs_queue_call_work(call);
525523
}
@@ -600,8 +598,8 @@ static int afs_deliver_cb_tell_me_about_yourself(struct afs_call *call)
600598
if (ret < 0)
601599
return ret;
602600

603-
/* no unmarshalling required */
604-
call->state = AFS_CALL_REPLYING;
601+
if (!afs_check_call_state(call, AFS_CALL_SV_REPLYING))
602+
return -EIO;
605603

606604
return afs_queue_call_work(call);
607605
}

fs/afs/internal.h

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,14 @@ struct afs_iget_data {
5151
};
5252

5353
enum afs_call_state {
54-
AFS_CALL_REQUESTING, /* request is being sent for outgoing call */
55-
AFS_CALL_AWAIT_REPLY, /* awaiting reply to outgoing call */
56-
AFS_CALL_AWAIT_OP_ID, /* awaiting op ID on incoming call */
57-
AFS_CALL_AWAIT_REQUEST, /* awaiting request data on incoming call */
58-
AFS_CALL_REPLYING, /* replying to incoming call */
59-
AFS_CALL_AWAIT_ACK, /* awaiting final ACK of incoming call */
60-
AFS_CALL_COMPLETE, /* Completed or failed */
54+
AFS_CALL_CL_REQUESTING, /* Client: Request is being sent */
55+
AFS_CALL_CL_AWAIT_REPLY, /* Client: Awaiting reply */
56+
AFS_CALL_CL_PROC_REPLY, /* Client: rxrpc call complete; processing reply */
57+
AFS_CALL_SV_AWAIT_OP_ID, /* Server: Awaiting op ID */
58+
AFS_CALL_SV_AWAIT_REQUEST, /* Server: Awaiting request data */
59+
AFS_CALL_SV_REPLYING, /* Server: Replying */
60+
AFS_CALL_SV_AWAIT_ACK, /* Server: Awaiting final ACK */
61+
AFS_CALL_COMPLETE, /* Completed or failed */
6162
};
6263

6364
/*
@@ -97,6 +98,7 @@ struct afs_call {
9798
size_t offset; /* offset into received data store */
9899
atomic_t usage;
99100
enum afs_call_state state;
101+
spinlock_t state_lock;
100102
int error; /* error code */
101103
u32 abort_code; /* Remote abort ID or 0 */
102104
unsigned request_size; /* size of request data */
@@ -543,6 +545,8 @@ struct afs_fs_cursor {
543545
#define AFS_FS_CURSOR_NO_VSLEEP 0x0020 /* Set to prevent sleep on VBUSY, VOFFLINE, ... */
544546
};
545547

548+
#include <trace/events/afs.h>
549+
546550
/*****************************************************************************/
547551
/*
548552
* addr_list.c
@@ -788,6 +792,49 @@ static inline int afs_transfer_reply(struct afs_call *call)
788792
return afs_extract_data(call, call->buffer, call->reply_max, false);
789793
}
790794

795+
static inline bool afs_check_call_state(struct afs_call *call,
796+
enum afs_call_state state)
797+
{
798+
return READ_ONCE(call->state) == state;
799+
}
800+
801+
static inline bool afs_set_call_state(struct afs_call *call,
802+
enum afs_call_state from,
803+
enum afs_call_state to)
804+
{
805+
bool ok = false;
806+
807+
spin_lock_bh(&call->state_lock);
808+
if (call->state == from) {
809+
call->state = to;
810+
trace_afs_call_state(call, from, to, 0, 0);
811+
ok = true;
812+
}
813+
spin_unlock_bh(&call->state_lock);
814+
return ok;
815+
}
816+
817+
static inline void afs_set_call_complete(struct afs_call *call,
818+
int error, u32 remote_abort)
819+
{
820+
enum afs_call_state state;
821+
bool ok = false;
822+
823+
spin_lock_bh(&call->state_lock);
824+
state = call->state;
825+
if (state != AFS_CALL_COMPLETE) {
826+
call->abort_code = remote_abort;
827+
call->error = error;
828+
call->state = AFS_CALL_COMPLETE;
829+
trace_afs_call_state(call, state, AFS_CALL_COMPLETE,
830+
error, remote_abort);
831+
ok = true;
832+
}
833+
spin_unlock_bh(&call->state_lock);
834+
if (ok)
835+
trace_afs_call_done(call);
836+
}
837+
791838
/*
792839
* security.c
793840
*/
@@ -932,8 +979,6 @@ static inline void afs_check_for_remote_deletion(struct afs_fs_cursor *fc,
932979
/*
933980
* debug tracing
934981
*/
935-
#include <trace/events/afs.h>
936-
937982
extern unsigned afs_debug;
938983

939984
#define dbgprintk(FMT,...) \

0 commit comments

Comments
 (0)