[image] Make text-based image data usable by string functions

Using standard string functions for parsing text-based image formats
is currently cumbersome since there is no guaranteed NUL terminator,
and so code must laboriously keep track of the remaining image length
and use only those string functions that accept a length limit.

Ensure that the byte immediately following the image data is always a
NUL, thereby allowing all string functions to be used when parsing
images.  Provide a "const char *text" pointer aliased to the image
data, to make it explicit that image data may always be treated as a
NUL-terminated string.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/core/image.c

@@ -74,6 +74,9 @@ static int require_trusted_images = 0;
 /** Prevent changes to image trust requirement */
 static int require_trusted_images_permanent = 0;
 
+/** Empty image terminating NUL */
+const char empty_image_data[] = { '\0' };
+
 /**
  * Free executable image
  *
@@ -110,7 +113,7 @@ void free_image ( struct refcnt *refcnt ) {
 
 	/* Free image data and image itself, if dynamically allocated */
 	if ( ! ( image->flags & IMAGE_STATIC ) ) {
-		ufree ( image->rwdata );
+		image_set_len ( image, 0 );
 		free ( image );
 	}
 }
@@ -134,6 +137,7 @@ struct image * alloc_image ( struct uri *uri ) {
 	ref_init ( &image->refcnt, free_image );
 	if ( uri && ( ( rc = image_set_uri ( image, uri ) ) != 0 ) )
 		goto err_set_uri;
+	image->data = empty_image_data;
 
 	return image;
 
@@ -243,19 +247,36 @@ int image_set_cmdline ( struct image *image, const char *cmdline ) {
  * @ret rc		Return status code
  */
 int image_set_len ( struct image *image, size_t len ) {
+	size_t alloc_len;
+	void *data;
 	void *new;
+	char *nul;
 
 	/* Refuse to reallocate static images */
 	if ( image->flags & IMAGE_STATIC )
 		return -ENOTTY;
 
+	/* Calculate allocation length (including a terminating NUL) */
+	alloc_len = ( len ? ( len + 1 ) : 0 );
+	if ( alloc_len < len )
+		return -ERANGE;
+
 	/* (Re)allocate image data */
-	new = urealloc ( image->rwdata, len );
+	data = image->rwdata;
+	if ( data == empty_image_data )
+		data = NULL;
+	new = urealloc ( data, alloc_len );
 	if ( ! new )
 		return -ENOMEM;
-	image->rwdata = new;
+	image->data = ( len ? new : empty_image_data );
 	image->len = len;
 
+	/* Add terminating NUL (if not the empty image) */
+	if ( len ) {
+		nul = ( image->rwdata + len );
+		*nul = '\0';
+	}
+
 	return 0;
 }
 
@@ -331,6 +352,17 @@ int register_image ( struct image *image ) {
 			return rc;
 	}
 
+	/* Check for terminating NUL (required for text-based images) */
+	if ( ! image->data ) {
+		DBGC ( image, "IMAGE %s has no data\n", image->name );
+		return -EINVAL;
+	}
+	if ( image->text[image->len] != '\0' ) {
+		DBGC ( image, "IMAGE %s missing terminating NUL\n",
+		       image->name );
+		return -EINVAL;
+	}
+
 	/* Add to image list */
 	image_get ( image );
 	image->flags |= IMAGE_REGISTERED;

src/image/embedded.c

@@ -25,6 +25,7 @@ FILE_SECBOOT ( PERMITTED );
 		  ".equ embedded_image_" #_index "_len, "		\
 			"( embedded_image_" #_index "_end - "		\
 			"  embedded_image_" #_index "_data )\n\t"	\
+		  ".byte 0\n\t" /* NUL */				\
 		  ".previous\n\t" );
 EMBED_ALL
 

src/include/ipxe/image.h

@@ -51,6 +51,13 @@ struct image {
 		const void *data;
 		/** Writable data */
 		void *rwdata;
+		/** Read-only text (guaranteed to be NUL terminated)
+		 *
+		 * Images are allocated with a trailing NUL byte, to
+		 * allow string functions to be used more easily on
+		 * image data.
+		 */
+		const char *text;
 	};
 	/** Length of raw file image */
 	size_t len;
@@ -186,6 +193,7 @@ struct image_tag {
 extern struct list_head images;
 extern struct image_tag current_image;
 extern struct image_tag selected_image;
+extern const char empty_image_data[];
 
 /** Iterate over all registered images */
 #define for_each_image( image ) \

src/tests/asn1_test.h

@@ -42,13 +42,15 @@ struct asn1_test {
  * @ret test		ASN.1 test
  */
 #define ASN1( _name, _type, _file, ... )				\
-	static const char _name ## __file[] = _file;			\
+	static const char _name ## __raw[] = _file;			\
+	static const char _name ## __file				\
+		[ sizeof ( _name ## __raw ) + 1 /* NUL */ ] = _file;	\
 	static struct image _name ## __image = {			\
 		.refcnt = REF_INIT ( ref_no_free ),			\
 		.name = #_name,						\
 		.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ),		\
 		.data = _name ## __file,				\
-		.len = sizeof ( _name ## __file ),			\
+		.len = ( sizeof ( _name ## __file ) - 1 /* NUL */ ),	\
 	};								\
 	static struct asn1_test_digest _name ## _expected[] = {		\
 		__VA_ARGS__						\

src/tests/cms_test.c

@@ -79,33 +79,41 @@ struct cms_test_keypair {
 
 /** Define a test image */
 #define IMAGE( NAME, DATA )						\
-	static const uint8_t NAME ## _data[] = DATA;			\
+	static const uint8_t NAME ## _raw[] = DATA;			\
+	static const uint8_t NAME ## _data				\
+		[ sizeof ( NAME ## _raw ) + 1 /* NUL */ ] = DATA;	\
 	static struct cms_test_image NAME = {				\
 		.image = {						\
 			.refcnt = REF_INIT ( ref_no_free ),		\
 			.name = #NAME,					\
 			.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ),	\
 			.data = NAME ## _data,				\
-			.len = sizeof ( NAME ## _data ),		\
+			.len = ( sizeof ( NAME ## _data )		\
+				 - 1 /* NUL */ ),			\
 		},							\
 	}
 
 /** Define a writable test image */
 #define IMAGE_RW( NAME, DATA )						\
-	static uint8_t NAME ## _data[] = DATA;				\
+	static uint8_t NAME ## _raw[] = DATA;				\
+	static uint8_t NAME ## _data					\
+		[ sizeof ( NAME ## _raw ) + 1 /* NUL */ ] = DATA;	\
 	static struct cms_test_image NAME = {				\
 		.image = {						\
 			.refcnt = REF_INIT ( ref_no_free ),		\
 			.name = #NAME,					\
 			.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ),	\
 			.data = NAME ## _data,				\
-			.len = sizeof ( NAME ## _data ),		\
+			.len = ( sizeof ( NAME ## _data )		\
+				 - 1 /* NUL */ ),			\
 		},							\
 	}
 
 /** Define a test message */
 #define MESSAGE( NAME, DATA )						\
-	static const uint8_t NAME ## _data[] = DATA;			\
+	static const uint8_t NAME ## _raw[] = DATA;			\
+	static const uint8_t NAME ## _data				\
+		[ sizeof ( NAME ## _raw ) + 1 /* NUL */ ] = DATA;	\
 	static struct cms_test_message NAME = {				\
 		.image = {						\
 			.refcnt = REF_INIT ( ref_no_free ),		\
@@ -113,7 +121,8 @@ struct cms_test_keypair {
 			.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ),	\
 			.type = &der_image_type,			\
 			.data = NAME ## _data,				\
-			.len = sizeof ( NAME ## _data ),		\
+			.len = ( sizeof ( NAME ## _data )		\
+				 - 1 /* NUL */ ),			\
 		},							\
 	}
 

src/tests/pixbuf_test.h

@@ -36,14 +36,16 @@ struct pixel_buffer_test {
  * @ret test		Pixel buffer test
  */
 #define PIX( _name, _type, _file, _width, _height, _data )		\
-	static const char _name ## __file[] = _file;			\
+	static const char _name ## __raw[] = _file;			\
+	static const char _name ## __file				\
+		[ sizeof ( _name ## __raw ) + 1 /* NUL */ ] = _file;	\
 	static const uint32_t _name ## __data[] = _data;		\
 	static struct image _name ## __image = {			\
 		.refcnt = REF_INIT ( ref_no_free ),			\
 		.name = #_name,						\
 		.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ),		\
 		.data = _name ## __file,				\
-		.len = sizeof ( _name ## __file ),			\
+		.len = ( sizeof ( _name ## __file ) - 1 /* NUL */ ),	\
 	};								\
 	static struct pixel_buffer_test _name = {			\
 		.type = _type,						\

src/tests/test.c

@@ -164,6 +164,7 @@ static struct image test_image = {
 	.name = "<TESTS>",
 	.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ),
 	.type = &test_image_type,
+	.data = empty_image_data,
 };
 
 static void test_init ( void ) {