Skip to contentSkip to conversation

fix use-after-free in sbp2.c

PATCH In review Martin Waitz 2 participants 3 messages Dec 1, 2003 – Dec 3, 2003

lore
Share & tools
lore

Martin Waitz

author
#0 lore raw

PATCH 1/1 fix use-after-free in sbp2.c

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

drivers/ieee1394/sbp2.c +10 -3
Index: work/drivers/ieee1394/sbp2.c
===================================================================
@@ -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

drivers/ieee1394/sbp2.c +2 -2
Index: work/drivers/ieee1394/sbp2.c
===================================================================
@@ -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;
Signature · 5 lines
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.   /
#1 lore raw
Quoted 5 lines · Martin Waitz

hi :)

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

Quoted 8 lines

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).

Signature · 4 lines
Debian     - http://www.debian.org/
Linux 1394 - http://www.linux1394.org/
Subversion - http://subversion.tigris.org/
WatchGuard - http://www.watchguard.com/
author
#2 lore raw

hi :)

Quoted 1 line · Ben Collins

Could you test what's in our repo first?

will do next time ;)

Quoted 2 lines

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! :)

Signature · 5 lines
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.   /