linux-kernel - [PATCH] iscsi-target: Fail connection on short writes/reads

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>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1418705338-23695-1-git-send-email-nab@daterainc.com>
Date:	Tue, 16 Dec 2014 04:48:58 +0000
From:	"Nicholas A. Bellinger" <nab@...erainc.com>
To:	target-devel <target-devel@...r.kernel.org>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Nicholas Bellinger <nab@...ux-iscsi.org>
Subject: [PATCH] iscsi-target: Fail connection on short writes/reads

From: Nicholas Bellinger <nab@...ux-iscsi.org>

This patch changes iscsit_do_[tx,rx]_data() to fail on short
write/reads when kernel_[send,recv]msg() returns a value different
than the requested transfer length, returning -EPIPE and thus
causing a connection reset to occur.

This avoids a potential bug in the original code where a short
write/read would result in kernel_[send,recv]msg() being called
again with the original iovec base + length.

In practice this has not been an issue because iscsit_do_tx_data()
is only used for transferring 48 byte headers + 4 byte digests,
along with seldom used control payloads from NOPIN + TEXT_RSP +
REJECT with less than 32k of data.  Nor has it been occuring with
iscsit_do_rx_data() because MSG_WAITALL won't return to caller
until the requested transfer length is reached, or an error has
occured.

So following Al's audit of iovec consumers, go ahead and fail
the connection on short writes/reads for now, and remove the
bogus logic.

Reported-by: Al Viro <viro@...iv.linux.org.uk>
Cc: David S. Miller <davem@...emloft.net>
Signed-off-by: Nicholas Bellinger <nab@...ux-iscsi.org>
---
 drivers/target/iscsi/iscsi_target_util.c | 48 +++++++++++++-------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index ce87ce9..9aa24f1 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -1326,7 +1326,7 @@ static int iscsit_do_rx_data(
 	struct iscsi_conn *conn,
 	struct iscsi_data_count *count)
 {
-	int data = count->data_length, rx_loop = 0, total_rx = 0, iov_len;
+	int ret, iov_len;
 	struct kvec *iov_p;
 	struct msghdr msg;
 
@@ -1338,35 +1338,31 @@ static int iscsit_do_rx_data(
 	iov_p = count->iov;
 	iov_len	= count->iov_count;
 
-	while (total_rx < data) {
-		rx_loop = kernel_recvmsg(conn->sock, &msg, iov_p, iov_len,
-					(data - total_rx), MSG_WAITALL);
-		if (rx_loop <= 0) {
-			pr_debug("rx_loop: %d total_rx: %d\n",
-				rx_loop, total_rx);
-			return rx_loop;
-		}
-		total_rx += rx_loop;
-		pr_debug("rx_loop: %d, total_rx: %d, data: %d\n",
-				rx_loop, total_rx, data);
+	ret = kernel_recvmsg(conn->sock, &msg, iov_p, iov_len,
+			     count->data_length, MSG_WAITALL);
+	if (ret != count->data_length) {
+		pr_err("Unexpected ret: %d recv data: %d\n",
+		       ret, count->data_length);
+		return -EPIPE;
 	}
+	pr_debug("ret: %d, recv data: %d\n", ret, count->data_length);
 
-	return total_rx;
+	return ret;
 }
 
 static int iscsit_do_tx_data(
 	struct iscsi_conn *conn,
 	struct iscsi_data_count *count)
 {
-	int data = count->data_length, total_tx = 0, tx_loop = 0, iov_len;
+	int ret, iov_len;
 	struct kvec *iov_p;
 	struct msghdr msg;
 
 	if (!conn || !conn->sock || !conn->conn_ops)
 		return -1;
 
-	if (data <= 0) {
-		pr_err("Data length is: %d\n", data);
+	if (count->data_length <= 0) {
+		pr_err("Data length is: %d\n", count->data_length);
 		return -1;
 	}
 
@@ -1375,20 +1371,16 @@ static int iscsit_do_tx_data(
 	iov_p = count->iov;
 	iov_len = count->iov_count;
 
-	while (total_tx < data) {
-		tx_loop = kernel_sendmsg(conn->sock, &msg, iov_p, iov_len,
-					(data - total_tx));
-		if (tx_loop <= 0) {
-			pr_debug("tx_loop: %d total_tx %d\n",
-				tx_loop, total_tx);
-			return tx_loop;
-		}
-		total_tx += tx_loop;
-		pr_debug("tx_loop: %d, total_tx: %d, data: %d\n",
-					tx_loop, total_tx, data);
+	ret = kernel_sendmsg(conn->sock, &msg, iov_p, iov_len,
+			     count->data_length);
+	if (ret != count->data_length) {
+		pr_err("Unexpected ret: %d send data %d\n",
+		       ret, count->data_length);
+		return -EPIPE;
 	}
+	pr_debug("ret: %d, sent data: %d\n", ret, count->data_length);
 
-	return total_tx;
+	return ret;
 }
 
 int rx_data(
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/