[stgt] [PATCH] Fix leak of task->data

[FUJITA Tomonori]

On Sun, 11 Aug 2013 11:04:01 -0700
Andy Grover <agrover at redhat.com> wrote:

> Steps to reproduce: In initiator, dd to the volume and bring the initiator
> interface up and down repeatedly. Valgrind of tgtd confirms the leak and
> the fix.
> 
> If a connection is terminated while tasks are not yet fully received,
> task->data will have been allocated but not referred to by a cmd in-
> or out-buffer, which happens in iscsi_target_cmd_queue only after the
> entire command is received. When freeing tasks for a closed connection,
> ensure task->data is freed if it isn't already freed by the pointer
> having been copied to scmd in or out buffer.

Nice catch! Thanks a lot!

> It might be nicer set task->data to NULL when its reference is copied,
> and then we could unconditionally call free() on it in iscsi_free_task,

Yeah.

> but task->data is referred to in many other places in iscsid.c. Maybe
> those places should refer to the in/out buffers instead?

Yeah, it would be nice to clean up the above area. The problem is, the
current code assmes that in/out buffers is for SCSI commands, and
iscsid.c needs to handle non SCSI command buffer (e.g. noop) too.

> Reported-by: Tomoaki Nishimura <t-nishimura at hf.jp.nec.com>
> Signed-off-by: Andy Grover <agrover at redhat.com>
> ---
>  usr/iscsi/iscsid.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
> index 005bac5..9bae331 100644
> --- a/usr/iscsi/iscsid.c
> +++ b/usr/iscsi/iscsid.c
> @@ -1227,6 +1227,10 @@ void iscsi_free_task(struct iscsi_task *task)
>  	conn->tp->free_data_buf(conn, scsi_get_in_buffer(&task->scmd));
>  	conn->tp->free_data_buf(conn, scsi_get_out_buffer(&task->scmd));
>  
> +	if ((task->data != scsi_get_in_buffer(&task->scmd)) ||
> +	    (task->data != scsi_get_out_buffer(&task->scmd)))
> +		conn->tp->free_data_buf(conn, task->data);
> +
>  	conn->tp->free_task(task);
>  	conn_put(conn);
>  }
> -- 
> 1.7.1
> 
> --
> 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
--
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