[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).
Signed-off-by: Michael Brown <mcb30@ipxe.org>
diff --git a/src/arch/x86/image/bzimage.c b/src/arch/x86/image/bzimage.c
index 769e0c9..b40bb2d 100644
--- a/src/arch/x86/image/bzimage.c
+++ b/src/arch/x86/image/bzimage.c
@@ -247,19 +247,17 @@
*
* @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 @@
} 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 @@
}
/* 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 @@
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 @@
*
* @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 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 @@
}
/* 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 @@
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 27aee75..4b607e2 100644
--- a/src/core/cpio.c
+++ b/src/core/cpio.c
@@ -77,17 +77,12 @@
*/
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 3e236ca..f6d3d8d 100644
--- a/src/core/image.c
+++ b/src/core/image.c
@@ -27,6 +27,7 @@
#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 @@
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 0a5a260..9e0c0f2 100644
--- a/src/include/ipxe/image.h
+++ b/src/include/ipxe/image.h
@@ -195,6 +195,7 @@
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 );
