[stgt] [RESEND] [PATCH] Fix for segfault in tgtd sendtargets

[Chris Webb]

I have an amd64 host exporting around 70 iscsi targets (each with one LUN)
using tgtd. Exporting the targets works fine, but when I run a 'sendtargets'
discovery against the host, tgtd segfaults in conn_exit() because of heap
corruption when it tries to free conn->rsp_buffer.

  (gdb) print conn->rsp.datasize                 
  $12 = 13104

It turns out conn->rsp_buffer is being filled beyond INCOMING_BUFSIZE (8192)
by text_key_add() because it checks for overflow of conn->tx_size instead of
conn->rsp.datasize. The faulty test is fixed by:

Signed-off-by: Chris Webb <chris at arachsys.com>

diff -uNrp a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
--- a/usr/iscsi/iscsid.c	2009-03-28 22:54:21.000000000 +0000
+++ b/usr/iscsi/iscsid.c	2009-03-28 23:09:06.000000000 +0000
@@ -160,7 +160,7 @@ void text_key_add(struct iscsi_connectio
 	if (!conn->rsp.datasize)
 		conn->rsp.data = conn->rsp_buffer;
 
-	if (conn->tx_size + len > INCOMING_BUFSIZE) {
+	if (conn->rsp.datasize + len > INCOMING_BUFSIZE) {
 		log_warning("Dropping key (%s=%s)", key, value);
 		return;
 	}


However, I don't think this is the right fix, because sendtargets would
remain broken as soon as the size of the target list exceeed
INCOMING_BUFSIZE. The following patch instead realloc()s the response buffer
if it needs to grow to accommodate the target list:

Signed-off-by: Chris Webb <chris at arachsys.com>

diff -uNrp a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
--- a/usr/iscsi/iscsid.c	2009-03-28 22:54:21.000000000 +0000
+++ b/usr/iscsi/iscsid.c	2009-03-28 23:26:48.000000000 +0000
@@ -160,12 +160,18 @@ void text_key_add(struct iscsi_connectio
 	if (!conn->rsp.datasize)
 		conn->rsp.data = conn->rsp_buffer;
 
-	if (conn->tx_size + len > INCOMING_BUFSIZE) {
-		log_warning("Dropping key (%s=%s)", key, value);
-		return;
+	buffer = conn->rsp_buffer;
+
+	if (conn->rsp.datasize + len > INCOMING_BUFSIZE) {
+		buffer = realloc(buffer, conn->rsp.datasize + len);
+		if (buffer)
+			conn->rsp_buffer = buffer;
+		else {
+			log_warning("Dropping key (%s=%s)", key, value);
+			return;
+		}
 	}
 
-	buffer = conn->rsp_buffer;
 	buffer += conn->rsp.datasize;
 	conn->rsp.datasize += len;
 

Best wishes,

Chris.
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html