Skip to content

Commit

Permalink
afs: Protect call->state changes against signals
Browse files Browse the repository at this point in the history
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 <dhowells@redhat.com>
  • Loading branch information
David Howells committed Nov 13, 2017
1 parent 13524ab commit 98bf40c
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 69 deletions.
26 changes: 12 additions & 14 deletions fs/afs/cmservice.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ static int afs_deliver_cb_callback(struct afs_call *call)

switch (call->unmarshall) {
case 0:
rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx);
call->offset = 0;
call->unmarshall++;

Expand Down Expand Up @@ -281,10 +280,12 @@ static int afs_deliver_cb_callback(struct afs_call *call)
break;
}

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

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

/* no unmarshalling required */
call->state = AFS_CALL_REPLYING;

/* we'll need the file server record as that tells us which set of
* vnodes to operate upon */
server = afs_find_server(call->net, &srx);
Expand All @@ -352,8 +350,6 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)

_enter("");

rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx);

_enter("{%u}", call->unmarshall);

switch (call->unmarshall) {
Expand Down Expand Up @@ -397,11 +393,12 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
break;
}

/* no unmarshalling required */
call->state = AFS_CALL_REPLYING;
if (!afs_check_call_state(call, AFS_CALL_SV_REPLYING))
return -EIO;

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

/* no unmarshalling required */
call->state = AFS_CALL_REPLYING;
if (!afs_check_call_state(call, AFS_CALL_SV_REPLYING))
return -EIO;

return afs_queue_call_work(call);
}
Expand Down Expand Up @@ -519,7 +516,8 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call)
break;
}

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

return afs_queue_call_work(call);
}
Expand Down Expand Up @@ -600,8 +598,8 @@ static int afs_deliver_cb_tell_me_about_yourself(struct afs_call *call)
if (ret < 0)
return ret;

/* no unmarshalling required */
call->state = AFS_CALL_REPLYING;
if (!afs_check_call_state(call, AFS_CALL_SV_REPLYING))
return -EIO;

return afs_queue_call_work(call);
}
63 changes: 54 additions & 9 deletions fs/afs/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,14 @@ struct afs_iget_data {
};

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

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

#include <trace/events/afs.h>

/*****************************************************************************/
/*
* addr_list.c
Expand Down Expand Up @@ -788,6 +792,49 @@ static inline int afs_transfer_reply(struct afs_call *call)
return afs_extract_data(call, call->buffer, call->reply_max, false);
}

static inline bool afs_check_call_state(struct afs_call *call,
enum afs_call_state state)
{
return READ_ONCE(call->state) == state;
}

static inline bool afs_set_call_state(struct afs_call *call,
enum afs_call_state from,
enum afs_call_state to)
{
bool ok = false;

spin_lock_bh(&call->state_lock);
if (call->state == from) {
call->state = to;
trace_afs_call_state(call, from, to, 0, 0);
ok = true;
}
spin_unlock_bh(&call->state_lock);
return ok;
}

static inline void afs_set_call_complete(struct afs_call *call,
int error, u32 remote_abort)
{
enum afs_call_state state;
bool ok = false;

spin_lock_bh(&call->state_lock);
state = call->state;
if (state != AFS_CALL_COMPLETE) {
call->abort_code = remote_abort;
call->error = error;
call->state = AFS_CALL_COMPLETE;
trace_afs_call_state(call, state, AFS_CALL_COMPLETE,
error, remote_abort);
ok = true;
}
spin_unlock_bh(&call->state_lock);
if (ok)
trace_afs_call_done(call);
}

/*
* security.c
*/
Expand Down Expand Up @@ -932,8 +979,6 @@ static inline void afs_check_for_remote_deletion(struct afs_fs_cursor *fc,
/*
* debug tracing
*/
#include <trace/events/afs.h>

extern unsigned afs_debug;

#define dbgprintk(FMT,...) \
Expand Down
Loading

0 comments on commit 98bf40c

Please sign in to comment.