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