[PATCH 1/2] [sanboot] Prevent leaking a stack reference for

Stefan Hajnoczi stefanha at gmail.com
Tue Jan 5 03:05:32 EST 2010


"keep-san" AoE

When the "keep-san" option is used, the function is exited without
unregistering the stack allocated int13h drive.  To prevent a dangling
pointer to the stack, these structs should be heap allocated.
---
 src/arch/i386/interface/pcbios/aoeboot.c |   54 ++++++++++++++++++-----------
 1 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/src/arch/i386/interface/pcbios/aoeboot.c
b/src/arch/i386/interface/pcbios/aoeboot.c
index 8446c15..2670b15 100644
--- a/src/arch/i386/interface/pcbios/aoeboot.c
+++ b/src/arch/i386/interface/pcbios/aoeboot.c
@@ -1,11 +1,11 @@
 #include <stdint.h>
 #include <string.h>
+#include <stdlib.h>
 #include <stdio.h>
-#include <byteswap.h>
+#include <errno.h>
 #include <gpxe/aoe.h>
 #include <gpxe/ata.h>
 #include <gpxe/netdevice.h>
-#include <gpxe/settings.h>
 #include <gpxe/sanboot.h>
 #include <gpxe/abft.h>
 #include <int13.h>
@@ -13,50 +13,62 @@
 FILE_LICENCE ( GPL2_OR_LATER );

 static int aoeboot ( const char *root_path ) {
-	struct ata_device ata;
-	struct int13_drive drive;
+	struct ata_device *ata;
+	struct int13_drive *drive;
 	int rc;

-	memset ( &ata, 0, sizeof ( ata ) );
-	memset ( &drive, 0, sizeof ( drive ) );
+	ata = zalloc ( sizeof ( *ata ) );
+	if ( ! ata ) {
+		rc = -ENOMEM;
+		goto err_alloc_ata;
+	}
+	drive = zalloc ( sizeof ( *drive ) );
+	if ( ! drive ) {
+		rc = -ENOMEM;
+		goto err_alloc_drive;
+	}

 	/* FIXME: ugly, ugly hack */
 	struct net_device *netdev = last_opened_netdev();

-	if ( ( rc = aoe_attach ( &ata, netdev, root_path ) ) != 0 ) {
+	if ( ( rc = aoe_attach ( ata, netdev, root_path ) ) != 0 ) {
 		printf ( "Could not attach AoE device: %s\n",
 			 strerror ( rc ) );
-		goto error_attach;
+		goto err_attach;
 	}
-	if ( ( rc = init_atadev ( &ata ) ) != 0 ) {
+	if ( ( rc = init_atadev ( ata ) ) != 0 ) {
 		printf ( "Could not initialise AoE device: %s\n",
 			 strerror ( rc ) );
-		goto error_init;
+		goto err_init;
 	}

 	/* FIXME: ugly, ugly hack */
 	struct aoe_session *aoe =
-		container_of ( ata.backend, struct aoe_session, refcnt );
+		container_of ( ata->backend, struct aoe_session, refcnt );
 	abft_fill_data ( aoe );

-	drive.blockdev = &ata.blockdev;
+	drive->blockdev = &ata->blockdev;

-	register_int13_drive ( &drive );
-	printf ( "Registered as BIOS drive %#02x\n", drive.drive );
-	printf ( "Booting from BIOS drive %#02x\n", drive.drive );
-	rc = int13_boot ( drive.drive );
+	register_int13_drive ( drive );
+	printf ( "Registered as BIOS drive %#02x\n", drive->drive );
+	printf ( "Booting from BIOS drive %#02x\n", drive->drive );
+	rc = int13_boot ( drive->drive );
 	printf ( "Boot failed\n" );

 	/* Leave drive registered, if instructed to do so */
 	if ( keep_san() )
 		return rc;

-	printf ( "Unregistering BIOS drive %#02x\n", drive.drive );
-	unregister_int13_drive ( &drive );
+	printf ( "Unregistering BIOS drive %#02x\n", drive->drive );
+	unregister_int13_drive ( drive );

- error_init:
-	aoe_detach ( &ata );
- error_attach:
+ err_init:
+	aoe_detach ( ata );
+ err_attach:
+	free ( drive );
+ err_alloc_drive:
+	free ( ata );
+ err_alloc_ata:
 	return rc;
 }

-- 
1.6.5




More information about the gPXE mailing list