[multiboot] Remove userptr_t from Multiboot and ELF image parsing

Simplify Multiboot and ELF image parsing by assuming that the
Multiboot and ELF headers are directly accessible via pointer
dereferences, and add some missing header validations.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
This commit is contained in:
Michael Brown
2025-04-28 11:20:16 +01:00
parent c8c5cd685f
commit ba2135d0fd
4 changed files with 105 additions and 88 deletions

View File

@@ -87,7 +87,7 @@ static int elfboot_exec ( struct image *image ) {
* @v dest Destination address * @v dest Destination address
* @ret rc Return status code * @ret rc Return status code
*/ */
static int elfboot_check_segment ( struct image *image, Elf_Phdr *phdr, static int elfboot_check_segment ( struct image *image, const Elf_Phdr *phdr,
physaddr_t dest ) { physaddr_t dest ) {
/* Check that ELF segment uses flat physical addressing */ /* Check that ELF segment uses flat physical addressing */
@@ -107,7 +107,7 @@ static int elfboot_check_segment ( struct image *image, Elf_Phdr *phdr,
* @ret rc Return status code * @ret rc Return status code
*/ */
static int elfboot_probe ( struct image *image ) { static int elfboot_probe ( struct image *image ) {
Elf32_Ehdr ehdr; const Elf32_Ehdr *ehdr;
static const uint8_t e_ident[] = { static const uint8_t e_ident[] = {
[EI_MAG0] = ELFMAG0, [EI_MAG0] = ELFMAG0,
[EI_MAG1] = ELFMAG1, [EI_MAG1] = ELFMAG1,
@@ -122,14 +122,19 @@ static int elfboot_probe ( struct image *image ) {
int rc; int rc;
/* Read ELF header */ /* Read ELF header */
copy_from_user ( &ehdr, image->data, 0, sizeof ( ehdr ) ); if ( image->len < sizeof ( *ehdr ) ) {
if ( memcmp ( ehdr.e_ident, e_ident, sizeof ( e_ident ) ) != 0 ) { DBGC ( image, "ELF %s too short for ELF header\n",
image->name );
return -ENOEXEC;
}
ehdr = image->data;
if ( memcmp ( ehdr->e_ident, e_ident, sizeof ( e_ident ) ) != 0 ) {
DBGC ( image, "ELF %s invalid identifier\n", image->name ); DBGC ( image, "ELF %s invalid identifier\n", image->name );
return -ENOEXEC; return -ENOEXEC;
} }
/* Check that this image uses flat physical addressing */ /* Check that this image uses flat physical addressing */
if ( ( rc = elf_segments ( image, &ehdr, elfboot_check_segment, if ( ( rc = elf_segments ( image, ehdr, elfboot_check_segment,
&entry, &max ) ) != 0 ) { &entry, &max ) ) != 0 ) {
DBGC ( image, "ELF %s is not loadable: %s\n", DBGC ( image, "ELF %s is not loadable: %s\n",
image->name, strerror ( rc ) ); image->name, strerror ( rc ) );

View File

@@ -35,7 +35,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <assert.h> #include <assert.h>
#include <realmode.h> #include <realmode.h>
#include <multiboot.h> #include <multiboot.h>
#include <ipxe/uaccess.h>
#include <ipxe/image.h> #include <ipxe/image.h>
#include <ipxe/segment.h> #include <ipxe/segment.h>
#include <ipxe/io.h> #include <ipxe/io.h>
@@ -87,14 +86,6 @@ FEATURE ( FEATURE_IMAGE, "MBOOT", DHCP_EB_FEATURE_MULTIBOOT, 1 );
*/ */
#define MB_UNSUPPORTED_FLAGS ( MB_COMPULSORY_FLAGS & ~MB_SUPPORTED_FLAGS ) #define MB_UNSUPPORTED_FLAGS ( MB_COMPULSORY_FLAGS & ~MB_SUPPORTED_FLAGS )
/** A multiboot header descriptor */
struct multiboot_header_info {
/** The actual multiboot header */
struct multiboot_header mb;
/** Offset of header within the multiboot image */
size_t offset;
};
/** Multiboot module command lines */ /** Multiboot module command lines */
static char __bss16_array ( mb_cmdlines, [MB_MAX_CMDLINE] ); static char __bss16_array ( mb_cmdlines, [MB_MAX_CMDLINE] );
#define mb_cmdlines __use_data16 ( mb_cmdlines ) #define mb_cmdlines __use_data16 ( mb_cmdlines )
@@ -267,82 +258,89 @@ static struct multiboot_module __bss16_array ( mbmodules, [MAX_MODULES] );
* Find multiboot header * Find multiboot header
* *
* @v image Multiboot file * @v image Multiboot file
* @v hdr Multiboot header descriptor to fill in * @ret offset Offset to Multiboot header, or negative error
* @ret rc Return status code
*/ */
static int multiboot_find_header ( struct image *image, static int multiboot_find_header ( struct image *image ) {
struct multiboot_header_info *hdr ) { const struct multiboot_header *mb;
uint32_t buf[64];
size_t offset; size_t offset;
unsigned int buf_idx;
uint32_t checksum; uint32_t checksum;
/* Scan through first 8kB of image file 256 bytes at a time. /* Scan through first 8kB of image file */
* (Use the buffering to avoid the overhead of a for ( offset = 0 ; offset < 8192 ; offset += 4 ) {
* copy_from_user() for every dword.)
*/
for ( offset = 0 ; offset < 8192 ; offset += sizeof ( buf[0] ) ) {
/* Check for end of image */ /* Check for end of image */
if ( offset > image->len ) if ( ( offset + sizeof ( *mb ) ) > image->len )
break; break;
/* Refill buffer if applicable */ mb = ( image->data + offset );
buf_idx = ( ( offset % sizeof ( buf ) ) / sizeof ( buf[0] ) );
if ( buf_idx == 0 ) {
copy_from_user ( buf, image->data, offset,
sizeof ( buf ) );
}
/* Check signature */ /* Check signature */
if ( buf[buf_idx] != MULTIBOOT_HEADER_MAGIC ) if ( mb->magic != MULTIBOOT_HEADER_MAGIC )
continue; continue;
/* Copy header and verify checksum */ /* Copy header and verify checksum */
copy_from_user ( &hdr->mb, image->data, offset, checksum = ( mb->magic + mb->flags + mb->checksum );
sizeof ( hdr->mb ) );
checksum = ( hdr->mb.magic + hdr->mb.flags +
hdr->mb.checksum );
if ( checksum != 0 ) if ( checksum != 0 )
continue; continue;
/* Record offset of multiboot header and return */ /* Return header */
hdr->offset = offset; return offset;
return 0;
} }
/* No multiboot header found */ /* No multiboot header found */
DBGC ( image, "MULTIBOOT %s has no multiboot header\n",
image->name );
return -ENOEXEC; return -ENOEXEC;
} }
/** /**
* Load raw multiboot image into memory * Load raw multiboot image into memory
* *
* @v image Multiboot file * @v image Multiboot image
* @v hdr Multiboot header descriptor * @v offset Offset to Multiboot header
* @ret entry Entry point * @ret entry Entry point
* @ret max Maximum used address * @ret max Maximum used address
* @ret rc Return status code * @ret rc Return status code
*/ */
static int multiboot_load_raw ( struct image *image, static int multiboot_load_raw ( struct image *image, size_t offset,
struct multiboot_header_info *hdr,
physaddr_t *entry, physaddr_t *max ) { physaddr_t *entry, physaddr_t *max ) {
size_t offset; const struct multiboot_header *mb = ( image->data + offset );
size_t filesz; size_t filesz;
size_t memsz; size_t memsz;
userptr_t buffer; void *buffer;
int rc; int rc;
/* Sanity check */ /* Sanity check */
if ( ! ( hdr->mb.flags & MB_FLAG_RAW ) ) { if ( ! ( mb->flags & MB_FLAG_RAW ) ) {
DBGC ( image, "MULTIBOOT %s is not flagged as a raw image\n", DBGC ( image, "MULTIBOOT %s is not flagged as a raw image\n",
image->name ); image->name );
return -EINVAL; return -EINVAL;
} }
/* Verify and prepare segment */ /* Calculate starting offset within file */
offset = ( hdr->offset - hdr->mb.header_addr + hdr->mb.load_addr ); if ( ( mb->load_addr > mb->header_addr ) ||
filesz = ( hdr->mb.load_end_addr ? ( ( mb->header_addr - mb->load_addr ) > offset ) ) {
( hdr->mb.load_end_addr - hdr->mb.load_addr ) : DBGC ( image, "MULTIBOOT %s has misplaced header\n",
image->name );
return -EINVAL;
}
offset -= ( mb->header_addr - mb->load_addr );
assert ( offset < image->len );
/* Calculate length of initialized data */
filesz = ( mb->load_end_addr ?
( mb->load_end_addr - mb->load_addr ) :
( image->len - offset ) ); ( image->len - offset ) );
memsz = ( hdr->mb.bss_end_addr ? if ( filesz > image->len ) {
( hdr->mb.bss_end_addr - hdr->mb.load_addr ) : filesz ); DBGC ( image, "MULTIBOOT %s has overlength data\n",
buffer = phys_to_virt ( hdr->mb.load_addr ); image->name );
return -EINVAL;
}
/* Calculate length of uninitialised data */
memsz = ( mb->bss_end_addr ?
( mb->bss_end_addr - mb->load_addr ) : filesz );
DBGC ( image, "MULTIBOOT %s loading [%zx,%zx) to [%x,%zx,%zx)\n",
image->name, offset, ( offset + filesz ), mb->load_addr,
( mb->load_addr + filesz ), ( mb->load_addr + memsz ) );
/* Verify and prepare segment */
buffer = phys_to_virt ( mb->load_addr );
if ( ( rc = prep_segment ( buffer, filesz, memsz ) ) != 0 ) { if ( ( rc = prep_segment ( buffer, filesz, memsz ) ) != 0 ) {
DBGC ( image, "MULTIBOOT %s could not prepare segment: %s\n", DBGC ( image, "MULTIBOOT %s could not prepare segment: %s\n",
image->name, strerror ( rc ) ); image->name, strerror ( rc ) );
@@ -353,8 +351,8 @@ static int multiboot_load_raw ( struct image *image,
memcpy ( buffer, ( image->data + offset ), filesz ); memcpy ( buffer, ( image->data + offset ), filesz );
/* Record execution entry point and maximum used address */ /* Record execution entry point and maximum used address */
*entry = hdr->mb.entry_addr; *entry = mb->entry_addr;
*max = ( hdr->mb.load_addr + memsz ); *max = ( mb->load_addr + memsz );
return 0; return 0;
} }
@@ -388,22 +386,24 @@ static int multiboot_load_elf ( struct image *image, physaddr_t *entry,
* @ret rc Return status code * @ret rc Return status code
*/ */
static int multiboot_exec ( struct image *image ) { static int multiboot_exec ( struct image *image ) {
struct multiboot_header_info hdr; const struct multiboot_header *mb;
physaddr_t entry; physaddr_t entry;
physaddr_t max; physaddr_t max;
int offset;
int rc; int rc;
/* Locate multiboot header, if present */ /* Locate multiboot header, if present */
if ( ( rc = multiboot_find_header ( image, &hdr ) ) != 0 ) { offset = multiboot_find_header ( image );
DBGC ( image, "MULTIBOOT %s has no multiboot header\n", if ( offset < 0 ) {
image->name ); rc = offset;
return rc; return rc;
} }
mb = ( image->data + offset );
/* Abort if we detect flags that we cannot support */ /* Abort if we detect flags that we cannot support */
if ( hdr.mb.flags & MB_UNSUPPORTED_FLAGS ) { if ( mb->flags & MB_UNSUPPORTED_FLAGS ) {
DBGC ( image, "MULTIBOOT %s flags %08x not supported\n", DBGC ( image, "MULTIBOOT %s flags %#08x not supported\n",
image->name, ( hdr.mb.flags & MB_UNSUPPORTED_FLAGS ) ); image->name, ( mb->flags & MB_UNSUPPORTED_FLAGS ) );
return -ENOTSUP; return -ENOTSUP;
} }
@@ -413,8 +413,10 @@ static int multiboot_exec ( struct image *image ) {
* behaviour. * behaviour.
*/ */
if ( ( ( rc = multiboot_load_elf ( image, &entry, &max ) ) != 0 ) && if ( ( ( rc = multiboot_load_elf ( image, &entry, &max ) ) != 0 ) &&
( ( rc = multiboot_load_raw ( image, &hdr, &entry, &max ) ) != 0 )) ( ( rc = multiboot_load_raw ( image, offset, &entry,
&max ) ) != 0 ) ) {
return rc; return rc;
}
/* Populate multiboot information structure */ /* Populate multiboot information structure */
memset ( &mbinfo, 0, sizeof ( mbinfo ) ); memset ( &mbinfo, 0, sizeof ( mbinfo ) );
@@ -469,17 +471,19 @@ static int multiboot_exec ( struct image *image ) {
* @ret rc Return status code * @ret rc Return status code
*/ */
static int multiboot_probe ( struct image *image ) { static int multiboot_probe ( struct image *image ) {
struct multiboot_header_info hdr; const struct multiboot_header *mb;
int offset;
int rc; int rc;
/* Locate multiboot header, if present */ /* Locate multiboot header, if present */
if ( ( rc = multiboot_find_header ( image, &hdr ) ) != 0 ) { offset = multiboot_find_header ( image );
DBGC ( image, "MULTIBOOT %s has no multiboot header\n", if ( offset < 0 ) {
image->name ); rc = offset;
return rc; return rc;
} }
DBGC ( image, "MULTIBOOT %s found header with flags %08x\n", mb = ( image->data + offset );
image->name, hdr.mb.flags ); DBGC ( image, "MULTIBOOT %s found header at +%#x with flags %#08x\n",
image->name, offset, mb->flags );
return 0; return 0;
} }

View File

@@ -35,7 +35,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <errno.h> #include <errno.h>
#include <elf.h> #include <elf.h>
#include <ipxe/uaccess.h>
#include <ipxe/segment.h> #include <ipxe/segment.h>
#include <ipxe/image.h> #include <ipxe/image.h>
#include <ipxe/elf.h> #include <ipxe/elf.h>
@@ -48,9 +47,9 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
* @v dest Destination address * @v dest Destination address
* @ret rc Return status code * @ret rc Return status code
*/ */
static int elf_load_segment ( struct image *image, Elf_Phdr *phdr, static int elf_load_segment ( struct image *image, const Elf_Phdr *phdr,
physaddr_t dest ) { physaddr_t dest ) {
userptr_t buffer = phys_to_virt ( dest ); void *buffer = phys_to_virt ( dest );
int rc; int rc;
DBGC ( image, "ELF %s loading segment [%x,%x) to [%lx,%lx,%lx)\n", DBGC ( image, "ELF %s loading segment [%x,%x) to [%lx,%lx,%lx)\n",
@@ -82,9 +81,11 @@ static int elf_load_segment ( struct image *image, Elf_Phdr *phdr,
* @ret max Maximum used address * @ret max Maximum used address
* @ret rc Return status code * @ret rc Return status code
*/ */
static int elf_segment ( struct image *image, Elf_Ehdr *ehdr, Elf_Phdr *phdr, static int elf_segment ( struct image *image, const Elf_Ehdr *ehdr,
const Elf_Phdr *phdr,
int ( * process ) ( struct image *image, int ( * process ) ( struct image *image,
Elf_Phdr *phdr, physaddr_t dest ), const Elf_Phdr *phdr,
physaddr_t dest ),
physaddr_t *entry, physaddr_t *max ) { physaddr_t *entry, physaddr_t *max ) {
physaddr_t dest; physaddr_t dest;
physaddr_t end; physaddr_t end;
@@ -151,11 +152,12 @@ static int elf_segment ( struct image *image, Elf_Ehdr *ehdr, Elf_Phdr *phdr,
* @ret max Maximum used address * @ret max Maximum used address
* @ret rc Return status code * @ret rc Return status code
*/ */
int elf_segments ( struct image *image, Elf_Ehdr *ehdr, int elf_segments ( struct image *image, const Elf_Ehdr *ehdr,
int ( * process ) ( struct image *image, Elf_Phdr *phdr, int ( * process ) ( struct image *image,
const Elf_Phdr *phdr,
physaddr_t dest ), physaddr_t dest ),
physaddr_t *entry, physaddr_t *max ) { physaddr_t *entry, physaddr_t *max ) {
Elf_Phdr phdr; const Elf_Phdr *phdr;
Elf_Off phoff; Elf_Off phoff;
unsigned int phnum; unsigned int phnum;
int rc; int rc;
@@ -169,13 +171,14 @@ int elf_segments ( struct image *image, Elf_Ehdr *ehdr,
/* Read and process ELF program headers */ /* Read and process ELF program headers */
for ( phoff = ehdr->e_phoff , phnum = ehdr->e_phnum ; phnum ; for ( phoff = ehdr->e_phoff , phnum = ehdr->e_phnum ; phnum ;
phoff += ehdr->e_phentsize, phnum-- ) { phoff += ehdr->e_phentsize, phnum-- ) {
if ( phoff > image->len ) { if ( ( image->len < phoff ) ||
( ( image->len - phoff ) < sizeof ( *phdr ) ) ) {
DBGC ( image, "ELF %s program header %d outside " DBGC ( image, "ELF %s program header %d outside "
"image\n", image->name, phnum ); "image\n", image->name, phnum );
return -ENOEXEC; return -ENOEXEC;
} }
copy_from_user ( &phdr, image->data, phoff, sizeof ( phdr ) ); phdr = ( image->data + phoff );
if ( ( rc = elf_segment ( image, ehdr, &phdr, process, if ( ( rc = elf_segment ( image, ehdr, phdr, process,
entry, max ) ) != 0 ) entry, max ) ) != 0 )
return rc; return rc;
} }
@@ -206,19 +209,23 @@ int elf_load ( struct image *image, physaddr_t *entry, physaddr_t *max ) {
[EI_MAG3] = ELFMAG3, [EI_MAG3] = ELFMAG3,
[EI_CLASS] = ELFCLASS, [EI_CLASS] = ELFCLASS,
}; };
Elf_Ehdr ehdr; const Elf_Ehdr *ehdr;
int rc; int rc;
/* Read ELF header */ /* Read ELF header */
copy_from_user ( &ehdr, image->data, 0, sizeof ( ehdr ) ); if ( image->len < sizeof ( *ehdr ) ) {
if ( memcmp ( &ehdr.e_ident[EI_MAG0], e_ident, DBGC ( image, "ELF %s too short for ELF header\n",
sizeof ( e_ident ) ) != 0 ) { image->name );
return -ENOEXEC;
}
ehdr = image->data;
if ( memcmp ( ehdr->e_ident, e_ident, sizeof ( e_ident ) ) != 0 ) {
DBGC ( image, "ELF %s has invalid signature\n", image->name ); DBGC ( image, "ELF %s has invalid signature\n", image->name );
return -ENOEXEC; return -ENOEXEC;
} }
/* Load ELF segments into memory */ /* Load ELF segments into memory */
if ( ( rc = elf_segments ( image, &ehdr, elf_load_segment, if ( ( rc = elf_segments ( image, ehdr, elf_load_segment,
entry, max ) ) != 0 ) entry, max ) ) != 0 )
return rc; return rc;

View File

@@ -19,9 +19,10 @@ typedef Elf32_Phdr Elf_Phdr;
typedef Elf32_Off Elf_Off; typedef Elf32_Off Elf_Off;
#define ELFCLASS ELFCLASS32 #define ELFCLASS ELFCLASS32
extern int elf_segments ( struct image *image, Elf_Ehdr *ehdr, extern int elf_segments ( struct image *image, const Elf_Ehdr *ehdr,
int ( * process ) ( struct image *image, int ( * process ) ( struct image *image,
Elf_Phdr *phdr, physaddr_t dest ), const Elf_Phdr *phdr,
physaddr_t dest ),
physaddr_t *entry, physaddr_t *max ); physaddr_t *entry, physaddr_t *max );
extern int elf_load ( struct image *image, physaddr_t *entry, physaddr_t *max ); extern int elf_load ( struct image *image, physaddr_t *entry, physaddr_t *max );