[stgt] [RESEND] [PATCH] Fix for segfault in tgtd sendtargets

[Arne Redlich]

Sorry for being that late in the game, but I think the patch should be
reverted:

Am Freitag, den 03.04.2009, 00:45 +0100 schrieb Chris Webb: 
> I have an amd64 host exporting around 70 iscsi targets (each with one LUN)
> using tgtd. Exporting the targets works fine, but when I run a 'sendtargets'
> discovery against the host, tgtd segfaults in conn_exit() because of heap
> corruption when it tries to free conn->rsp_buffer.
> 
>   (gdb) print conn->rsp.datasize                 
>   $12 = 13104
> 
> It turns out conn->rsp_buffer is being filled beyond INCOMING_BUFSIZE (8192)
> by text_key_add() because it checks for overflow of conn->tx_size instead of
> conn->rsp.datasize. The faulty test is fixed by:
> 
> Signed-off-by: Chris Webb <chris at arachsys.com>
> 
> diff -uNrp a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
> --- a/usr/iscsi/iscsid.c	2009-03-28 22:54:21.000000000 +0000
> +++ b/usr/iscsi/iscsid.c	2009-03-28 23:09:06.000000000 +0000
> @@ -160,7 +160,7 @@ void text_key_add(struct iscsi_connectio
>  	if (!conn->rsp.datasize)
>  		conn->rsp.data = conn->rsp_buffer;
>  
> -	if (conn->tx_size + len > INCOMING_BUFSIZE) {
> +	if (conn->rsp.datasize + len > INCOMING_BUFSIZE) {
>  		log_warning("Dropping key (%s=%s)", key, value);
>  		return;
>  	}
> 
> 
> However, I don't think this is the right fix, 

No, it actually is.

> because sendtargets would
> remain broken as soon as the size of the target list exceeed
> INCOMING_BUFSIZE. The following patch instead realloc()s the response buffer
> if it needs to grow to accommodate the target list:

The buffer cannot be arbitrarily reallocated. Ultimately this will lead
to a violation of the MaxRecvDSL the initiator (implicitly) declared. To
get this right the target list will have to be split into multiple PDUs,
which is AFAIK not supported.

Arne 

> Signed-off-by: Chris Webb <chris at arachsys.com>
> 
> diff -uNrp a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
> --- a/usr/iscsi/iscsid.c	2009-03-28 22:54:21.000000000 +0000
> +++ b/usr/iscsi/iscsid.c	2009-03-28 23:26:48.000000000 +0000
> @@ -160,12 +160,18 @@ void text_key_add(struct iscsi_connectio
>  	if (!conn->rsp.datasize)
>  		conn->rsp.data = conn->rsp_buffer;
>  
> -	if (conn->tx_size + len > INCOMING_BUFSIZE) {
> -		log_warning("Dropping key (%s=%s)", key, value);
> -		return;
> +	buffer = conn->rsp_buffer;
> +
> +	if (conn->rsp.datasize + len > INCOMING_BUFSIZE) {
> +		buffer = realloc(buffer, conn->rsp.datasize + len);
> +		if (buffer)
> +			conn->rsp_buffer = buffer;
> +		else {
> +			log_warning("Dropping key (%s=%s)", key, value);
> +			return;
> +		}
>  	}
>  
> -	buffer = conn->rsp_buffer;
>  	buffer += conn->rsp.datasize;
>  	conn->rsp.datasize += len;
>  
> 
> Best wishes,
> 
> Chris.
> --
> 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