[PATCH] rxrpc: Fix aborting of unexposed client calls

From: David Howells
Date: Fri Dec 23 2022 - 08:30:11 EST


If a client call gets completed early, before any DATA packets have been
transmitted, say by getting interrupted by a signal, it may still cause the
transmission of an ABORT packet (which is pointless).

Further, because the call didn't go through rxrpc_expose_client_call(), the
connection channel's call counter didn't get incremented. This means that
the next call on that channel will have the same callNumber, and the server
will probably just abort it, causing an I/O error in kafs.

This, for example, can sometimes be induced with git checkout as that uses
an alarm to advance the progress display. The signal generated may
interrupt the call whilst it's waiting for a channel, but the call still
sets up even if it is pending an abort, and then sends an abort instead of
the first data.

Fix this by the following means:

(1) Always mark service calls as exposed (RXRPC_CALL_EXPOSED).

(2) Don't mark client calls as exposed until they have data to transmit.

(3) Don't send an ABORT packet unless exposed.

(4) Do the connection of client calls after processing call-pokes so that
early aborts have a chance to get dealt with first (it narrows the
window, but can't eliminate it).

Reported-by: Marc Dionne <marc.dionne@xxxxxxxxxxxx>
Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
cc: linux-afs@xxxxxxxxxxxxxxxxxxx
---

net/rxrpc/call_event.c | 6 ++++--
net/rxrpc/call_object.c | 1 +
net/rxrpc/call_state.c | 3 ++-
net/rxrpc/io_thread.c | 6 +++---
4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index 91df6fbede9c..148bb7fc0415 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -305,9 +305,11 @@ static void rxrpc_decant_prepared_tx(struct rxrpc_call *call)
{
struct rxrpc_txbuf *txb;

- if (rxrpc_is_client_call(call) &&
- !test_bit(RXRPC_CALL_EXPOSED, &call->flags))
+ if (!test_bit(RXRPC_CALL_EXPOSED, &call->flags)) {
+ if (list_empty(&call->tx_sendmsg))
+ return;
rxrpc_expose_client_call(call);
+ }

while ((txb = list_first_entry_or_null(&call->tx_sendmsg,
struct rxrpc_txbuf, call_link))) {
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 45cc16c1be15..75b6ad542fdc 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -455,6 +455,7 @@ void rxrpc_incoming_call(struct rxrpc_sock *rx,
call->cong_tstamp = skb->tstamp;

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

spin_lock(&conn->state_lock);

diff --git a/net/rxrpc/call_state.c b/net/rxrpc/call_state.c
index 59a5588805ac..fae42f733e85 100644
--- a/net/rxrpc/call_state.c
+++ b/net/rxrpc/call_state.c
@@ -49,7 +49,8 @@ bool rxrpc_abort_call(struct rxrpc_call *call, rxrpc_seq_t seq,
if (!rxrpc_set_call_completion(call, RXRPC_CALL_LOCALLY_ABORTED,
abort_code, error))
return false;
- rxrpc_send_abort_packet(call);
+ if (test_bit(RXRPC_CALL_EXPOSED, &call->flags))
+ rxrpc_send_abort_packet(call);
return true;
}

diff --git a/net/rxrpc/io_thread.c b/net/rxrpc/io_thread.c
index 48e8bfd6e2ef..34d4a3a5cb72 100644
--- a/net/rxrpc/io_thread.c
+++ b/net/rxrpc/io_thread.c
@@ -435,9 +435,6 @@ int rxrpc_io_thread(void *data)
&local->client_conn_flags))
rxrpc_discard_expired_client_conns(local);

- if (!list_empty(&local->new_client_calls))
- rxrpc_connect_client_calls(local);
-
/* Deal with calls that want immediate attention. */
if ((call = list_first_entry_or_null(&local->call_attend_q,
struct rxrpc_call,
@@ -452,6 +449,9 @@ int rxrpc_io_thread(void *data)
continue;
}

+ if (!list_empty(&local->new_client_calls))
+ rxrpc_connect_client_calls(local);
+
/* Process received packets and errors. */
if ((skb = __skb_dequeue(&rx_queue))) {
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);