From 60b5532cfc1393a8d34cece1b49f953bd72c303c Mon Sep 17 00:00:00 2001
From: Michael Brown
Date: Thu, 22 Dec 2022 14:59:29 +0000
Subject: [cachedhcp] Include VLAN tag in filter for applying cached DHCPACK

When chainloading iPXE from a VLAN device, the MAC address within the
cached DHCPACK will match the MAC address of the trunk device created
by iPXE, and the cached DHCPACK will then end up being erroneously
applied to the trunk device.  This tends to break outbound IPv4
routing, since both the trunk and VLAN devices will have the same
assigned IPv4 address.

Fix by recording the VLAN tag along with the cached DHCPACK, and
treating the VLAN tag as part of the filter used to match the cached
DHCPACK against candidate network devices.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 src/core/cachedhcp.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

(limited to 'src/core')

diff --git a/src/core/cachedhcp.c b/src/core/cachedhcp.c
index c4ca46e3a..ef2146626 100644
--- a/src/core/cachedhcp.c
+++ b/src/core/cachedhcp.c
@@ -29,6 +29,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <ipxe/dhcppkt.h>
 #include <ipxe/init.h>
 #include <ipxe/netdevice.h>
+#include <ipxe/vlan.h>
 #include <ipxe/cachedhcp.h>
 
 /** @file
@@ -43,6 +44,8 @@ struct cached_dhcp_packet {
 	const char *name;
 	/** DHCP packet (if any) */
 	struct dhcp_packet *dhcppkt;
+	/** VLAN tag (if applicable) */
+	unsigned int vlan;
 };
 
 /** Cached DHCPACK */
@@ -136,15 +139,26 @@ static int cachedhcp_apply ( struct cached_dhcp_packet *cache,
 		 * matches this network device.
 		 */
 		if ( memcmp ( ll_addr, chaddr, ll_addr_len ) != 0 ) {
-			DBGC ( colour, "CACHEDHCP %s does not match %s\n",
-			       cache->name, netdev->name );
+			DBGC ( colour, "CACHEDHCP %s %s does not match %s\n",
+			       cache->name, ll_protocol->ntoa ( chaddr ),
+			       netdev->name );
+			return 0;
+		}
+
+		/* Do nothing unless cached packet's VLAN tag matches
+		 * this network device.
+		 */
+		if ( vlan_tag ( netdev ) != cache->vlan ) {
+			DBGC ( colour, "CACHEDHCP %s VLAN %d does not match "
+			       "%s\n", cache->name, cache->vlan,
+			       netdev->name );
 			return 0;
 		}
-		DBGC ( colour, "CACHEDHCP %s is for %s\n",
-		       cache->name, netdev->name );
 
 		/* Use network device's settings block */
 		settings = netdev_settings ( netdev );
+		DBGC ( colour, "CACHEDHCP %s is for %s\n",
+		       cache->name, netdev->name );
 	}
 
 	/* Register settings */
@@ -165,12 +179,13 @@ static int cachedhcp_apply ( struct cached_dhcp_packet *cache,
  * Record cached DHCP packet
  *
  * @v cache		Cached DHCP packet
+ * @v vlan		VLAN tag, if any
  * @v data		DHCPACK packet buffer
  * @v max_len		Maximum possible length
  * @ret rc		Return status code
  */
-int cachedhcp_record ( struct cached_dhcp_packet *cache, userptr_t data,
-		       size_t max_len ) {
+int cachedhcp_record ( struct cached_dhcp_packet *cache, unsigned int vlan,
+		       userptr_t data, size_t max_len ) {
 	struct dhcp_packet *dhcppkt;
 	struct dhcp_packet *tmp;
 	struct dhcphdr *dhcphdr;
@@ -225,6 +240,7 @@ int cachedhcp_record ( struct cached_dhcp_packet *cache, userptr_t data,
 	DBGC ( colour, "CACHEDHCP %s at %#08lx+%#zx/%#zx\n", cache->name,
 	       user_to_phys ( data, 0 ), len, max_len );
 	cache->dhcppkt = dhcppkt;
+	cache->vlan = vlan;
 
 	return 0;
 }
-- 
cgit v1.2.3-55-g7522


From 7147532c3fbf9a7061e74549f6f920a91ca9a80d Mon Sep 17 00:00:00 2001
From: Michael Brown
Date: Thu, 22 Dec 2022 15:12:34 +0000
Subject: [cachedhcp] Retain cached DHCPACK after startup if not already
 consumed

We currently free an unclaimed cached DHCPACK immediately after
startup, in order to free up memory.  This prevents the cached DHCPACK
from being applied to a device that is created after startup, such as
a VLAN device created via the "vcreate" command.

Retain any unclaimed DHCPACK after startup to allow it to be matched
against (and applied to) any device that gets created at runtime.
Free the DHCPACK during shutdown if it still remains unclaimed, in
order to exit with memory cleanly freed.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 src/core/cachedhcp.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

(limited to 'src/core')

diff --git a/src/core/cachedhcp.c b/src/core/cachedhcp.c
index ef2146626..60213f02d 100644
--- a/src/core/cachedhcp.c
+++ b/src/core/cachedhcp.c
@@ -246,31 +246,49 @@ int cachedhcp_record ( struct cached_dhcp_packet *cache, unsigned int vlan,
 }
 
 /**
- * Cached DHCPACK startup function
+ * Cached DHCP packet startup function
  *
  */
 static void cachedhcp_startup ( void ) {
 
 	/* Apply cached ProxyDHCPOFFER, if any */
 	cachedhcp_apply ( &cached_proxydhcp, NULL );
+	cachedhcp_free ( &cached_proxydhcp );
 
 	/* Apply cached PXEBSACK, if any */
 	cachedhcp_apply ( &cached_pxebs, NULL );
+	cachedhcp_free ( &cached_pxebs );
 
-	/* Free any remaining cached packets */
+	/* Report unclaimed DHCPACK, if any.  Do not free yet, since
+	 * it may still be claimed by a dynamically created device
+	 * such as a VLAN device.
+	 */
 	if ( cached_dhcpack.dhcppkt ) {
 		DBGC ( colour, "CACHEDHCP %s unclaimed\n",
 		       cached_dhcpack.name );
 	}
+}
+
+/**
+ * Cached DHCP packet shutdown function
+ *
+ * @v booting		System is shutting down for OS boot
+ */
+static void cachedhcp_shutdown ( int booting __unused ) {
+
+	/* Free cached DHCPACK, if any */
+	if ( cached_dhcpack.dhcppkt ) {
+		DBGC ( colour, "CACHEDHCP %s never claimed\n",
+		       cached_dhcpack.name );
+	}
 	cachedhcp_free ( &cached_dhcpack );
-	cachedhcp_free ( &cached_proxydhcp );
-	cachedhcp_free ( &cached_pxebs );
 }
 
 /** Cached DHCPACK startup function */
 struct startup_fn cachedhcp_startup_fn __startup_fn ( STARTUP_LATE ) = {
 	.name = "cachedhcp",
 	.startup = cachedhcp_startup,
+	.shutdown = cachedhcp_shutdown,
 };
 
 /**
-- 
cgit v1.2.3-55-g7522


From 76a286530a8b5bdbab81c3851b851dea2da32114 Mon Sep 17 00:00:00 2001
From: Michael Brown
Date: Mon, 13 Feb 2023 20:40:42 +0000
Subject: [image] Check delimiters when parsing command-line key-value
 arguments

The Linux kernel bzImage image format and the CPIO archive constructor
will parse the image command line for certain arguments of the form
"key=value".  This parsing is currently implemented using strstr() in
a way that can cause a false positive suffix match.  For example, a
command line containing "highmem=<n>" would erroneously be treated as
containing a value for "mem=<n>".

Fix by centralising the logic used for parsing such arguments, and
including a check that the argument immediately follows a whitespace
delimiter (or is at the start of the string).

Reported-by: Filippo Giunchedi <filippo@esaurito.net>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 src/arch/x86/image/bzimage.c | 37 ++++++++++++++++---------------------
 src/core/cpio.c              |  9 ++-------
 src/core/image.c             | 31 +++++++++++++++++++++++++++++++
 src/include/ipxe/image.h     |  1 +
 4 files changed, 50 insertions(+), 28 deletions(-)

(limited to 'src/core')

diff --git a/src/arch/x86/image/bzimage.c b/src/arch/x86/image/bzimage.c
index 769e0c9d8..b40bb2d07 100644
--- a/src/arch/x86/image/bzimage.c
+++ b/src/arch/x86/image/bzimage.c
@@ -247,19 +247,17 @@ static void bzimage_update_header ( struct image *image,
  *
  * @v image		bzImage file
  * @v bzimg		bzImage context
- * @v cmdline		Kernel command line
  * @ret rc		Return status code
  */
 static int bzimage_parse_cmdline ( struct image *image,
-				   struct bzimage_context *bzimg,
-				   char *cmdline ) {
+				   struct bzimage_context *bzimg ) {
+	const char *vga;
+	const char *mem;
 	char *sep;
-	char *vga;
-	char *mem;
+	char *end;
 
 	/* Look for "vga=" */
-	if ( ( vga = strstr ( cmdline, "vga=" ) ) ) {
-		vga += 4;
+	if ( ( vga = image_argument ( image, "vga=" ) ) ) {
 		sep = strchr ( vga, ' ' );
 		if ( sep )
 			*sep = '\0';
@@ -270,10 +268,10 @@ static int bzimage_parse_cmdline ( struct image *image,
 		} else if ( strcmp ( vga, "ask" ) == 0 ) {
 			bzimg->vid_mode = BZI_VID_MODE_ASK;
 		} else {
-			bzimg->vid_mode = strtoul ( vga, &vga, 0 );
-			if ( *vga ) {
+			bzimg->vid_mode = strtoul ( vga, &end, 0 );
+			if ( *end ) {
 				DBGC ( image, "bzImage %p strange \"vga=\" "
-				       "terminator '%c'\n", image, *vga );
+				       "terminator '%c'\n", image, *end );
 			}
 		}
 		if ( sep )
@@ -281,10 +279,9 @@ static int bzimage_parse_cmdline ( struct image *image,
 	}
 
 	/* Look for "mem=" */
-	if ( ( mem = strstr ( cmdline, "mem=" ) ) ) {
-		mem += 4;
-		bzimg->mem_limit = strtoul ( mem, &mem, 0 );
-		switch ( *mem ) {
+	if ( ( mem = image_argument ( image, "mem=" ) ) ) {
+		bzimg->mem_limit = strtoul ( mem, &end, 0 );
+		switch ( *end ) {
 		case 'G':
 		case 'g':
 			bzimg->mem_limit <<= 10;
@@ -302,7 +299,7 @@ static int bzimage_parse_cmdline ( struct image *image,
 			break;
 		default:
 			DBGC ( image, "bzImage %p strange \"mem=\" "
-			       "terminator '%c'\n", image, *mem );
+			       "terminator '%c'\n", image, *end );
 			break;
 		}
 		bzimg->mem_limit -= 1;
@@ -316,11 +313,10 @@ static int bzimage_parse_cmdline ( struct image *image,
  *
  * @v image		bzImage image
  * @v bzimg		bzImage context
- * @v cmdline		Kernel command line
  */
 static void bzimage_set_cmdline ( struct image *image,
-				  struct bzimage_context *bzimg,
-				  const char *cmdline ) {
+				  struct bzimage_context *bzimg ) {
+	const char *cmdline = ( image->cmdline ? image->cmdline : "" );
 	size_t cmdline_len;
 
 	/* Copy command line down to real-mode portion */
@@ -528,7 +524,6 @@ static void bzimage_load_initrds ( struct image *image,
  */
 static int bzimage_exec ( struct image *image ) {
 	struct bzimage_context bzimg;
-	char *cmdline = ( image->cmdline ? image->cmdline : "" );
 	int rc;
 
 	/* Read and parse header from image */
@@ -551,7 +546,7 @@ static int bzimage_exec ( struct image *image ) {
 	}
 
 	/* Parse command line for bootloader parameters */
-	if ( ( rc = bzimage_parse_cmdline ( image, &bzimg, cmdline ) ) != 0)
+	if ( ( rc = bzimage_parse_cmdline ( image, &bzimg ) ) != 0)
 		return rc;
 
 	/* Check that initrds can be loaded */
@@ -568,7 +563,7 @@ static int bzimage_exec ( struct image *image ) {
 		      bzimg.rm_filesz, bzimg.pm_sz );
 
 	/* Store command line */
-	bzimage_set_cmdline ( image, &bzimg, cmdline );
+	bzimage_set_cmdline ( image, &bzimg );
 
 	/* Prepare for exiting.  Must do this before loading initrds,
 	 * since loading the initrds will corrupt the external heap.
diff --git a/src/core/cpio.c b/src/core/cpio.c
index 27aee7581..4b607e260 100644
--- a/src/core/cpio.c
+++ b/src/core/cpio.c
@@ -77,17 +77,12 @@ size_t cpio_name_len ( struct image *image ) {
  */
 static void cpio_parse_cmdline ( struct image *image,
 				 struct cpio_header *cpio ) {
-	const char *cmdline;
-	char *arg;
+	const char *arg;
 	char *end;
 	unsigned int mode;
 
-	/* Skip image filename */
-	cmdline = ( cpio_name ( image ) + cpio_name_len ( image ) );
-
 	/* Look for "mode=" */
-	if ( ( arg = strstr ( cmdline, "mode=" ) ) ) {
-		arg += 5;
+	if ( ( arg = image_argument ( image, "mode=" ) ) ) {
 		mode = strtoul ( arg, &end, 8 /* Octal for file mode */ );
 		if ( *end && ( *end != ' ' ) ) {
 			DBGC ( image, "CPIO %p strange \"mode=\" "
diff --git a/src/core/image.c b/src/core/image.c
index 3e236ca60..f6d3d8ddd 100644
--- a/src/core/image.c
+++ b/src/core/image.c
@@ -27,6 +27,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <ctype.h>
 #include <errno.h>
 #include <assert.h>
 #include <libgen.h>
@@ -569,3 +570,33 @@ struct image * image_memory ( const char *name, userptr_t data, size_t len ) {
  err_alloc_image:
 	return NULL;
 }
+
+/**
+ * Find argument within image command line
+ *
+ * @v image		Image
+ * @v key		Argument search key (including trailing delimiter)
+ * @ret value		Argument value, or NULL if not found
+ */
+const char * image_argument ( struct image *image, const char *key ) {
+	const char *cmdline = image->cmdline;
+	const char *search;
+	const char *match;
+	const char *next;
+
+	/* Find argument */
+	for ( search = cmdline ; search ; search = next ) {
+
+		/* Find next occurrence, if any */
+		match = strstr ( search, key );
+		if ( ! match )
+			break;
+		next = ( match + strlen ( key ) );
+
+		/* Check preceding delimiter, if any */
+		if ( ( match == cmdline ) || isspace ( match[-1] ) )
+			return next;
+	}
+
+	return NULL;
+}
diff --git a/src/include/ipxe/image.h b/src/include/ipxe/image.h
index 0a5a26034..9e0c0f22a 100644
--- a/src/include/ipxe/image.h
+++ b/src/include/ipxe/image.h
@@ -195,6 +195,7 @@ extern struct image * image_find_selected ( void );
 extern int image_set_trust ( int require_trusted, int permanent );
 extern struct image * image_memory ( const char *name, userptr_t data,
 				     size_t len );
+extern const char * image_argument ( struct image *image, const char *key );
 extern int image_pixbuf ( struct image *image, struct pixel_buffer **pixbuf );
 extern int image_asn1 ( struct image *image, size_t offset,
 			struct asn1_cursor **cursor );
-- 
cgit v1.2.3-55-g7522


From 33cb56cf1b7a7138542fe18fd86898fdca2e8f0a Mon Sep 17 00:00:00 2001
From: Michael Brown
Date: Tue, 28 Feb 2023 16:22:19 +0000
Subject: [params] Rename "form parameter" to "request parameter"

Prepare for the parameter mechanism to be generalised to specifying
request parameters that are passed via mechanisms other than an
application/x-www-form-urlencoded form.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 src/config/general.h         |  2 +-
 src/core/params.c            | 10 +++++-----
 src/core/parseopt.c          |  2 +-
 src/hci/commands/param_cmd.c |  4 ++--
 src/include/ipxe/params.h    | 16 ++++++++--------
 src/include/ipxe/uri.h       |  2 +-
 src/tests/uri_test.c         | 22 +++++++++++-----------
 7 files changed, 29 insertions(+), 29 deletions(-)

(limited to 'src/core')

diff --git a/src/config/general.h b/src/config/general.h
index 72a6a9a1b..e75a2affd 100644
--- a/src/config/general.h
+++ b/src/config/general.h
@@ -150,7 +150,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 //#define POWEROFF_CMD		/* Power off command */
 //#define IMAGE_TRUST_CMD	/* Image trust management commands */
 //#define PCI_CMD		/* PCI commands */
-//#define PARAM_CMD		/* Form parameter commands */
+//#define PARAM_CMD		/* Request parameter commands */
 //#define NEIGHBOUR_CMD		/* Neighbour management commands */
 //#define PING_CMD		/* Ping command */
 //#define CONSOLE_CMD		/* Console command */
diff --git a/src/core/params.c b/src/core/params.c
index e1f66acca..23206bef6 100644
--- a/src/core/params.c
+++ b/src/core/params.c
@@ -25,7 +25,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 
 /** @file
  *
- * Form parameters
+ * Request parameters
  *
  */
 
@@ -37,7 +37,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 static LIST_HEAD ( parameters );
 
 /**
- * Free form parameter list
+ * Free request parameter list
  *
  * @v refcnt		Reference count
  */
@@ -60,7 +60,7 @@ static void free_parameters ( struct refcnt *refcnt ) {
 }
 
 /**
- * Find form parameter list by name
+ * Find request parameter list by name
  *
  * @v name		Parameter list name (may be NULL)
  * @ret params		Parameter list, or NULL if not found
@@ -78,7 +78,7 @@ struct parameters * find_parameters ( const char *name ) {
 }
 
 /**
- * Create form parameter list
+ * Create request parameter list
  *
  * @v name		Parameter list name (may be NULL)
  * @ret params		Parameter list, or NULL on failure
@@ -118,7 +118,7 @@ struct parameters * create_parameters ( const char *name ) {
 }
 
 /**
- * Add form parameter
+ * Add request parameter
  *
  * @v params		Parameter list
  * @v key		Parameter key
diff --git a/src/core/parseopt.c b/src/core/parseopt.c
index 007080088..1dbfc7aef 100644
--- a/src/core/parseopt.c
+++ b/src/core/parseopt.c
@@ -302,7 +302,7 @@ int parse_autovivified_setting ( char *text, struct named_setting *setting ) {
 }
 
 /**
- * Parse form parameter list name
+ * Parse request parameter list name
  *
  * @v text		Text
  * @ret params		Parameter list
diff --git a/src/hci/commands/param_cmd.c b/src/hci/commands/param_cmd.c
index bff04f2ff..9e3260de1 100644
--- a/src/hci/commands/param_cmd.c
+++ b/src/hci/commands/param_cmd.c
@@ -25,7 +25,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 
 /** @file
  *
- * Form parameter commands
+ * Request parameter commands
  *
  */
 
@@ -154,7 +154,7 @@ static int param_exec ( int argc, char **argv ) {
 	return rc;
 }
 
-/** Form parameter commands */
+/** Request parameter commands */
 struct command param_commands[] __command = {
 	{
 		.name = "params",
diff --git a/src/include/ipxe/params.h b/src/include/ipxe/params.h
index dd3292efc..955f57acc 100644
--- a/src/include/ipxe/params.h
+++ b/src/include/ipxe/params.h
@@ -3,7 +3,7 @@
 
 /** @file
  *
- * Form parameters
+ * Request parameters
  *
  */
 
@@ -12,7 +12,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <ipxe/list.h>
 #include <ipxe/refcnt.h>
 
-/** A form parameter list */
+/** A request parameter list */
 struct parameters {
 	/** Reference count */
 	struct refcnt refcnt;
@@ -24,9 +24,9 @@ struct parameters {
 	struct list_head entries;
 };
 
-/** A form parameter */
+/** A request parameter */
 struct parameter {
-	/** List of form parameters */
+	/** List of request parameters */
 	struct list_head list;
 	/** Key */
 	const char *key;
@@ -35,7 +35,7 @@ struct parameter {
 };
 
 /**
- * Increment form parameter list reference count
+ * Increment request parameter list reference count
  *
  * @v params		Parameter list, or NULL
  * @ret params		Parameter list as passed in
@@ -47,7 +47,7 @@ params_get ( struct parameters *params ) {
 }
 
 /**
- * Decrement form parameter list reference count
+ * Decrement request parameter list reference count
  *
  * @v params		Parameter list, or NULL
  */
@@ -57,7 +57,7 @@ params_put ( struct parameters *params ) {
 }
 
 /**
- * Claim ownership of form parameter list
+ * Claim ownership of request parameter list
  *
  * @v params		Parameter list
  * @ret params		Parameter list
@@ -71,7 +71,7 @@ claim_parameters ( struct parameters *params ) {
 	return params;
 }
 
-/** Iterate over all form parameters in a list */
+/** Iterate over all request parameters in a list */
 #define for_each_param( param, params )				\
 	list_for_each_entry ( (param), &(params)->entries, list )
 
diff --git a/src/include/ipxe/uri.h b/src/include/ipxe/uri.h
index e5b7c8616..a94b525e1 100644
--- a/src/include/ipxe/uri.h
+++ b/src/include/ipxe/uri.h
@@ -84,7 +84,7 @@ struct uri {
 	const char *equery;
 	/** Fragment (with original URI encoding) */
 	const char *efragment;
-	/** Form parameters */
+	/** Request parameters */
 	struct parameters *params;
 } __attribute__ (( packed ));
 
diff --git a/src/tests/uri_test.c b/src/tests/uri_test.c
index 338f479cd..5e7060323 100644
--- a/src/tests/uri_test.c
+++ b/src/tests/uri_test.c
@@ -92,7 +92,7 @@ struct uri_churi_test {
 	const char *expected;
 };
 
-/** A form parameter URI test list */
+/** A request parameter URI test list */
 struct uri_params_test_list {
 	/** Key */
 	const char *key;
@@ -100,7 +100,7 @@ struct uri_params_test_list {
 	const char *value;
 };
 
-/** A form parameter URI test */
+/** A request parameter URI test */
 struct uri_params_test {
 	/** URI string */
 	const char *string;
@@ -403,9 +403,9 @@ static void uri_churi_okx ( struct uri_churi_test *test, const char *file,
 #define uri_churi_ok( test ) uri_churi_okx ( test, __FILE__, __LINE__ )
 
 /**
- * Report form parameter URI test list result
+ * Report request parameter URI test list result
  *
- * @v test		Form parameter URI test
+ * @v test		Request parameter URI test
  * @v uri		URI
  * @v file		Test code file
  * @v line		Test code line
@@ -437,9 +437,9 @@ static void uri_params_list_okx ( struct uri_params_test *test,
 	uri_params_list_okx ( test, __FILE__, __LINE__ )
 
 /**
- * Report form parameter URI test result
+ * Report request parameter URI test result
  *
- * @v test		Form parameter URI test
+ * @v test		Request parameter URI test
  * @v file		Test code file
  * @v line		Test code line
  */
@@ -879,7 +879,7 @@ static struct uri_churi_test uri_churi[] = {
 	}
 };
 
-/** Form parameter URI test list */
+/** Request parameter URI test list */
 static struct uri_params_test_list uri_params_list[] = {
 	{
 		"vendor",
@@ -899,7 +899,7 @@ static struct uri_params_test_list uri_params_list[] = {
 	}
 };
 
-/** Form parameter URI test */
+/** Request parameter URI test */
 static struct uri_params_test uri_params = {
 	"http://boot.ipxe.org/demo/boot.php##params",
 	{
@@ -912,7 +912,7 @@ static struct uri_params_test uri_params = {
 	uri_params_list,
 };
 
-/** Named form parameter URI test list */
+/** Named request parameter URI test list */
 static struct uri_params_test_list uri_named_params_list[] = {
 	{
 		"mac",
@@ -928,7 +928,7 @@ static struct uri_params_test_list uri_named_params_list[] = {
 	}
 };
 
-/** Named form parameter URI test */
+/** Named request parameter URI test */
 static struct uri_params_test uri_named_params = {
 	"http://192.168.100.4:3001/register##params=foo",
 	{
@@ -996,7 +996,7 @@ static void uri_test_exec ( void ) {
 	/* Current working URI tests */
 	uri_churi_ok ( uri_churi );
 
-	/* Form parameter URI tests */
+	/* Request parameter URI tests */
 	uri_params_ok ( &uri_params );
 	uri_params_ok ( &uri_named_params );
 }
-- 
cgit v1.2.3-55-g7522


From 96bb6ba441653a30729ade38dc6c23bc9e2d2339 Mon Sep 17 00:00:00 2001
From: Michael Brown
Date: Tue, 28 Feb 2023 17:46:13 +0000
Subject: [params] Allow for arbitrary HTTP request headers to be specified

Extend the request parameter mechanism to allow for arbitrary HTTP
headers to be specified via e.g.:

  params
  param --header Referer http://www.example.com
  imgfetch http://192.168.0.1/script.ipxe##params

Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 src/core/params.c            | 11 ++++++++---
 src/hci/commands/param_cmd.c | 10 +++++++++-
 src/include/ipxe/params.h    | 11 ++++++++++-
 src/net/tcp/httpcore.c       | 43 ++++++++++++++++++++++++++++++++++---------
 src/tests/uri_test.c         | 13 ++++++++++++-
 5 files changed, 73 insertions(+), 15 deletions(-)

(limited to 'src/core')

diff --git a/src/core/params.c b/src/core/params.c
index 23206bef6..58c829f62 100644
--- a/src/core/params.c
+++ b/src/core/params.c
@@ -123,10 +123,12 @@ struct parameters * create_parameters ( const char *name ) {
  * @v params		Parameter list
  * @v key		Parameter key
  * @v value		Parameter value
+ * @v flags		Parameter flags
  * @ret param		Parameter, or NULL on failure
  */
 struct parameter * add_parameter ( struct parameters *params,
-				   const char *key, const char *value ) {
+				   const char *key, const char *value,
+				   unsigned int flags ) {
 	struct parameter *param;
 	size_t key_len;
 	size_t value_len;
@@ -147,11 +149,14 @@ struct parameter * add_parameter ( struct parameters *params,
 	param->key = key_copy;
 	strcpy ( value_copy, value );
 	param->value = value_copy;
+	param->flags = flags;
 
 	/* Add to list of parameters */
 	list_add_tail ( &param->list, &params->entries );
 
-	DBGC ( params, "PARAMS \"%s\" added \"%s\"=\"%s\"\n",
-	       params->name, param->key, param->value );
+	DBGC ( params, "PARAMS \"%s\" added \"%s\"=\"%s\"%s%s\n",
+	       params->name, param->key, param->value,
+	       ( ( param->flags & PARAMETER_FORM ) ? " (form)" : "" ),
+	       ( ( param->flags & PARAMETER_HEADER ) ? " (header)" : "" ) );
 	return param;
 }
diff --git a/src/hci/commands/param_cmd.c b/src/hci/commands/param_cmd.c
index 9e3260de1..dad99f840 100644
--- a/src/hci/commands/param_cmd.c
+++ b/src/hci/commands/param_cmd.c
@@ -90,12 +90,16 @@ static int params_exec ( int argc, char **argv ) {
 struct param_options {
 	/** Parameter list name */
 	char *params;
+	/** Parameter is a header */
+	int header;
 };
 
 /** "param" option list */
 static struct option_descriptor param_opts[] = {
 	OPTION_DESC ( "params", 'p', required_argument,
 		      struct param_options, params, parse_string ),
+	OPTION_DESC ( "header", 'H', no_argument,
+		      struct param_options, header, parse_flag ),
 };
 
 /** "param" command descriptor */
@@ -114,6 +118,7 @@ static int param_exec ( int argc, char **argv ) {
 	struct param_options opts;
 	char *key;
 	char *value;
+	unsigned int flags;
 	struct parameters *params;
 	struct parameter *param;
 	int rc;
@@ -132,12 +137,15 @@ static int param_exec ( int argc, char **argv ) {
 		goto err_parse_value;
 	}
 
+	/* Construct flags */
+	flags = ( opts.header ? PARAMETER_HEADER : PARAMETER_FORM );
+
 	/* Identify parameter list */
 	if ( ( rc = parse_parameters ( opts.params, &params ) ) != 0 )
 		goto err_parse_parameters;
 
 	/* Add parameter */
-	param = add_parameter ( params, key, value );
+	param = add_parameter ( params, key, value, flags );
 	if ( ! param ) {
 		rc = -ENOMEM;
 		goto err_add_parameter;
diff --git a/src/include/ipxe/params.h b/src/include/ipxe/params.h
index 955f57acc..61e46e029 100644
--- a/src/include/ipxe/params.h
+++ b/src/include/ipxe/params.h
@@ -32,8 +32,16 @@ struct parameter {
 	const char *key;
 	/** Value */
 	const char *value;
+	/** Flags */
+	unsigned int flags;
 };
 
+/** Request parameter is a form parameter */
+#define PARAMETER_FORM 0x0001
+
+/** Request parameter is a header parameter */
+#define PARAMETER_HEADER 0x0002
+
 /**
  * Increment request parameter list reference count
  *
@@ -78,6 +86,7 @@ claim_parameters ( struct parameters *params ) {
 extern struct parameters * find_parameters ( const char *name );
 extern struct parameters * create_parameters ( const char *name );
 extern struct parameter * add_parameter ( struct parameters *params,
-					  const char *key, const char *value );
+					  const char *key, const char *value,
+					  unsigned int flags );
 
 #endif /* _IPXE_PARAMS_H */
diff --git a/src/net/tcp/httpcore.c b/src/net/tcp/httpcore.c
index c970d54bf..9ad39656d 100644
--- a/src/net/tcp/httpcore.c
+++ b/src/net/tcp/httpcore.c
@@ -830,7 +830,9 @@ static int http_transfer_complete ( struct http_transaction *http ) {
  */
 static int http_format_headers ( struct http_transaction *http, char *buf,
 				 size_t len ) {
+	struct parameters *params = http->uri->params;
 	struct http_request_header *header;
+	struct parameter *param;
 	size_t used;
 	size_t remaining;
 	char *line;
@@ -844,7 +846,7 @@ static int http_format_headers ( struct http_transaction *http, char *buf,
 		DBGC2 ( http, "HTTP %p TX %s\n", http, buf );
 	used += ssnprintf ( ( buf + used ), ( len - used ), "\r\n" );
 
-	/* Construct all headers */
+	/* Construct all fixed headers */
 	for_each_table_entry ( header, HTTP_REQUEST_HEADERS ) {
 
 		/* Determine header value length */
@@ -869,6 +871,23 @@ static int http_format_headers ( struct http_transaction *http, char *buf,
 		used += ssnprintf ( ( buf + used ), ( len - used ), "\r\n" );
 	}
 
+	/* Construct parameter headers, if any */
+	if ( params ) {
+
+		/* Construct all parameter headers */
+		for_each_param ( param, params ) {
+
+			/* Skip non-header parameters */
+			if ( ! ( param->flags & PARAMETER_HEADER ) )
+				continue;
+
+			/* Add parameter */
+			used += ssnprintf ( ( buf + used ), ( len - used ),
+					    "%s: %s\r\n", param->key,
+					    param->value );
+		}
+	}
+
 	/* Construct terminating newline */
 	used += ssnprintf ( ( buf + used ), ( len - used ), "\r\n" );
 
@@ -1851,14 +1870,15 @@ static struct http_state http_trailers = {
  */
 
 /**
- * Construct HTTP parameter list
+ * Construct HTTP form parameter list
  *
  * @v params		Parameter list
  * @v buf		Buffer to contain HTTP POST parameters
  * @v len		Length of buffer
  * @ret len		Length of parameter list (excluding terminating NUL)
  */
-static size_t http_params ( struct parameters *params, char *buf, size_t len ) {
+static size_t http_form_params ( struct parameters *params, char *buf,
+				 size_t len ) {
 	struct parameter *param;
 	ssize_t remaining = len;
 	size_t frag_len;
@@ -1867,6 +1887,10 @@ static size_t http_params ( struct parameters *params, char *buf, size_t len ) {
 	len = 0;
 	for_each_param ( param, params ) {
 
+		/* Skip non-form parameters */
+		if ( ! ( param->flags & PARAMETER_FORM ) )
+			continue;
+
 		/* Add the "&", if applicable */
 		if ( len ) {
 			if ( remaining > 0 )
@@ -1920,25 +1944,26 @@ int http_open_uri ( struct interface *xfer, struct uri *uri ) {
 	size_t check_len;
 	int rc;
 
-	/* Calculate length of parameter list, if any */
-	len = ( params ? http_params ( params, NULL, 0 ) : 0 );
+	/* Calculate length of form parameter list, if any */
+	len = ( params ? http_form_params ( params, NULL, 0 ) : 0 );
 
-	/* Use POST if and only if there are parameters */
+	/* Use POST if and only if there are form parameters */
 	if ( len ) {
 
 		/* Use POST */
 		method = &http_post;
 		type = "application/x-www-form-urlencoded";
 
-		/* Allocate temporary parameter list */
+		/* Allocate temporary form parameter list */
 		data = zalloc ( len + 1 /* NUL */ );
 		if ( ! data ) {
 			rc = -ENOMEM;
 			goto err_alloc;
 		}
 
-		/* Construct temporary parameter list */
-		check_len = http_params ( params, data, ( len + 1 /* NUL */ ) );
+		/* Construct temporary form parameter list */
+		check_len = http_form_params ( params, data,
+					       ( len + 1 /* NUL */ ) );
 		assert ( check_len == len );
 
 	} else {
diff --git a/src/tests/uri_test.c b/src/tests/uri_test.c
index 5e7060323..9d2f6dba5 100644
--- a/src/tests/uri_test.c
+++ b/src/tests/uri_test.c
@@ -98,6 +98,8 @@ struct uri_params_test_list {
 	const char *key;
 	/** Value */
 	const char *value;
+	/** Flags */
+	unsigned int flags;
 };
 
 /** A request parameter URI test */
@@ -428,6 +430,7 @@ static void uri_params_list_okx ( struct uri_params_test *test,
 			      file, line );
 			okx ( strcmp ( param->value, list->value ) == 0,
 			      file, line );
+			okx ( param->flags == list->flags, file, line );
 			list++;
 		}
 		okx ( list->key == NULL, file, line );
@@ -456,7 +459,8 @@ static void uri_params_okx ( struct uri_params_test *test, const char *file,
 	okx ( params != NULL, file, line );
 	if ( params ) {
 		for ( list = test->list ; list->key ; list++ ) {
-			param = add_parameter ( params, list->key, list->value);
+			param = add_parameter ( params, list->key, list->value,
+						list->flags );
 			okx ( param != NULL, file, line );
 		}
 	}
@@ -884,18 +888,22 @@ static struct uri_params_test_list uri_params_list[] = {
 	{
 		"vendor",
 		"10ec",
+		PARAMETER_FORM,
 	},
 	{
 		"device",
 		"8139",
+		PARAMETER_FORM,
 	},
 	{
 		"uuid",
 		"f59fac00-758f-498f-9fe5-87d790045d94",
+		PARAMETER_HEADER,
 	},
 	{
 		NULL,
 		NULL,
+		0,
 	}
 };
 
@@ -917,14 +925,17 @@ static struct uri_params_test_list uri_named_params_list[] = {
 	{
 		"mac",
 		"00:1e:65:80:d3:b6",
+		PARAMETER_FORM,
 	},
 	{
 		"serial",
 		"LXTQ20Z1139322762F2000",
+		PARAMETER_FORM,
 	},
 	{
 		NULL,
 		NULL,
+		0,
 	}
 };
 
-- 
cgit v1.2.3-55-g7522


From e51e7bbad7d043a6b369b0050ff4e1c0e62f3b5d Mon Sep 17 00:00:00 2001
From: Michael Brown
Date: Mon, 6 Mar 2023 16:55:54 +0000
Subject: [image] Consistently use for_each_image() to iterate over images

Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 src/core/image.c             | 2 +-
 src/interface/efi/efi_file.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

(limited to 'src/core')

diff --git a/src/core/image.c b/src/core/image.c
index f6d3d8ddd..5a9aebc74 100644
--- a/src/core/image.c
+++ b/src/core/image.c
@@ -312,7 +312,7 @@ void unregister_image ( struct image *image ) {
 struct image * find_image ( const char *name ) {
 	struct image *image;
 
-	list_for_each_entry ( image, &images, list ) {
+	for_each_image ( image ) {
 		if ( strcmp ( image->name, name ) == 0 )
 			return image;
 	}
diff --git a/src/interface/efi/efi_file.c b/src/interface/efi/efi_file.c
index 7bcf8d597..f2b44fa73 100644
--- a/src/interface/efi/efi_file.c
+++ b/src/interface/efi/efi_file.c
@@ -130,7 +130,7 @@ static struct image * efi_file_find ( const char *name ) {
 	struct image *image;
 
 	/* Find image */
-	list_for_each_entry ( image, &images, list ) {
+	for_each_image ( image ) {
 		if ( strcasecmp ( image->name, name ) == 0 )
 			return image;
 	}
-- 
cgit v1.2.3-55-g7522


From 9e1f7a3659071004f4b8c76f2593da6287f0d575 Mon Sep 17 00:00:00 2001
From: Michael Brown
Date: Mon, 6 Mar 2023 16:28:48 +0000
Subject: [image] Always unregister currently executing image

We unregister script images during their execution, to prevent a
"boot" command from re-executing the containing script.  This also has
the side effect of preventing executing scripts from showing up within
the Linux magic initrd image (or the Multiboot module list).

Additional logic in bzimage.c and efi_file.c prevents a currently
executing kernel from showing up within the magic initrd image.
Similar logic in multiboot.c prevents the Multiboot kernel from
showing up as a Multiboot module.

This still leaves some corner cases that are not covered correctly.
For example: when using a gzip-compressed kernel image, nothing will
currently hide the original compressed image from the magic initrd.

Fix by moving the logic that temporarily unregisters the current image
from script_exec() to image_exec(), so that it applies to all image
types, and simplify the magic initrd and Multiboot module list
construction logic on the basis that no further filtering of the
registered image list is necessary.

This change has the side effect of hiding currently executing EFI
images from the virtual filesystem exposed by iPXE.  For example, when
using iPXE to boot wimboot, the wimboot binary itself will no longer
be visible within the virtual filesystem.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 src/arch/x86/image/bzimage.c   |  8 --------
 src/arch/x86/image/multiboot.c |  4 ----
 src/core/image.c               | 12 +++++++++---
 src/image/script.c             |  9 ---------
 src/interface/efi/efi_file.c   |  6 +-----
 5 files changed, 10 insertions(+), 29 deletions(-)

(limited to 'src/core')

diff --git a/src/arch/x86/image/bzimage.c b/src/arch/x86/image/bzimage.c
index b40bb2d07..b15bd5563 100644
--- a/src/arch/x86/image/bzimage.c
+++ b/src/arch/x86/image/bzimage.c
@@ -355,10 +355,6 @@ static size_t bzimage_load_initrd ( struct image *image,
 	size_t offset;
 	size_t pad_len;
 
-	/* Do not include kernel image itself as an initrd */
-	if ( initrd == image )
-		return 0;
-
 	/* Create cpio header for non-prebuilt images */
 	offset = cpio_header ( initrd, &cpio );
 
@@ -406,10 +402,6 @@ static int bzimage_check_initrds ( struct image *image,
 	/* Calculate total loaded length of initrds */
 	for_each_image ( initrd ) {
 
-		/* Skip kernel */
-		if ( initrd == image )
-			continue;
-
 		/* Calculate length */
 		len += bzimage_load_initrd ( image, initrd, UNULL );
 		len = bzimage_align ( len );
diff --git a/src/arch/x86/image/multiboot.c b/src/arch/x86/image/multiboot.c
index 0c85df708..c1c63bc97 100644
--- a/src/arch/x86/image/multiboot.c
+++ b/src/arch/x86/image/multiboot.c
@@ -204,10 +204,6 @@ static int multiboot_add_modules ( struct image *image, physaddr_t start,
 			break;
 		}
 
-		/* Do not include kernel image itself as a module */
-		if ( module_image == image )
-			continue;
-
 		/* Page-align the module */
 		start = ( ( start + 0xfff ) & ~0xfff );
 
diff --git a/src/core/image.c b/src/core/image.c
index 5a9aebc74..b280eb4d3 100644
--- a/src/core/image.c
+++ b/src/core/image.c
@@ -349,9 +349,8 @@ int image_exec ( struct image *image ) {
 	/* Preserve record of any currently-running image */
 	saved_current_image = current_image;
 
-	/* Take out a temporary reference to the image.  This allows
-	 * the image to unregister itself if necessary, without
-	 * automatically freeing itself.
+	/* Take out a temporary reference to the image, so that it
+	 * does not get freed when temporarily unregistered.
 	 */
 	current_image = image_get ( image );
 
@@ -371,6 +370,9 @@ int image_exec ( struct image *image ) {
 	/* Record boot attempt */
 	syslog ( LOG_NOTICE, "Executing \"%s\"\n", image->name );
 
+	/* Temporarily unregister the image during its execution */
+	unregister_image ( image );
+
 	/* Try executing the image */
 	if ( ( rc = image->type->exec ( image ) ) != 0 ) {
 		DBGC ( image, "IMAGE %s could not execute: %s\n",
@@ -387,6 +389,10 @@ int image_exec ( struct image *image ) {
 			 image->name, strerror ( rc ) );
 	}
 
+	/* Re-register image (unless due to be replaced) */
+	if ( ! image->replacement )
+		register_image ( image );
+
 	/* Pick up replacement image before we drop the original
 	 * image's temporary reference.  The replacement image must
 	 * already be registered, so we don't need to hold a temporary
diff --git a/src/image/script.c b/src/image/script.c
index 28050868a..b34df1e21 100644
--- a/src/image/script.c
+++ b/src/image/script.c
@@ -197,11 +197,6 @@ static int script_exec ( struct image *image ) {
 	size_t saved_offset;
 	int rc;
 
-	/* Temporarily de-register image, so that a "boot" command
-	 * doesn't throw us into an execution loop.
-	 */
-	unregister_image ( image );
-
 	/* Preserve state of any currently-running script */
 	saved_offset = script_offset;
 
@@ -212,10 +207,6 @@ static int script_exec ( struct image *image ) {
 	/* Restore saved state */
 	script_offset = saved_offset;
 
-	/* Re-register image (unless we have been replaced) */
-	if ( ! image->replacement )
-		register_image ( image );
-
 	return rc;
 }
 
diff --git a/src/interface/efi/efi_file.c b/src/interface/efi/efi_file.c
index f2b44fa73..a0bba1606 100644
--- a/src/interface/efi/efi_file.c
+++ b/src/interface/efi/efi_file.c
@@ -240,10 +240,6 @@ static size_t efi_file_read_initrd ( struct efi_file_reader *reader ) {
 	len = 0;
 	for_each_image ( image ) {
 
-		/* Ignore currently executing image */
-		if ( image == current_image )
-			continue;
-
 		/* Pad to alignment boundary */
 		pad_len = ( ( -reader->pos ) & ( INITRD_ALIGN - 1 ) );
 		if ( pad_len ) {
@@ -1091,7 +1087,7 @@ int efi_file_install ( EFI_HANDLE handle ) {
 	 * instance only if the initrd is non-empty, since Linux does
 	 * not gracefully handle a zero-length initrd.
 	 */
-	load = ( list_is_singular ( &images ) ? NULL : &efi_file_initrd.load );
+	load = ( have_images() ? &efi_file_initrd.load : NULL );
 	if ( ( rc = efi_file_path_install ( &efi_file_initrd_path.vendor.Header,
 					    load ) ) != 0 ) {
 		goto err_initrd;
-- 
cgit v1.2.3-55-g7522