sysadmin/ipxe
1
0
Fork 0
mirror of https://github.com/ipxe/ipxe synced 2025-12-17 01:52:08 +03:00
Code Issues Packages Projects Releases Wiki Activity

[image] Make image data read-only to most consumers

Browse Source 
Almost all image consumers do not need to modify the content of the
image.  Now that the image data is a pointer type (rather than the
opaque userptr_t type), we can rely on the compiler to enforce this at
build time.

Change the .data field to be a const pointer, so that the compiler can
verify that image consumers do not modify the image content.  Provide
a transparent .rwdata field for consumers who have a legitimate (and
now explicit) reason to modify the image content.

We do not attempt to impose any runtime restriction on checking
whether or not an image is writable.  The only existing instances of
genuinely read-only images are the various unit test images, and it is
acceptable for defective test cases to result in a segfault rather
than a runtime error.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
This commit is contained in:
Michael Brown
2025-04-30 14:14:51 +01:00
parent cd803ff2e2
commit 05ad7833c5
15 changed files with 45 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/arch/x86/image/initrd.c
View File

@@ -63,10 +63,9 @@ static physaddr_t initrd_squash_high ( physaddr_t top ) {
/* Find the highest image not yet in its final position */ /* Find the highest image not yet in its final position */
highest = NULL; highest = NULL;
for_each_image ( initrd ) { for_each_image ( initrd ) {
data = initrd->data; if ( ( virt_to_phys ( initrd->data ) < current ) &&
if ( ( virt_to_phys ( data ) < current ) &&
( ( highest == NULL ) || ( ( highest == NULL ) ||
( virt_to_phys ( data ) > ( virt_to_phys ( initrd->data ) >
virt_to_phys ( highest->data ) ) ) ) { virt_to_phys ( highest->data ) ) ) ) {
highest = initrd; highest = initrd;
} }
@@ -144,9 +143,9 @@ static void initrd_swap ( struct image *low, struct image *high,
/* Swap fragments */ /* Swap fragments */
memcpy ( free, ( high->data + len ), frag_len ); memcpy ( free, ( high->data + len ), frag_len );
memmove ( ( low->data + new_len ), ( low->data + len ), memmove ( ( low->rwdata + new_len ), ( low->data + len ),
low->len ); low->len );
memcpy ( ( low->data + len ), free, frag_len ); memcpy ( ( low->rwdata + len ), free, frag_len );
len = new_len; len = new_len;
} }
@@ -165,8 +164,8 @@ static void initrd_swap ( struct image *low, struct image *high,
static int initrd_swap_any ( void *free, size_t free_len ) { static int initrd_swap_any ( void *free, size_t free_len ) {
struct image *low; struct image *low;
struct image *high; struct image *high;
const void *adjacent;
size_t padded_len; size_t padded_len;
void *adjacent;
/* Find any pair of initrds that can be swapped */ /* Find any pair of initrds that can be swapped */
for_each_image ( low ) { for_each_image ( low ) {

2
src/arch/x86/image/sdi.c
View File

@@ -51,7 +51,7 @@ FEATURE ( FEATURE_IMAGE, "SDI", DHCP_EB_FEATURE_SDI, 1 );
* @ret rc Return status code * @ret rc Return status code
*/ */
static int sdi_exec ( struct image *image ) { static int sdi_exec ( struct image *image ) {
struct sdi_header *sdi; const struct sdi_header *sdi;
uint32_t sdiptr; uint32_t sdiptr;
/* Sanity check */ /* Sanity check */

2
src/core/fdt.c
View File

@@ -734,7 +734,7 @@ static int fdt_parse_image ( struct fdt *fdt, struct image *image ) {
int rc; int rc;
/* Parse image */ /* Parse image */
if ( ( rc = fdt_parse ( fdt, image->data, image->len ) ) != 0 ) { if ( ( rc = fdt_parse ( fdt, image->rwdata, image->len ) ) != 0 ) {
DBGC ( fdt, "FDT image \"%s\" is invalid: %s\n", DBGC ( fdt, "FDT image \"%s\" is invalid: %s\n",
image->name, strerror ( rc ) ); image->name, strerror ( rc ) );
return rc; return rc;

8
src/core/image.c
View File

@@ -108,7 +108,7 @@ void free_image ( struct refcnt *refcnt ) {
/* Free image data and image itself, if dynamically allocated */ /* Free image data and image itself, if dynamically allocated */
if ( ! ( image->flags & IMAGE_STATIC ) ) { if ( ! ( image->flags & IMAGE_STATIC ) ) {
ufree ( image->data ); ufree ( image->rwdata );
free ( image ); free ( image );
} }
} }
@@ -248,10 +248,10 @@ int image_set_len ( struct image *image, size_t len ) {
return -ENOTTY; return -ENOTTY;
/* (Re)allocate image data */ /* (Re)allocate image data */
new = urealloc ( image->data, len ); new = urealloc ( image->rwdata, len );
if ( ! new ) if ( ! new )
return -ENOMEM; return -ENOMEM;
image->data = new; image->rwdata = new;
image->len = len; image->len = len;
return 0; return 0;
@@ -273,7 +273,7 @@ int image_set_data ( struct image *image, const void *data, size_t len ) {
return rc; return rc;
/* Copy in new image data */ /* Copy in new image data */
memcpy ( image->data, data, len ); memcpy ( image->rwdata, data, len );
return 0; return 0;
} }

6
src/crypto/cms.c
View File

@@ -1079,7 +1079,7 @@ int cms_decrypt ( struct cms_message *cms, struct image *image,
final_len = ( ( image->len && is_block_cipher ( cipher ) ) ? final_len = ( ( image->len && is_block_cipher ( cipher ) ) ?
cipher->blocksize : 0 ); cipher->blocksize : 0 );
bulk_len = ( image->len - final_len ); bulk_len = ( image->len - final_len );
cipher_decrypt ( cipher, ctx, image->data, image->data, bulk_len ); cipher_decrypt ( cipher, ctx, image->data, image->rwdata, bulk_len );
/* Decrypt final block */ /* Decrypt final block */
cipher_decrypt ( cipher, ctx, ( image->data + bulk_len ), final, cipher_decrypt ( cipher, ctx, ( image->data + bulk_len ), final,
@@ -1117,7 +1117,7 @@ int cms_decrypt ( struct cms_message *cms, struct image *image,
* have to include include any error-handling code path to * have to include include any error-handling code path to
* reconstruct the block padding. * reconstruct the block padding.
*/ */
memcpy ( ( image->data + bulk_len ), final, final_len ); memcpy ( ( image->rwdata + bulk_len ), final, final_len );
image->len -= pad_len; image->len -= pad_len;
/* Clear image type and re-register image, if applicable */ /* Clear image type and re-register image, if applicable */
@@ -1137,7 +1137,7 @@ int cms_decrypt ( struct cms_message *cms, struct image *image,
* containing the potentially invalid (and therefore * containing the potentially invalid (and therefore
* unreproducible) block padding. * unreproducible) block padding.
*/ */
cipher_encrypt ( cipher, ctxdup, image->data, image->data, bulk_len ); cipher_encrypt ( cipher, ctxdup, image->data, image->rwdata, bulk_len );
if ( original_flags & IMAGE_REGISTERED ) { if ( original_flags & IMAGE_REGISTERED ) {
register_image ( image ); /* Cannot fail on re-registration */ register_image ( image ); /* Cannot fail on re-registration */
image_put ( image ); image_put ( image );

18
src/image/efi_image.c
View File

@@ -243,10 +243,15 @@ static int efi_image_exec ( struct image *image ) {
goto err_shim_install; goto err_shim_install;
} }
/* Attempt loading image */ /* Attempt loading image
*
* LoadImage() does not (allegedly) modify the image content,
* but requires a non-const pointer to SourceBuffer. We
* therefore use the .rwdata field rather than .data.
*/
handle = NULL; handle = NULL;
if ( ( efirc = bs->LoadImage ( FALSE, efi_image_handle, path, if ( ( efirc = bs->LoadImage ( FALSE, efi_image_handle, path,
exec->data, exec->len, exec->rwdata, exec->len,
&handle ) ) != 0 ) { &handle ) ) != 0 ) {
/* Not an EFI image */ /* Not an EFI image */
rc = -EEFI_LOAD ( efirc ); rc = -EEFI_LOAD ( efirc );
@@ -377,10 +382,15 @@ static int efi_image_probe ( struct image *image ) {
EFI_STATUS efirc; EFI_STATUS efirc;
int rc; int rc;
/* Attempt loading image */ /* Attempt loading image
*
* LoadImage() does not (allegedly) modify the image content,
* but requires a non-const pointer to SourceBuffer. We
* therefore use the .rwdata field rather than .data.
*/
handle = NULL; handle = NULL;
if ( ( efirc = bs->LoadImage ( FALSE, efi_image_handle, &empty_path, if ( ( efirc = bs->LoadImage ( FALSE, efi_image_handle, &empty_path,
image->data, image->len, image->rwdata, image->len,
&handle ) ) != 0 ) { &handle ) ) != 0 ) {
/* Not an EFI image */ /* Not an EFI image */
rc = -EEFI_LOAD ( efirc ); rc = -EEFI_LOAD ( efirc );

13
src/image/embedded.c
View File

@@ -10,7 +10,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <string.h> #include <string.h>
#include <ipxe/image.h> #include <ipxe/image.h>
#include <ipxe/uaccess.h>
#include <ipxe/init.h> #include <ipxe/init.h>
/* Raw image data for all embedded images */ /* Raw image data for all embedded images */
@@ -34,7 +33,7 @@ EMBED_ALL
.refcnt = REF_INIT ( free_image ), \ .refcnt = REF_INIT ( free_image ), \
.name = _name, \ .name = _name, \
.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \ .flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \
.data = ( userptr_t ) ( embedded_image_ ## _index ## _data ), \ .rwdata = embedded_image_ ## _index ## _data, \
.len = ( size_t ) embedded_image_ ## _index ## _len, \ .len = ( size_t ) embedded_image_ ## _index ## _len, \
}, },
static struct image embedded_images[] = { static struct image embedded_images[] = {
@@ -58,18 +57,8 @@ static void embedded_init ( void ) {
for ( i = 0 ; i < ( int ) ( sizeof ( embedded_images ) / for ( i = 0 ; i < ( int ) ( sizeof ( embedded_images ) /
sizeof ( embedded_images[0] ) ) ; i++ ) { sizeof ( embedded_images[0] ) ) ; i++ ) {
image = &embedded_images[i]; image = &embedded_images[i];
/* virt_to_user() cannot be used in a static
* initialiser, so we cast the pointer to a userptr_t
* in the initialiser and fix it up here. (This will
* actually be a no-op on most platforms.)
*/
data = ( ( void * ) image->data );
image->data = virt_to_user ( data );
DBG ( "Embedded image \"%s\": %zd bytes at %p\n", DBG ( "Embedded image \"%s\": %zd bytes at %p\n",
image->name, image->len, data ); image->name, image->len, data );
if ( ( rc = register_image ( image ) ) != 0 ) { if ( ( rc = register_image ( image ) ) != 0 ) {
DBG ( "Could not register embedded image \"%s\": " DBG ( "Could not register embedded image \"%s\": "
"%s\n", image->name, strerror ( rc ) ); "%s\n", image->name, strerror ( rc ) );

3
src/image/zlib.c
View File

@@ -65,7 +65,8 @@ int zlib_deflate ( enum deflate_format format, const void *data, size_t len,
deflate_init ( deflate, format ); deflate_init ( deflate, format );
/* Initialise output chunk */ /* Initialise output chunk */
deflate_chunk_init ( &out, extracted->data, 0, extracted->len ); deflate_chunk_init ( &out, extracted->rwdata, 0,
extracted->len );
/* Decompress data */ /* Decompress data */
if ( ( rc = deflate_inflate ( deflate, data, len, if ( ( rc = deflate_inflate ( deflate, data, len,

7
src/include/ipxe/image.h
View File

@@ -46,7 +46,12 @@ struct image {
* If the @c IMAGE_STATIC flag is set, then this is a * If the @c IMAGE_STATIC flag is set, then this is a
* statically allocated image. * statically allocated image.
*/ */
void *data; union {
/** Read-only data */
const void *data;
/** Writable data */
void *rwdata;
};
/** Length of raw file image */ /** Length of raw file image */
size_t len; size_t len;

2
src/interface/efi/efi_cmdline.c
View File

@@ -113,7 +113,7 @@ static int efi_cmdline_init ( void ) {
DBGC ( colour, "CMDLINE using command line \"%s\"\n", cmdline ); DBGC ( colour, "CMDLINE using command line \"%s\"\n", cmdline );
/* Prepare and register image */ /* Prepare and register image */
efi_cmdline_image.data = virt_to_user ( cmdline ); efi_cmdline_image.data = cmdline;
efi_cmdline_image.len = strlen ( cmdline ); efi_cmdline_image.len = strlen ( cmdline );
if ( efi_cmdline_image.len && if ( efi_cmdline_image.len &&
( ( rc = register_image ( &efi_cmdline_image ) ) != 0 ) ) { ( ( rc = register_image ( &efi_cmdline_image ) ) != 0 ) ) {

3
src/tests/asn1_test.c
View File

@@ -59,9 +59,6 @@ void asn1_okx ( struct asn1_test *test, const char *file, unsigned int line ) {
/* Sanity check */ /* Sanity check */
assert ( sizeof ( out ) == digest->digestsize ); assert ( sizeof ( out ) == digest->digestsize );
/* Correct image data pointer */
test->image->data = virt_to_user ( ( void * ) test->image->data );
/* Check that image is detected as correct type */ /* Check that image is detected as correct type */
okx ( register_image ( test->image ) == 0, file, line ); okx ( register_image ( test->image ) == 0, file, line );
okx ( test->image->type == test->type, file, line ); okx ( test->image->type == test->type, file, line );

2
src/tests/asn1_test.h
View File

@@ -47,7 +47,7 @@ struct asn1_test {
.refcnt = REF_INIT ( ref_no_free ), \ .refcnt = REF_INIT ( ref_no_free ), \
.name = #_name, \ .name = #_name, \
.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \ .flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \
.data = ( userptr_t ) ( _name ## __file ), \ .data = _name ## __file, \
.len = sizeof ( _name ## __file ), \ .len = sizeof ( _name ## __file ), \
}; \ }; \
static struct asn1_test_digest _name ## _expected[] = { \ static struct asn1_test_digest _name ## _expected[] = { \

32
src/tests/cms_test.c
View File

@@ -37,7 +37,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <ipxe/sha256.h> #include <ipxe/sha256.h>
#include <ipxe/x509.h> #include <ipxe/x509.h>
#include <ipxe/image.h> #include <ipxe/image.h>
#include <ipxe/uaccess.h>
#include <ipxe/der.h> #include <ipxe/der.h>
#include <ipxe/cms.h> #include <ipxe/cms.h>
#include <ipxe/privkey.h> #include <ipxe/privkey.h>
@@ -86,7 +85,7 @@ struct cms_test_keypair {
.refcnt = REF_INIT ( ref_no_free ), \ .refcnt = REF_INIT ( ref_no_free ), \
.name = #NAME, \ .name = #NAME, \
.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \ .flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \
.data = ( userptr_t ) ( NAME ## _data ), \ .data = NAME ## _data, \
.len = sizeof ( NAME ## _data ), \ .len = sizeof ( NAME ## _data ), \
}, \ }, \
} }
@@ -99,7 +98,7 @@ struct cms_test_keypair {
.refcnt = REF_INIT ( ref_no_free ), \ .refcnt = REF_INIT ( ref_no_free ), \
.name = #NAME, \ .name = #NAME, \
.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \ .flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \
.data = ( userptr_t ) ( NAME ## _data ), \ .data = NAME ## _data, \
.len = sizeof ( NAME ## _data ), \ .len = sizeof ( NAME ## _data ), \
}, \ }, \
} }
@@ -113,7 +112,7 @@ struct cms_test_keypair {
.name = #NAME, \ .name = #NAME, \
.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \ .flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \
.type = &der_image_type, \ .type = &der_image_type, \
.data = ( userptr_t ) ( NAME ## _data ), \ .data = NAME ## _data, \
.len = sizeof ( NAME ## _data ), \ .len = sizeof ( NAME ## _data ), \
}, \ }, \
} }
@@ -1652,16 +1651,9 @@ static time_t test_expired = 1375573111ULL; /* Sat Aug 3 23:38:31 2013 */
*/ */
static void cms_message_okx ( struct cms_test_message *msg, static void cms_message_okx ( struct cms_test_message *msg,
const char *file, unsigned int line ) { const char *file, unsigned int line ) {
const void *data = ( ( void * ) msg->image.data );
/* Fix up image data pointer */
msg->image.data = virt_to_user ( data );
/* Check ability to parse message */ /* Check ability to parse message */
okx ( cms_message ( &msg->image, &msg->cms ) == 0, file, line ); okx ( cms_message ( &msg->image, &msg->cms ) == 0, file, line );
/* Reset image data pointer */
msg->image.data = ( ( userptr_t ) data );
} }
#define cms_message_ok( msg ) \ #define cms_message_ok( msg ) \
cms_message_okx ( msg, __FILE__, __LINE__ ) cms_message_okx ( msg, __FILE__, __LINE__ )
@@ -1705,10 +1697,6 @@ static void cms_verify_okx ( struct cms_test_message *msg,
time_t time, struct x509_chain *store, time_t time, struct x509_chain *store,
struct x509_root *root, const char *file, struct x509_root *root, const char *file,
unsigned int line ) { unsigned int line ) {
const void *data = ( ( void * ) img->image.data );
/* Fix up image data pointer */
img->image.data = virt_to_user ( data );
/* Invalidate any certificates from previous tests */ /* Invalidate any certificates from previous tests */
x509_invalidate_chain ( msg->cms->certificates ); x509_invalidate_chain ( msg->cms->certificates );
@@ -1717,9 +1705,6 @@ static void cms_verify_okx ( struct cms_test_message *msg,
okx ( cms_verify ( msg->cms, &img->image, name, time, store, okx ( cms_verify ( msg->cms, &img->image, name, time, store,
root ) == 0, file, line ); root ) == 0, file, line );
okx ( img->image.flags & IMAGE_TRUSTED, file, line ); okx ( img->image.flags & IMAGE_TRUSTED, file, line );
/* Reset image data pointer */
img->image.data = ( ( userptr_t ) data );
} }
#define cms_verify_ok( msg, img, name, time, store, root ) \ #define cms_verify_ok( msg, img, name, time, store, root ) \
cms_verify_okx ( msg, img, name, time, store, root, \ cms_verify_okx ( msg, img, name, time, store, root, \
@@ -1742,10 +1727,6 @@ static void cms_verify_fail_okx ( struct cms_test_message *msg,
time_t time, struct x509_chain *store, time_t time, struct x509_chain *store,
struct x509_root *root, const char *file, struct x509_root *root, const char *file,
unsigned int line ) { unsigned int line ) {
const void *data = ( ( void * ) img->image.data );
/* Fix up image data pointer */
img->image.data = virt_to_user ( data );
/* Invalidate any certificates from previous tests */ /* Invalidate any certificates from previous tests */
x509_invalidate_chain ( msg->cms->certificates ); x509_invalidate_chain ( msg->cms->certificates );
@@ -1754,9 +1735,6 @@ static void cms_verify_fail_okx ( struct cms_test_message *msg,
okx ( cms_verify ( msg->cms, &img->image, name, time, store, okx ( cms_verify ( msg->cms, &img->image, name, time, store,
root ) != 0, file, line ); root ) != 0, file, line );
okx ( ! ( img->image.flags & IMAGE_TRUSTED ), file, line ); okx ( ! ( img->image.flags & IMAGE_TRUSTED ), file, line );
/* Reset image data pointer */
img->image.data = ( ( userptr_t ) data );
} }
#define cms_verify_fail_ok( msg, img, name, time, store, root ) \ #define cms_verify_fail_ok( msg, img, name, time, store, root ) \
cms_verify_fail_okx ( msg, img, name, time, store, root, \ cms_verify_fail_okx ( msg, img, name, time, store, root, \
@@ -1777,10 +1755,6 @@ static void cms_decrypt_okx ( struct cms_test_image *img,
struct cms_test_keypair *keypair, struct cms_test_keypair *keypair,
struct cms_test_image *expected, struct cms_test_image *expected,
const char *file, unsigned int line ) { const char *file, unsigned int line ) {
const void *data = ( ( void * ) img->image.data );
/* Fix up image data pointer */
img->image.data = virt_to_user ( data );
/* Check ability to decrypt image */ /* Check ability to decrypt image */
okx ( cms_decrypt ( envelope->cms, &img->image, NULL, okx ( cms_decrypt ( envelope->cms, &img->image, NULL,

7
src/tests/pixbuf_test.c
View File

@@ -55,9 +55,6 @@ void pixbuf_okx ( struct pixel_buffer_test *test, const char *file,
assert ( ( test->width * test->height * sizeof ( test->data[0] ) ) assert ( ( test->width * test->height * sizeof ( test->data[0] ) )
== test->len ); == test->len );
/* Correct image data pointer */
test->image->data = virt_to_user ( ( void * ) test->image->data );
/* Check that image is detected as correct type */ /* Check that image is detected as correct type */
okx ( register_image ( test->image ) == 0, file, line ); okx ( register_image ( test->image ) == 0, file, line );
okx ( test->image->type == test->type, file, line ); okx ( test->image->type == test->type, file, line );
@@ -72,8 +69,8 @@ void pixbuf_okx ( struct pixel_buffer_test *test, const char *file,
/* Check pixel buffer data */ /* Check pixel buffer data */
okx ( pixbuf->len == test->len, file, line ); okx ( pixbuf->len == test->len, file, line );
okx ( memcmp ( pixbuf->data, virt_to_user ( test->data ), okx ( memcmp ( pixbuf->data, test->data, test->len ) == 0,
test->len ) == 0, file, line ); file, line );
pixbuf_put ( pixbuf ); pixbuf_put ( pixbuf );
} }

2
src/tests/pixbuf_test.h
View File

@@ -42,7 +42,7 @@ struct pixel_buffer_test {
.refcnt = REF_INIT ( ref_no_free ), \ .refcnt = REF_INIT ( ref_no_free ), \
.name = #_name, \ .name = #_name, \
.flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \ .flags = ( IMAGE_STATIC | IMAGE_STATIC_NAME ), \
.data = ( userptr_t ) ( _name ## __file ), \ .data = _name ## __file, \
.len = sizeof ( _name ## __file ), \ .len = sizeof ( _name ## __file ), \
}; \ }; \
static struct pixel_buffer_test _name = { \ static struct pixel_buffer_test _name = { \