[PATCH] fix use-after-free in sbp2.c

[PATCH] fix use-after-free in sbp2.c

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 3013 bytes --]

hi :)

when using some checking code (CONFIG_DEBUG_{SLAB,SPINLOCK_SLEEP},
sbp2 fails to log in into my external hd enclosure.

that is because sbp2_agent_reset sends a packet and waits
for its delivery.
however, the function used to create the packet activates
auto-destruct of the packet via hpsb_set_packet_complete_task.
thus, the semaphore used for synchronization is destroyed
while the sending task is waiting.

the following patch (against -test11) fixes sbp2 for me



Index: work/drivers/ieee1394/sbp2.c
===================================================================
--- work.orig/drivers/ieee1394/sbp2.c	2003-11-28 23:49:11.000000000 +0100
+++ work/drivers/ieee1394/sbp2.c	2003-12-01 02:27:40.000000000 +0100
@@ -385,14 +387,18 @@
         if (!packet)
                 return NULL;
 
-	hpsb_set_packet_complete_task(packet, (void (*)(void*))sbp2_free_packet,
-				      packet);
-
 	hpsb_node_fill_packet(ne, packet);
 
 	return packet;
 }
 
+static void
+sbp2util_autoclean_packet(struct hpsb_packet *packet)
+{
+	hpsb_set_packet_complete_task(packet, (void (*)(void*))sbp2_free_packet,
+				      packet);
+}
+
 
 /*
  * This function is called to create a pool of command orbs used for
@@ -1763,6 +1769,7 @@
 	if (wait) {
 		down(&packet->state_change);
 		down(&packet->state_change);
+		sbp2_free_packet(packet); 
 	}
 
 	/*
@@ -2063,6 +2070,7 @@
 				SBP2_ERR("sbp2util_allocate_write_packet failed");
 				return(-ENOMEM);
 			}
+			sbp2util_autoclean_packet(packet);
 		
 			packet->data[0] = ORB_SET_NODE_ID(hi->host->node_id);
 			packet->data[1] = command->command_orb_dma;
@@ -2113,6 +2121,7 @@
 				SBP2_ERR("sbp2util_allocate_write_packet failed");
 				return(-ENOMEM);
 			}
+			sbp2util_autoclean_packet(packet);
 
 			SBP2_ORB_DEBUG("ring doorbell, command orb %p", command_orb);


the following small part is needed to make sbp2 compile with
debug enabled

Index: work/drivers/ieee1394/sbp2.c
===================================================================
--- work.orig/drivers/ieee1394/sbp2.c	2003-11-28 23:49:11.000000000 +0100
+++ work/drivers/ieee1394/sbp2.c	2003-12-01 02:27:40.000000000 +0100
@@ -603,7 +609,7 @@
 	struct unit_directory *ud;
 	struct sbp2scsi_host_info *hi;
 
-	SBP2_DEBUG(__FUNCTION__);
+	SBP2_DEBUG("%s()", __FUNCTION__);
 
 	ud = container_of(dev, struct unit_directory, device);
 
@@ -628,7 +634,7 @@
 	struct unit_directory *ud;
 	struct scsi_id_instance_data *scsi_id;
 
-	SBP2_DEBUG(__FUNCTION__);
+	SBP2_DEBUG("%s()", __FUNCTION__);
 
 	ud = container_of(dev, struct unit_directory, device);
 	scsi_group = ud->device.driver_data;
 

-- 
CU,		  / Friedrich-Alexander University Erlangen, Germany
Martin Waitz	//  Department of Computer Science 3       _________
______________/// - - - - - - - - - - - - - - - - - - - - ///
dies ist eine manuell generierte mail, sie beinhaltet    //
tippfehler und ist auch ohne grossbuchstaben gueltig.   /

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] fix use-after-free in sbp2.c

[Ben Collins]

On Mon, Dec 01, 2003 at 10:02:12PM +0100, Martin Waitz wrote:
> hi :)
> 
> when using some checking code (CONFIG_DEBUG_{SLAB,SPINLOCK_SLEEP},
> sbp2 fails to log in into my external hd enclosure.
> 
> that is because sbp2_agent_reset sends a packet and waits
> for its delivery.
> however, the function used to create the packet activates
> auto-destruct of the packet via hpsb_set_packet_complete_task.
> thus, the semaphore used for synchronization is destroyed
> while the sending task is waiting.
> 
> the following patch (against -test11) fixes sbp2 for me

Could you test what's in our repo first? We've already fixed this, but
it was done in a way different way than you did (we got rid of the
semaphore).

-- 
Debian     - http://www.debian.org/
Linux 1394 - http://www.linux1394.org/
Subversion - http://subversion.tigris.org/
WatchGuard - http://www.watchguard.com/

Re: [PATCH] fix use-after-free in sbp2.c

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

hi :)

On Tue, Dec 02, 2003 at 06:31:25PM -0500, Ben Collins wrote:
> Could you test what's in our repo first?
will do next time ;)

> We've already fixed this, but it was done in a way different way than
> you did (we got rid of the semaphore).
well thats even better

your code is much cleaner,
i just wanted to make it work with minimal changes as
i didn't knew it was already fixed.

thanks for the great 1394 work! :)

-- 
CU,		  / Friedrich-Alexander University Erlangen, Germany
Martin Waitz	//  Department of Computer Science 3       _________
______________/// - - - - - - - - - - - - - - - - - - - - ///
dies ist eine manuell generierte mail, sie beinhaltet    //
tippfehler und ist auch ohne grossbuchstaben gueltig.   /

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] fix use-after-free in sbp2.c

[Bob Gill]

Hi! :)
 I applied your patch (by hand, but the patch is small) to
2.6.0-test11.  Its working fine here.  The debug/spinlock errors are
gone and all sbp2 devices show up in /proc/scsi/scsi.  


Cheers,
-- 
Bob Gill <gillb4@telusplanet.net>