Provide individually cached constructed copies of DHCP packets via

PXENV_GET_CACHED_INFO.  If we dont do this, Altiris' NBP screws up; it
relies on being able to grab pointers to each of the three packets and
then read them at will later.

src/interface/pxe/pxe_preboot.c

@@ -32,22 +32,42 @@
 #include <gpxe/netdevice.h>
 #include <gpxe/isapnp.h>
 #include <gpxe/init.h>
+#include <gpxe/if_ether.h>
 #include <basemem_packet.h>
 #include "pxe.h"
 #include "pxe_call.h"
 
-/** Filename used for last TFTP request
- *
- * This is a bug-for-bug compatibility hack needed in order to work
- * with Microsoft Remote Installation Services (RIS).  The filename
- * used in a call to PXENV_RESTART_TFTP must be returned as the DHCP
- * filename in subsequent calls to PXENV_GET_CACHED_INFO.
- */
-static char *pxe_ris_filename = NULL;
-
 /* Avoid dragging in isapnp.o unnecessarily */
 uint16_t isapnp_read_port;
 
+/** Zero-based versions of PXENV_GET_CACHED_INFO::PacketType */
+enum pxe_cached_info_indices {
+	CACHED_INFO_DHCPDISCOVER = ( PXENV_PACKET_TYPE_DHCP_DISCOVER - 1 ),
+	CACHED_INFO_DHCPACK = ( PXENV_PACKET_TYPE_DHCP_ACK - 1 ),
+	CACHED_INFO_BINL = ( PXENV_PACKET_TYPE_CACHED_REPLY - 1 ),
+	NUM_CACHED_INFOS
+};
+
+/** A cached DHCP packet */
+union pxe_cached_info {
+	struct dhcphdr dhcphdr;
+	char raw[ETH_FRAME_LEN];
+};
+
+/* The case in which the caller doesn't supply a buffer is really
+ * awkward to support given that we have multiple sources of options,
+ * and that we don't actually store the DHCP packets.  (We may not
+ * even have performed DHCP; we may have obtained all configuration
+ * from non-volatile stored options or from the command line.)
+ *
+ * Some NBPs rely on the buffers we provide being persistent, so we
+ * can't just use the temporary packet buffer.  4.5kB of base memory
+ * always wasted just because some clients are too lazy to provide
+ * their own buffers...
+ */
+static union pxe_cached_info __bss16_array ( cached_info, [NUM_CACHED_INFOS] );
+#define cached_info __use_data16 ( cached_info )
+
 /**
  * UNLOAD BASE CODE STACK
  *
@@ -72,8 +92,8 @@ PXENV_EXIT_t pxenv_get_cached_info ( struct s_PXENV_GET_CACHED_INFO
 	int ( * dhcp_packet_creator ) ( struct net_device *, int,
 					struct dhcp_option_block *, void *,
 					size_t, struct dhcp_packet * );
+	unsigned int idx;
 	unsigned int msgtype;
-	void *data = NULL;
 	size_t len;
 	userptr_t buffer;
 	int rc;
@@ -83,88 +103,86 @@ PXENV_EXIT_t pxenv_get_cached_info ( struct s_PXENV_GET_CACHED_INFO
 	DBG ( " to %04x:%04x+%x", get_cached_info->Buffer.segment,
 	      get_cached_info->Buffer.offset, get_cached_info->BufferSize );
 
-	/* The case in which the caller doesn't supply a buffer is
-	 * really awkward to support given that we have multiple
-	 * sources of options, and that we don't actually store the
-	 * DHCP packets.  (We may not even have performed DHCP; we may
-	 * have obtained all configuration from non-volatile stored
-	 * options or from the command line.)  We provide the caller
-	 * with our base-memory temporary packet buffer and construct
-	 * the packet in there.
-	 *
-	 * To add to the fun, Intel decided at some point in the
-	 * evolution of the PXE specification to add the BufferLimit
-	 * field, which we are meant to fill in with the length of our
-	 * packet buffer, so that the caller can safely modify the
-	 * boot server reply packet stored therein.  However, this
-	 * field was not present in earlier versions of the PXE spec,
-	 * and there is at least one PXE NBP (Altiris) which allocates
-	 * only exactly enough space for this earlier, shorter version
-	 * of the structure.  If we actually fill in the BufferLimit
-	 * field, we therefore risk trashing random areas of the
-	 * caller's memory.  If we *don't* fill it in, then the caller
-	 * is at liberty to assume that whatever random value happened
-	 * to be in that location represents the length of the buffer
-	 * we've just passed back to it.
-	 *
-	 * Since older PXE stacks won't fill this field in anyway,
-	 * it's probably safe to assume that no callers actually rely
-	 * on it, so we choose to not fill it in.
-	 */
+	/* Sanity check */
+        idx = ( get_cached_info->PacketType - 1 );
+	if ( idx >= ( sizeof ( cached_info ) / sizeof ( cached_info[0] ) ) ) {
+		DBG ( " bad PacketType" );
+		goto err;
+	}
+
+	/* Construct cached version of packet, if not already constructed. */
+	if ( ! cached_info[idx].dhcphdr.op ) {
+		/* Construct DHCP packet */
+		if ( get_cached_info->PacketType ==
+		     PXENV_PACKET_TYPE_DHCP_DISCOVER ) {
+			dhcp_packet_creator = create_dhcp_request;
+			msgtype = DHCPDISCOVER;
+		} else {
+			dhcp_packet_creator = create_dhcp_response;
+			msgtype = DHCPACK;
+		}
+		if ( ( rc = dhcp_packet_creator ( pxe_netdev, msgtype,
+						  NULL, &cached_info[idx],
+						  sizeof ( cached_info[idx] ),
+						  &dhcppkt ) ) != 0 ) {
+			DBG ( " failed to build packet" );
+			goto err;
+		}
+	}
+
 	len = get_cached_info->BufferSize;
 	if ( len == 0 ) {
-		len = sizeof ( basemem_packet );
+		/* Point client at our cached buffer.
+		 *
+		 * To add to the fun, Intel decided at some point in
+		 * the evolution of the PXE specification to add the
+		 * BufferLimit field, which we are meant to fill in
+		 * with the length of our packet buffer, so that the
+		 * caller can safely modify the boot server reply
+		 * packet stored therein.  However, this field was not
+		 * present in earlier versions of the PXE spec, and
+		 * there is at least one PXE NBP (Altiris) which
+		 * allocates only exactly enough space for this
+		 * earlier, shorter version of the structure.  If we
+		 * actually fill in the BufferLimit field, we
+		 * therefore risk trashing random areas of the
+		 * caller's memory.  If we *don't* fill it in, then
+		 * the caller is at liberty to assume that whatever
+		 * random value happened to be in that location
+		 * represents the length of the buffer we've just
+		 * passed back to it.
+		 *
+		 * Since older PXE stacks won't fill this field in
+		 * anyway, it's probably safe to assume that no
+		 * callers actually rely on it, so we choose to not
+		 * fill it in.
+		 */
 		get_cached_info->Buffer.segment = rm_ds;
 		get_cached_info->Buffer.offset =
-			( unsigned int ) ( & __from_data16 ( basemem_packet ) );
-		DBG ( " using %04x:%04x+'%x'", get_cached_info->Buffer.segment,
+			( unsigned ) ( & __from_data16 ( cached_info[idx] ) );
+		get_cached_info->BufferSize = sizeof ( cached_info[idx] );
+		DBG ( " returning %04x:%04x+%04x['%x']",
+		      get_cached_info->Buffer.segment,
 		      get_cached_info->Buffer.offset,
+		      get_cached_info->BufferSize,
 		      get_cached_info->BufferLimit );
-	}
-
-	/* Allocate space for temporary copy */
-	data = malloc ( len );
-	if ( ! data ) {
-		DBG ( " out of memory" );
-		goto err;
-	}
-
-	/* Construct DHCP packet */
-	if ( get_cached_info->PacketType == PXENV_PACKET_TYPE_DHCP_DISCOVER ) {
-		dhcp_packet_creator = create_dhcp_request;
-		msgtype = DHCPDISCOVER;
 	} else {
-		dhcp_packet_creator = create_dhcp_response;
-		msgtype = DHCPACK;
-	}
-	if ( ( rc = dhcp_packet_creator ( pxe_netdev, msgtype, NULL,
-					  data, len, &dhcppkt ) ) != 0 ) {
-		DBG ( " failed to build packet" );
-		goto err;
+		/* Copy packet to client buffer */
+		if ( len < sizeof ( cached_info[idx] ) ) {
+			DBG ( " buffer too short" );
+			goto err;
+		}
+		buffer = real_to_user ( get_cached_info->Buffer.segment,
+					get_cached_info->Buffer.offset );
+		copy_to_user ( buffer, 0, &cached_info[idx],
+			       sizeof ( cached_info[idx] ) );
+		get_cached_info->BufferSize = sizeof ( cached_info[idx] );
 	}
 
-	/* Overwrite filename to work around Microsoft RIS bug */
-	if ( pxe_ris_filename ) {
-		DBG ( " applying RIS hack" );
-		strncpy ( dhcppkt.dhcphdr->file, pxe_ris_filename,
-			  sizeof ( dhcppkt.dhcphdr->file ) );
-	}
-
-	/* Copy packet to client buffer */
-	buffer = real_to_user ( get_cached_info->Buffer.segment,
-				get_cached_info->Buffer.offset );
-	len = dhcppkt.len;
-	DBG ( " length %x", len );
-	copy_to_user ( buffer, 0, data, len );
-	get_cached_info->BufferSize = len;
-
-	free ( data );
 	get_cached_info->Status = PXENV_STATUS_SUCCESS;
 	return PXENV_EXIT_SUCCESS;
 
  err:
-	if ( data )
-		free ( data );
 	get_cached_info->Status = PXENV_STATUS_OUT_OF_RESOURCES;
 	return PXENV_EXIT_FAILURE;
 }
@@ -179,13 +197,18 @@ PXENV_EXIT_t pxenv_restart_tftp ( struct s_PXENV_TFTP_READ_FILE
 
 	DBG ( "PXENV_RESTART_TFTP " );
 
-	/* Work around Microsoft RIS bug */
-	free ( pxe_ris_filename );
-	pxe_ris_filename = strdup ( ( char * ) restart_tftp->FileName );
-	if ( ! pxe_ris_filename ) {
-		restart_tftp->Status = PXENV_STATUS_OUT_OF_RESOURCES;
-		return PXENV_EXIT_FAILURE;
-	}
+	/* This is a bug-for-bug compatibility hack needed in order to
+	 * work with Microsoft Remote Installation Services (RIS).
+	 * The filename used in a call to PXENV_RESTART_TFTP must be
+	 * returned as the DHCP filename in subsequent calls to
+	 * PXENV_GET_CACHED_INFO.
+	 */
+	memcpy ( cached_info[CACHED_INFO_DHCPACK].dhcphdr.file,
+		 restart_tftp->FileName,
+		 sizeof ( cached_info[CACHED_INFO_DHCPACK].dhcphdr.file ) );
+	memcpy ( cached_info[CACHED_INFO_BINL].dhcphdr.file,
+		 restart_tftp->FileName,
+		 sizeof ( cached_info[CACHED_INFO_BINL].dhcphdr.file ) );
 
 	/* Words cannot describe the complete mismatch between the PXE
 	 * specification and any possible version of reality...