[PATCH net-next] net/smc: abort the connection when the peer overruns the RMB

[Hidayath Khan <hidayath@linux.ibm.com>]

smc_cdc_msg_recv_action() accumulates the peer's producer cursor advance
into conn->bytes_to_rcv:

        atomic_add(diff_prod, &conn->bytes_to_rcv);
        /* guarantee 0 <= bytes_to_rcv <= rmb_desc->len */

Nothing enforces that comment.  A conforming peer cannot advance its
producer cursor past our consumer cursor, so a peer that does fills the
accumulator without bound.

Bounding the cursor does not help: a wrap increment with count 0 is a legal
advance of exactly one bufferful, so every per-cursor check accepts it
while smc_curs_diff() returns size for each such message.

The copy-length clamps bound the copy but never repair the counter, so
SIOCINQ reports a readable length that does not exist and
smc_rx_data_available() keeps poll() readable forever with nothing for
recvmsg() to return.

Drop the connection instead, reusing the abort smc_cdc_msg_validate()
performs for an out-of-range sequence number.  The test precedes the
atomic_add() so the counter is never left inflated; abandoning the rest of
the message matches smc_cdc_msg_validate(), and abort_work tears the
connection down.

The test is written as a subtraction.  diff_prod is not bounded by the RMB:
smc_curs_diff() documents "difference cannot exceed size" as an assumption
about its inputs, but the peer controls the cursor it is computed from, so
it can return up to INT_MAX.  Adding that to bytes_to_rcv would overflow
and the comparison would then read as false, defeating the check in exactly
the case it exists for.  Subtracting cannot overflow, since bytes_to_rcv is
in [0, rmb_desc->len].

Both transports are affected: the accumulator is shared.

Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.com>
Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
---
 net/smc/smc_cdc.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
index 32d6d03df321..d8f747bbc6f6 100644
--- a/net/smc/smc_cdc.c
+++ b/net/smc/smc_cdc.c
@@ -305,6 +305,27 @@ static void smc_cdc_handle_urg_data_arrival(struct smc_sock *smc,
 	sk_send_sigurg(&smc->sk);
 }
 
+/* Drop a connection whose peer violated the CDC protocol.  @link is the link
+ * the offending message arrived on, or NULL when it is not known to the caller
+ * (SMC-D, and the shared receive path).  When @link is NULL conn->lnk is left
+ * unchanged: for SMC-R it already points to the right QP; for SMC-D there is
+ * no per-link QP to abort on.
+ */
+static void smc_cdc_abort_conn(struct smc_sock *smc, struct smc_link *link)
+{
+	struct smc_connection *conn = &smc->conn;
+
+	conn->out_of_sync = 1;  /* prevent any further receives */
+	spin_lock_bh(&conn->send_lock);
+	conn->local_tx_ctrl.conn_state_flags.peer_conn_abort = 1;
+	if (link)
+		conn->lnk = link;
+	spin_unlock_bh(&conn->send_lock);
+	sock_hold(&smc->sk); /* sock_put in abort_work */
+	if (!queue_work(smc_close_wq, &conn->abort_work))
+		sock_put(&smc->sk);
+}
+
 static void smc_cdc_msg_validate(struct smc_sock *smc, struct smc_cdc_msg *cdc,
 				 struct smc_link *link)
 {
@@ -314,17 +335,8 @@ static void smc_cdc_msg_validate(struct smc_sock *smc, struct smc_cdc_msg *cdc,
 
 	/* check that seqnum was seen before */
 	diff = conn->local_rx_ctrl.seqno - recv_seq;
-	if (diff < 0) { /* diff larger than 0x7fff */
-		/* drop connection */
-		conn->out_of_sync = 1;	/* prevent any further receives */
-		spin_lock_bh(&conn->send_lock);
-		conn->local_tx_ctrl.conn_state_flags.peer_conn_abort = 1;
-		conn->lnk = link;
-		spin_unlock_bh(&conn->send_lock);
-		sock_hold(&smc->sk); /* sock_put in abort_work */
-		if (!queue_work(smc_close_wq, &conn->abort_work))
-			sock_put(&smc->sk);
-	}
+	if (diff < 0) /* diff larger than 0x7fff */
+		smc_cdc_abort_conn(smc, link);  /* drop connection */
 }
 
 static void smc_cdc_msg_recv_action(struct smc_sock *smc,
@@ -376,6 +388,11 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc,
 	diff_prod = smc_curs_diff(conn->rmb_desc->len, &prod_old,
 				  &conn->local_rx_ctrl.prod);
 	if (diff_prod) {
+		if (diff_prod > conn->rmb_desc->len -
+				atomic_read(&conn->bytes_to_rcv)) {
+			smc_cdc_abort_conn(smc, NULL);
+			return;
+		}
 		if (conn->local_rx_ctrl.prod_flags.urg_data_present)
 			smc_cdc_handle_urg_data_arrival(smc, &diff_prod);
 		/* bytes_to_rcv is decreased in smc_recvmsg */

base-commit: cd138abf911f7a6b19656e446b09cff4453d1f3d
-- 
2.52.0