On Sun, 11 Aug 2013 11:04:01 -0700 Andy Grover <agrover@xxxxxxxxxx> 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@xxxxxxxxxxxxx> > Signed-off-by: Andy Grover <agrover@xxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
