Skip to content

Commit

Permalink
rxrpc: Call state should be read with READ_ONCE() under some circumst…
Browse files Browse the repository at this point in the history 
…ances

The call state may be changed at any time by the data-ready routine in
response to received packets, so if the call state is to be read and acted
upon several times in a function, READ_ONCE() must be used unless the call
state lock is held.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David Howells authored and David S. Miller committed Mar 7, 2017
1 parent 02b2faa commit 146d8fe
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 25 deletions.
12 changes: 7 additions & 5 deletions net/rxrpc/input.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb,
u16 skew)
{
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
enum rxrpc_call_state state;
unsigned int offset = sizeof(struct rxrpc_wire_header);
unsigned int ix;
rxrpc_serial_t serial = sp->hdr.serial, ack_serial = 0;
Expand All @@ -434,14 +435,15 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb,
_proto("Rx DATA %%%u { #%u f=%02x }",
sp->hdr.serial, seq, sp->hdr.flags);

if (call->state >= RXRPC_CALL_COMPLETE)
state = READ_ONCE(call->state);
if (state >= RXRPC_CALL_COMPLETE)
return;

/* Received data implicitly ACKs all of the request packets we sent
* when we're acting as a client.
*/
if ((call->state == RXRPC_CALL_CLIENT_SEND_REQUEST ||
call->state == RXRPC_CALL_CLIENT_AWAIT_REPLY) &&
if ((state == RXRPC_CALL_CLIENT_SEND_REQUEST ||
state == RXRPC_CALL_CLIENT_AWAIT_REPLY) &&
!rxrpc_receiving_reply(call))
return;

Expand Down Expand Up @@ -799,7 +801,7 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
return rxrpc_proto_abort("AK0", call, 0);

/* Ignore ACKs unless we are or have just been transmitting. */
switch (call->state) {
switch (READ_ONCE(call->state)) {
case RXRPC_CALL_CLIENT_SEND_REQUEST:
case RXRPC_CALL_CLIENT_AWAIT_REPLY:
case RXRPC_CALL_SERVER_SEND_REPLY:
Expand Down Expand Up @@ -940,7 +942,7 @@ static void rxrpc_input_call_packet(struct rxrpc_call *call,
static void rxrpc_input_implicit_end_call(struct rxrpc_connection *conn,
struct rxrpc_call *call)
{
switch (call->state) {
switch (READ_ONCE(call->state)) {
case RXRPC_CALL_SERVER_AWAIT_ACK:
rxrpc_call_completed(call);
break;
Expand Down
4 changes: 2 additions & 2 deletions net/rxrpc/recvmsg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
msg->msg_namelen = len;
}

switch (call->state) {
switch (READ_ONCE(call->state)) {
case RXRPC_CALL_SERVER_ACCEPTING:
ret = rxrpc_recvmsg_new_call(rx, call, msg, flags);
break;
Expand Down Expand Up @@ -640,7 +640,7 @@ int rxrpc_kernel_recv_data(struct socket *sock, struct rxrpc_call *call,

mutex_lock(&call->user_mutex);

switch (call->state) {
switch (READ_ONCE(call->state)) {
case RXRPC_CALL_CLIENT_RECV_REPLY:
case RXRPC_CALL_SERVER_RECV_REQUEST:
case RXRPC_CALL_SERVER_ACK_REQUEST:
Expand Down
48 changes: 30 additions & 18 deletions net/rxrpc/sendmsg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ rxrpc_new_client_call_for_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg,
int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
__releases(&rx->sk.sk_lock.slock)
{
enum rxrpc_call_state state;
enum rxrpc_command cmd;
struct rxrpc_call *call;
unsigned long user_call_ID = 0;
Expand Down Expand Up @@ -526,13 +527,17 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
return PTR_ERR(call);
/* ... and we have the call lock. */
} else {
ret = -EBUSY;
if (call->state == RXRPC_CALL_UNINITIALISED ||
call->state == RXRPC_CALL_CLIENT_AWAIT_CONN ||
call->state == RXRPC_CALL_SERVER_PREALLOC ||
call->state == RXRPC_CALL_SERVER_SECURING ||
call->state == RXRPC_CALL_SERVER_ACCEPTING)
switch (READ_ONCE(call->state)) {
case RXRPC_CALL_UNINITIALISED:
case RXRPC_CALL_CLIENT_AWAIT_CONN:
case RXRPC_CALL_SERVER_PREALLOC:
case RXRPC_CALL_SERVER_SECURING:
case RXRPC_CALL_SERVER_ACCEPTING:
ret = -EBUSY;
goto error_release_sock;
default:
break;
}

ret = mutex_lock_interruptible(&call->user_mutex);
release_sock(&rx->sk);
Expand All @@ -542,10 +547,11 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
}
}

state = READ_ONCE(call->state);
_debug("CALL %d USR %lx ST %d on CONN %p",
call->debug_id, call->user_call_ID, call->state, call->conn);
call->debug_id, call->user_call_ID, state, call->conn);

if (call->state >= RXRPC_CALL_COMPLETE) {
if (state >= RXRPC_CALL_COMPLETE) {
/* it's too late for this call */
ret = -ESHUTDOWN;
} else if (cmd == RXRPC_CMD_SEND_ABORT) {
Expand All @@ -555,12 +561,12 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
} else if (cmd != RXRPC_CMD_SEND_DATA) {
ret = -EINVAL;
} else if (rxrpc_is_client_call(call) &&
call->state != RXRPC_CALL_CLIENT_SEND_REQUEST) {
state != RXRPC_CALL_CLIENT_SEND_REQUEST) {
/* request phase complete for this client call */
ret = -EPROTO;
} else if (rxrpc_is_service_call(call) &&
call->state != RXRPC_CALL_SERVER_ACK_REQUEST &&
call->state != RXRPC_CALL_SERVER_SEND_REPLY) {
state != RXRPC_CALL_SERVER_ACK_REQUEST &&
state != RXRPC_CALL_SERVER_SEND_REPLY) {
/* Reply phase not begun or not complete for service call. */
ret = -EPROTO;
} else {
Expand Down Expand Up @@ -605,14 +611,20 @@ int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
_debug("CALL %d USR %lx ST %d on CONN %p",
call->debug_id, call->user_call_ID, call->state, call->conn);

if (call->state >= RXRPC_CALL_COMPLETE) {
ret = -ESHUTDOWN; /* it's too late for this call */
} else if (call->state != RXRPC_CALL_CLIENT_SEND_REQUEST &&
call->state != RXRPC_CALL_SERVER_ACK_REQUEST &&
call->state != RXRPC_CALL_SERVER_SEND_REPLY) {
ret = -EPROTO; /* request phase complete for this client call */
} else {
switch (READ_ONCE(call->state)) {
case RXRPC_CALL_CLIENT_SEND_REQUEST:
case RXRPC_CALL_SERVER_ACK_REQUEST:
case RXRPC_CALL_SERVER_SEND_REPLY:
ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg, len);
break;
case RXRPC_CALL_COMPLETE:
/* It's too late for this call */
ret = -ESHUTDOWN;
break;
default:
/* Request phase complete for this client call */
ret = -EPROTO;
break;
}

mutex_unlock(&call->user_mutex);
Expand Down

0 comments on commit 146d8fe

Please sign in to comment.