public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fc_transport: Write outside array bounds
@ 2009-07-28 12:40 Roel Kluin
  2009-07-28 13:28 ` James Smart
  0 siblings, 1 reply; 2+ messages in thread
From: Roel Kluin @ 2009-07-28 12:40 UTC (permalink / raw)
  To: James.Bottomley, linux-scsi, Andrew Morton

If it's possible to turn on all the optional attributes, there are more
attributes than the length of array i->private_host_attrs[], so the last one
will be out-of-bounds. (There is a BUG_ON there, but it's after the write,
rather than before).

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
Found with Parfait, http://research.sun.com/projects/parfait/

in fc_attach_transport()
...
count=0;
14 x SETUP_HOST_ATTRIBUTE_RD()
if (ft->vport_create)
	2 x SETUP_HOST_ATTRIBUTE_RD_NS()
1 x SETUP_HOST_ATTRIBUTE_RW()
1 - 4 x SETUP_PRIVATE_HOST_ATTRIBUTE_RW()

all these definitions set private_host_attrs[count] (21 elements)
and increase the index count thereafter.

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 292c02f..8092e56 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2123,8 +2123,12 @@ fc_attach_transport(struct fc_function_template *ft)
 		SETUP_PRIVATE_HOST_ATTRIBUTE_RW(issue_lip);
 	if (ft->vport_create)
 		SETUP_PRIVATE_HOST_ATTRIBUTE_RW(vport_create);
-	if (ft->vport_delete)
-		SETUP_PRIVATE_HOST_ATTRIBUTE_RW(vport_delete);
+	if (ft->vport_delete) {
+		if(count < FC_HOST_NUM_ATTRS)
+			SETUP_PRIVATE_HOST_ATTRIBUTE_RW(vport_delete);
+		else
+			count++;
+	}
 
 	BUG_ON(count > FC_HOST_NUM_ATTRS);
 

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] fc_transport: Write outside array bounds
  2009-07-28 12:40 [PATCH] fc_transport: Write outside array bounds Roel Kluin
@ 2009-07-28 13:28 ` James Smart
  0 siblings, 0 replies; 2+ messages in thread
From: James Smart @ 2009-07-28 13:28 UTC (permalink / raw)
  To: Roel Kluin
  Cc: James.Bottomley@HansenPartnership.com, linux-scsi@vger.kernel.org,
	Andrew Morton

This shouldn't be the case, and if it is, isn't the right way to handle it.
It may simply be someone forgot to update FC_HOST_NUM_ATTRS. I'll double
check and get back shortly.

-- james s


Roel Kluin wrote:
> If it's possible to turn on all the optional attributes, there are more
> attributes than the length of array i->private_host_attrs[], so the last one
> will be out-of-bounds. (There is a BUG_ON there, but it's after the write,
> rather than before).
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> Found with Parfait, http://research.sun.com/projects/parfait/
> 
> in fc_attach_transport()
> ...
> count=0;
> 14 x SETUP_HOST_ATTRIBUTE_RD()
> if (ft->vport_create)
> 	2 x SETUP_HOST_ATTRIBUTE_RD_NS()
> 1 x SETUP_HOST_ATTRIBUTE_RW()
> 1 - 4 x SETUP_PRIVATE_HOST_ATTRIBUTE_RW()
> 
> all these definitions set private_host_attrs[count] (21 elements)
> and increase the index count thereafter.
> 
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index 292c02f..8092e56 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -2123,8 +2123,12 @@ fc_attach_transport(struct fc_function_template *ft)
>  		SETUP_PRIVATE_HOST_ATTRIBUTE_RW(issue_lip);
>  	if (ft->vport_create)
>  		SETUP_PRIVATE_HOST_ATTRIBUTE_RW(vport_create);
> -	if (ft->vport_delete)
> -		SETUP_PRIVATE_HOST_ATTRIBUTE_RW(vport_delete);
> +	if (ft->vport_delete) {
> +		if(count < FC_HOST_NUM_ATTRS)
> +			SETUP_PRIVATE_HOST_ATTRIBUTE_RW(vport_delete);
> +		else
> +			count++;
> +	}
>  
>  	BUG_ON(count > FC_HOST_NUM_ATTRS);
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2009-07-28 13:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-28 12:40 [PATCH] fc_transport: Write outside array bounds Roel Kluin
2009-07-28 13:28 ` James Smart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox