[Stgt-devel] [PATCH] task data leak

[FUJITA Tomonori]

From: Pete Wyckoff <pw at osc.edu>
Subject: [Stgt-devel] [PATCH] task data leak
Date: Fri, 7 Dec 2007 15:23:53 -0500

> iscsi_scsi_cmd_rx_start always allocates a buffer of 4096 to accommodate
> assumptions in spc, sbc, etc.  Even when a SCSI command asks for data
> length of zero, task->data is allocated to 4096.  However this is never
> assigned as in or out buf on scmd.  Thus never freed.
> 
> This works around that by freeing an orphaned task->data.  Again, though,
> the better solution is to fix up all the little functions like inquiry
> that cause this situation in the first place.
> 
> Signed-off-by: Pete Wyckoff <pw at osc.edu>
> ---
>  usr/iscsi/iscsid.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
> index 1e0172a..ab1999d 100644
> --- a/usr/iscsi/iscsid.c
> +++ b/usr/iscsi/iscsid.c
> @@ -1024,6 +1024,17 @@ void iscsi_free_task(struct iscsi_task *task)
>  {
>  	struct iscsi_connection *conn = task->conn;
>  
> +	/*
> +	 * Catch case when data_len is zero but pushed up to 4096
> +	 * to work around spc allocation assumption, but then later
> +	 * determined to be DATA_NONE and not used as either in or
> +	 * out buffer.
> +	 */
> +	if (task->data &&
> +	    task->data != scsi_get_in_buffer(&task->scmd) &&
> +	    task->data != scsi_get_out_buffer(&task->scmd))
> +		conn->tp->free_data_buf(conn, task->data);
> +

Nice catch, thanks.

How about this patch?

diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index 1e0172a..e27c74c 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -1435,7 +1435,7 @@ static int iscsi_scsi_cmd_rx_start(struct iscsi_connection *conn)
 	/*
 	 * fix spc, sbc, etc. they assume that buffer is long enough.
 	 */
-	if (data_len < 4096)
+	if (data_len && data_len < 4096)
 		data_len = 4096;
 
 	ext_len = ahs_len ? sizeof(req->cdb) + ahs_len : 0;