Skip to content

Commit

Permalink
Merge branch 'rxrpc-call-state-fixes'
Browse files Browse the repository at this point in the history
David Howells says:

====================
rxrpc: Call state fixes

Here some call state fixes for AF_RXRPC.

 (1) Fix the state of a call to not treat the challenge-response cycle as
     part of an incoming call's state set.  The problem is that it makes
     handling received of the final packet in the receive phase difficult
     as that wants to change the call state - but security negotiations may
     not yet be complete.

 (2) Fix a race between the changing of the call state at the end of the
     request reception phase of a service call, recvmsg() collecting the last
     data and sendmsg() trying to send the reply before the I/O thread has
     advanced the call state.

Link: https://lore.kernel.org/20250203110307.7265-2-dhowells@redhat.com
====================

Link: https://patch.msgid.link/20250204230558.712536-1-dhowells@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Feb 6, 2025
2 parents 811b8f5 + 2d7b30a commit 884af6a
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 11 deletions.
2 changes: 1 addition & 1 deletion net/rxrpc/ar-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ enum rxrpc_call_flag {
RXRPC_CALL_EXCLUSIVE, /* The call uses a once-only connection */
RXRPC_CALL_RX_IS_IDLE, /* recvmsg() is idle - send an ACK */
RXRPC_CALL_RECVMSG_READ_ALL, /* recvmsg() read all of the received data */
RXRPC_CALL_CONN_CHALLENGING, /* The connection is being challenged */
};

/*
Expand All @@ -602,7 +603,6 @@ enum rxrpc_call_state {
RXRPC_CALL_CLIENT_AWAIT_REPLY, /* - client awaiting reply */
RXRPC_CALL_CLIENT_RECV_REPLY, /* - client receiving reply phase */
RXRPC_CALL_SERVER_PREALLOC, /* - service preallocation */
RXRPC_CALL_SERVER_SECURING, /* - server securing request connection */
RXRPC_CALL_SERVER_RECV_REQUEST, /* - server receiving request */
RXRPC_CALL_SERVER_ACK_REQUEST, /* - server pending ACK of request */
RXRPC_CALL_SERVER_SEND_REPLY, /* - server sending reply */
Expand Down
6 changes: 2 additions & 4 deletions net/rxrpc/call_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const char *const rxrpc_call_states[NR__RXRPC_CALL_STATES] = {
[RXRPC_CALL_CLIENT_AWAIT_REPLY] = "ClAwtRpl",
[RXRPC_CALL_CLIENT_RECV_REPLY] = "ClRcvRpl",
[RXRPC_CALL_SERVER_PREALLOC] = "SvPrealc",
[RXRPC_CALL_SERVER_SECURING] = "SvSecure",
[RXRPC_CALL_SERVER_RECV_REQUEST] = "SvRcvReq",
[RXRPC_CALL_SERVER_ACK_REQUEST] = "SvAckReq",
[RXRPC_CALL_SERVER_SEND_REPLY] = "SvSndRpl",
Expand Down Expand Up @@ -453,17 +452,16 @@ void rxrpc_incoming_call(struct rxrpc_sock *rx,
call->cong_tstamp = skb->tstamp;

__set_bit(RXRPC_CALL_EXPOSED, &call->flags);
rxrpc_set_call_state(call, RXRPC_CALL_SERVER_SECURING);
rxrpc_set_call_state(call, RXRPC_CALL_SERVER_RECV_REQUEST);

spin_lock(&conn->state_lock);

switch (conn->state) {
case RXRPC_CONN_SERVICE_UNSECURED:
case RXRPC_CONN_SERVICE_CHALLENGING:
rxrpc_set_call_state(call, RXRPC_CALL_SERVER_SECURING);
__set_bit(RXRPC_CALL_CONN_CHALLENGING, &call->flags);
break;
case RXRPC_CONN_SERVICE:
rxrpc_set_call_state(call, RXRPC_CALL_SERVER_RECV_REQUEST);
break;

case RXRPC_CONN_ABORTED:
Expand Down
4 changes: 1 addition & 3 deletions net/rxrpc/conn_event.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,8 @@ static void rxrpc_abort_calls(struct rxrpc_connection *conn)
*/
static void rxrpc_call_is_secure(struct rxrpc_call *call)
{
if (call && __rxrpc_call_state(call) == RXRPC_CALL_SERVER_SECURING) {
rxrpc_set_call_state(call, RXRPC_CALL_SERVER_RECV_REQUEST);
if (call && __test_and_clear_bit(RXRPC_CALL_CONN_CHALLENGING, &call->flags))
rxrpc_notify_socket(call);
}
}

/*
Expand Down
12 changes: 10 additions & 2 deletions net/rxrpc/input.c
Original file line number Diff line number Diff line change
Expand Up @@ -448,11 +448,19 @@ static void rxrpc_input_queue_data(struct rxrpc_call *call, struct sk_buff *skb,
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
bool last = sp->hdr.flags & RXRPC_LAST_PACKET;

skb_queue_tail(&call->recvmsg_queue, skb);
spin_lock_irq(&call->recvmsg_queue.lock);

__skb_queue_tail(&call->recvmsg_queue, skb);
rxrpc_input_update_ack_window(call, window, wtop);
trace_rxrpc_receive(call, last ? why + 1 : why, sp->hdr.serial, sp->hdr.seq);
if (last)
/* Change the state inside the lock so that recvmsg syncs
* correctly with it and using sendmsg() to send a reply
* doesn't race.
*/
rxrpc_end_rx_phase(call, sp->hdr.serial);

spin_unlock_irq(&call->recvmsg_queue.lock);
}

/*
Expand Down Expand Up @@ -657,7 +665,7 @@ static bool rxrpc_input_split_jumbo(struct rxrpc_call *call, struct sk_buff *skb
rxrpc_propose_delay_ACK(call, sp->hdr.serial,
rxrpc_propose_ack_input_data);
}
if (notify) {
if (notify && !test_bit(RXRPC_CALL_CONN_CHALLENGING, &call->flags)) {
trace_rxrpc_notify_socket(call->debug_id, sp->hdr.serial);
rxrpc_notify_socket(call);
}
Expand Down
2 changes: 1 addition & 1 deletion net/rxrpc/sendmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
} else {
switch (rxrpc_call_state(call)) {
case RXRPC_CALL_CLIENT_AWAIT_CONN:
case RXRPC_CALL_SERVER_SECURING:
case RXRPC_CALL_SERVER_RECV_REQUEST:
if (p.command == RXRPC_CMD_SEND_ABORT)
break;
fallthrough;
Expand Down

0 comments on commit 884af6a

Please sign in to comment.