lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Fri, 23 Dec 2022 13:29:20 +0000
From:   David Howells <dhowells@...hat.com>
To:     marc.dionne@...istor.com
Cc:     dhowells@...hat.com, linux-afs@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH] rxrpc: Fix aborting of unexposed client calls

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@...istor.com>
Signed-off-by: David Howells <dhowells@...hat.com>
cc: linux-afs@...ts.infradead.org
---

 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);


Powered by blists - more mailing lists