[PATCH] iscsi: avoid oops when destroying unbound connection

[PATCH] iscsi: avoid oops when destroying unbound connection

[Pete Wyckoff]

Initialize the conn->item list head in a connection, used for
associating it with a session, to avoid an oops if list_del happens
before the connection is bound.  The oops is easy enough to
reproduce by calling CREATE_SESSION, CREATE_CONN, then DESTROY_CONN
(but no BIND_CONN) from userspace.

Signed-off-by: Pete Wyckoff <pw@osc.edu>
---
 drivers/scsi/libiscsi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index c542d0e..66fc239 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1477,6 +1477,7 @@ iscsi_conn_setup(struct iscsi_cls_sessio
 	INIT_LIST_HEAD(&conn->run_list);
 	INIT_LIST_HEAD(&conn->mgmt_run_list);
 	INIT_LIST_HEAD(&conn->xmitqueue);
+	INIT_LIST_HEAD(&conn->item);
 
 	/* initialize general immediate & non-immediate PDU commands queue */
 	conn->immqueue = kfifo_alloc(session->mgmtpool_max * sizeof(void*),

Re: [PATCH] iscsi: avoid oops when destroying unbound connection

[Mike Christie]

Pete Wyckoff wrote:
> Initialize the conn->item list head in a connection, used for
> associating it with a session, to avoid an oops if list_del happens
> before the connection is bound.  The oops is easy enough to
> reproduce by calling CREATE_SESSION, CREATE_CONN, then DESTROY_CONN
> (but no BIND_CONN) from userspace.
> 
> Signed-off-by: Pete Wyckoff <pw@osc.edu>
> ---
>  drivers/scsi/libiscsi.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index c542d0e..66fc239 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -1477,6 +1477,7 @@ iscsi_conn_setup(struct iscsi_cls_sessio
>  	INIT_LIST_HEAD(&conn->run_list);
>  	INIT_LIST_HEAD(&conn->mgmt_run_list);
>  	INIT_LIST_HEAD(&conn->xmitqueue);
> +	INIT_LIST_HEAD(&conn->item);
>  
>  	/* initialize general immediate & non-immediate PDU commands queue */
>  	conn->immqueue = kfifo_alloc(session->mgmtpool_max * sizeof(void*),


Thanks for the patch. We actually already fixed this by just removing
that list. It was not being used for what it was originally intended and
was getting in the way.

The fix should be in 2.6.19-rc4 when it comes out.

Re: [PATCH] iscsi: avoid oops when destroying unbound connection

[Pete Wyckoff]

michaelc@cs.wisc.edu wrote on Mon, 30 Oct 2006 12:02 -0600:
> Thanks for the patch. We actually already fixed this by just removing
> that list. It was not being used for what it was originally intended and
> was getting in the way.
> 
> The fix should be in 2.6.19-rc4 when it comes out.

That's even better.  For future reference, where is the SCSI or
iSCSI development tree?  I had guessed it might be jejb's
scsi-misc-2.6 git, but that still has conn->item, as does your
linux-2.6-iscsi git.  If I can check for fixes before sending off a
patch, it will save us all some time.

		-- Pete

Re: [PATCH] iscsi: avoid oops when destroying unbound connection

[Mike Christie]

Pete Wyckoff wrote:
> michaelc@cs.wisc.edu wrote on Mon, 30 Oct 2006 12:02 -0600:
>> Thanks for the patch. We actually already fixed this by just removing
>> that list. It was not being used for what it was originally intended and
>> was getting in the way.
>>
>> The fix should be in 2.6.19-rc4 when it comes out.
> 
> That's even better.  For future reference, where is the SCSI or
> iSCSI development tree?  I had guessed it might be jejb's
> scsi-misc-2.6 git, but that still has conn->item, as does your
> linux-2.6-iscsi git.  If I can check for fixes before sending off a

You have to checkout the iscsi branch (git checkout iscsi) after you
have downloaded the linux-2.6-iscsi.git tree.

> patch, it will save us all some time.
> 

James switches from scsi-misc to scsi-rc fixes depending on the timing
(rc4 vs not even rc1) and patch type (fix vs feature). This patch was
put in scsi-rc-fixes here

http://kernel.org/git/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=98644047916c24258fb47c3dab2bed8a44f53b83

and sent to upwards with his last patchset.