From: Michael Brown Date: Wed, 23 Apr 2025 08:53:38 +0000 (+0100) Subject: [smbios] Remove userptr_t from SMBIOS structure parsing X-Git-Tag: rolling/bin~387 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0bf0f8716a3c7f85455707d8c2d727d130ee1024;p=thirdparty%2Fipxe.git [smbios] Remove userptr_t from SMBIOS structure parsing Simplify the SMBIOS structure parsing code by assuming that all structure content is fully accessible via pointer dereferences. In particular, this allows the convoluted find_smbios_structure() and read_smbios_structure() to be combined into a single function smbios_structure() that just returns a direct pointer to the SMBIOS structure, with smbios_string() similarly now returning a direct pointer to the relevant string. Signed-off-by: Michael Brown --- diff --git a/src/arch/x86/interface/pcbios/bios_smbios.c b/src/arch/x86/interface/pcbios/bios_smbios.c index e43c74bad..aaec1eea1 100644 --- a/src/arch/x86/interface/pcbios/bios_smbios.c +++ b/src/arch/x86/interface/pcbios/bios_smbios.c @@ -45,19 +45,18 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); * @ret rc Return status code */ static int bios_find_smbios2 ( struct smbios *smbios ) { - struct smbios_entry entry; - int rc; + const struct smbios_entry *entry; /* Scan through BIOS segment to find SMBIOS 32-bit entry point */ - if ( ( rc = find_smbios_entry ( real_to_virt ( BIOS_SEG, 0 ), 0x10000, - &entry ) ) != 0 ) - return rc; + entry = find_smbios_entry ( real_to_virt ( BIOS_SEG, 0 ), 0x10000 ); + if ( ! entry ) + return -ENOENT; /* Fill in entry point descriptor structure */ - smbios->address = phys_to_virt ( entry.smbios_address ); - smbios->len = entry.smbios_len; - smbios->count = entry.smbios_count; - smbios->version = SMBIOS_VERSION ( entry.major, entry.minor ); + smbios->address = phys_to_virt ( entry->smbios_address ); + smbios->len = entry->smbios_len; + smbios->count = entry->smbios_count; + smbios->version = SMBIOS_VERSION ( entry->major, entry->minor ); return 0; } @@ -69,26 +68,25 @@ static int bios_find_smbios2 ( struct smbios *smbios ) { * @ret rc Return status code */ static int bios_find_smbios3 ( struct smbios *smbios ) { - struct smbios3_entry entry; - int rc; + const struct smbios3_entry *entry; /* Scan through BIOS segment to find SMBIOS 64-bit entry point */ - if ( ( rc = find_smbios3_entry ( real_to_virt ( BIOS_SEG, 0 ), 0x10000, - &entry ) ) != 0 ) - return rc; + entry = find_smbios3_entry ( real_to_virt ( BIOS_SEG, 0 ), 0x10000 ); + if ( ! entry ) + return -ENOENT; /* Check that address is accessible */ - if ( entry.smbios_address > ~( ( physaddr_t ) 0 ) ) { + if ( entry->smbios_address > ~( ( physaddr_t ) 0 ) ) { DBG ( "SMBIOS3 at %08llx is inaccessible\n", - ( ( unsigned long long ) entry.smbios_address ) ); + ( ( unsigned long long ) entry->smbios_address ) ); return -ENOTSUP; } /* Fill in entry point descriptor structure */ - smbios->address = phys_to_virt ( entry.smbios_address ); - smbios->len = entry.smbios_len; + smbios->address = phys_to_virt ( entry->smbios_address ); + smbios->len = entry->smbios_len; smbios->count = 0; - smbios->version = SMBIOS_VERSION ( entry.major, entry.minor ); + smbios->version = SMBIOS_VERSION ( entry->major, entry->minor ); return 0; } diff --git a/src/drivers/net/smsc95xx.c b/src/drivers/net/smsc95xx.c index 3ec49584d..0210e9240 100644 --- a/src/drivers/net/smsc95xx.c +++ b/src/drivers/net/smsc95xx.c @@ -64,92 +64,67 @@ static struct profiler smsc95xx_out_profiler __profiler = */ static int smsc95xx_vm3_fetch_mac ( struct smscusb_device *smscusb ) { struct net_device *netdev = smscusb->netdev; - struct smbios_structure structure; - struct smbios_system_information system; - struct { - char manufacturer[ 10 /* "Honeywell" + NUL */ ]; - char product[ 4 /* "VM3" + NUL */ ]; - char mac[ base16_encoded_len ( ETH_ALEN ) + 1 /* NUL */ ]; - } strings; + const struct smbios_header *structure; + const struct smbios_system_information *system; + const char *manufacturer; + const char *product; + const char *mac; int len; int rc; /* Find system information */ - if ( ( rc = find_smbios_structure ( SMBIOS_TYPE_SYSTEM_INFORMATION, 0, - &structure ) ) != 0 ) { + structure = smbios_structure ( SMBIOS_TYPE_SYSTEM_INFORMATION, 0 ); + if ( ! structure ) { DBGC ( smscusb, "SMSC95XX %p could not find system " - "information: %s\n", smscusb, strerror ( rc ) ); - return rc; - } - - /* Read system information */ - if ( ( rc = read_smbios_structure ( &structure, &system, - sizeof ( system ) ) ) != 0 ) { - DBGC ( smscusb, "SMSC95XX %p could not read system " - "information: %s\n", smscusb, strerror ( rc ) ); - return rc; + "information\n", smscusb ); + return -ENOENT; } - - /* NUL-terminate all strings to be fetched */ - memset ( &strings, 0, sizeof ( strings ) ); + system = container_of ( structure, struct smbios_system_information, + header ); /* Fetch system manufacturer name */ - len = read_smbios_string ( &structure, system.manufacturer, - strings.manufacturer, - ( sizeof ( strings.manufacturer ) - 1 ) ); - if ( len < 0 ) { - rc = len; + manufacturer = smbios_string ( structure, system->manufacturer ); + if ( ! manufacturer ) { DBGC ( smscusb, "SMSC95XX %p could not read manufacturer " - "name: %s\n", smscusb, strerror ( rc ) ); - return rc; + "name\n", smscusb ); + return -ENOENT; } /* Fetch system product name */ - len = read_smbios_string ( &structure, system.product, strings.product, - ( sizeof ( strings.product ) - 1 ) ); - if ( len < 0 ) { - rc = len; - DBGC ( smscusb, "SMSC95XX %p could not read product name: " - "%s\n", smscusb, strerror ( rc ) ); - return rc; + product = smbios_string ( structure, system->product ); + if ( ! product ) { + DBGC ( smscusb, "SMSC95XX %p could not read product name\n", + smscusb ); + return -ENOENT; } /* Ignore non-VM3 devices */ - if ( ( strcmp ( strings.manufacturer, "Honeywell" ) != 0 ) || - ( strcmp ( strings.product, "VM3" ) != 0 ) ) + if ( ( strcmp ( manufacturer, "Honeywell" ) != 0 ) || + ( strcmp ( product, "VM3" ) != 0 ) ) return -ENOTTY; /* Find OEM strings */ - if ( ( rc = find_smbios_structure ( SMBIOS_TYPE_OEM_STRINGS, 0, - &structure ) ) != 0 ) { - DBGC ( smscusb, "SMSC95XX %p could not find OEM strings: %s\n", - smscusb, strerror ( rc ) ); - return rc; + structure = smbios_structure ( SMBIOS_TYPE_OEM_STRINGS, 0 ); + if ( ! structure ) { + DBGC ( smscusb, "SMSC95XX %p could not find OEM strings\n", + smscusb ); + return -ENOENT; } /* Fetch MAC address */ - len = read_smbios_string ( &structure, SMSC95XX_VM3_OEM_STRING_MAC, - strings.mac, ( sizeof ( strings.mac ) - 1 )); - if ( len < 0 ) { - rc = len; - DBGC ( smscusb, "SMSC95XX %p could not read OEM string: %s\n", - smscusb, strerror ( rc ) ); - return rc; - } - - /* Sanity check */ - if ( len != ( ( int ) ( sizeof ( strings.mac ) - 1 ) ) ) { - DBGC ( smscusb, "SMSC95XX %p invalid MAC address \"%s\"\n", - smscusb, strings.mac ); - return -EINVAL; + mac = smbios_string ( structure, SMSC95XX_VM3_OEM_STRING_MAC ); + if ( ! mac ) { + DBGC ( smscusb, "SMSC95XX %p could not read OEM string\n", + smscusb ); + return -ENOENT; } /* Decode MAC address */ - len = base16_decode ( strings.mac, netdev->hw_addr, ETH_ALEN ); + len = base16_decode ( mac, netdev->hw_addr, ETH_ALEN ); if ( len < 0 ) { rc = len; DBGC ( smscusb, "SMSC95XX %p invalid MAC address \"%s\"\n", - smscusb, strings.mac ); + smscusb, mac ); return rc; } diff --git a/src/include/ipxe/smbios.h b/src/include/ipxe/smbios.h index f36a5ad40..d9e2c38ed 100644 --- a/src/include/ipxe/smbios.h +++ b/src/include/ipxe/smbios.h @@ -12,7 +12,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); #include #include #include -#include /** * Provide an SMBIOS API implementation @@ -126,16 +125,6 @@ struct smbios_header { uint16_t handle; } __attribute__ (( packed )); -/** SMBIOS structure descriptor */ -struct smbios_structure { - /** Copy of SMBIOS structure header */ - struct smbios_header header; - /** Offset of structure within SMBIOS */ - size_t offset; - /** Length of strings section */ - size_t strings_len; -}; - /** SMBIOS system information structure */ struct smbios_system_information { /** SMBIOS structure header */ @@ -207,7 +196,7 @@ struct smbios_enclosure_information { */ struct smbios { /** Start of SMBIOS structures */ - userptr_t address; + const void *address; /** Length of SMBIOS structures */ size_t len; /** Number of SMBIOS structures */ @@ -226,17 +215,14 @@ struct smbios { #define SMBIOS_VERSION( major, minor ) ( ( (major) << 8 ) | (minor) ) extern int find_smbios ( struct smbios *smbios ); -extern int find_smbios_entry ( userptr_t start, size_t len, - struct smbios_entry *entry ); -extern int find_smbios3_entry ( userptr_t start, size_t len, - struct smbios3_entry *entry ); -extern int find_smbios_structure ( unsigned int type, unsigned int instance, - struct smbios_structure *structure ); -extern int read_smbios_structure ( struct smbios_structure *structure, - void *data, size_t len ); -extern int read_smbios_string ( struct smbios_structure *structure, - unsigned int index, - void *data, size_t len ); +extern const struct smbios_entry * find_smbios_entry ( const void *start, + size_t len ); +extern const struct smbios3_entry * find_smbios3_entry ( const void *start, + size_t len ); +extern const struct smbios_header * smbios_structure ( unsigned int type, + unsigned int instance ); +extern const char * smbios_string ( const struct smbios_header *header, + unsigned int index ); extern int smbios_version ( void ); extern void smbios_clear ( void ); diff --git a/src/interface/efi/efi_smbios.c b/src/interface/efi/efi_smbios.c index 3c1b77bdc..5d0e69d6b 100644 --- a/src/interface/efi/efi_smbios.c +++ b/src/interface/efi/efi_smbios.c @@ -20,6 +20,7 @@ FILE_LICENCE ( GPL2_OR_LATER ); #include +#include #include #include #include diff --git a/src/interface/linux/linux_smbios.c b/src/interface/linux/linux_smbios.c index abe1b19d7..a12c936ed 100644 --- a/src/interface/linux/linux_smbios.c +++ b/src/interface/linux/linux_smbios.c @@ -35,7 +35,7 @@ static const char smbios_entry_filename[] = static const char smbios_filename[] = "/sys/firmware/dmi/tables/DMI"; /** Cache SMBIOS data */ -static userptr_t smbios_data; +static void *smbios_data; /** * Find SMBIOS @@ -46,7 +46,7 @@ static userptr_t smbios_data; static int linux_find_smbios ( struct smbios *smbios ) { struct smbios3_entry *smbios3_entry; struct smbios_entry *smbios_entry; - userptr_t entry; + void *entry; void *data; int len; int rc; @@ -98,6 +98,7 @@ static int linux_find_smbios ( struct smbios *smbios ) { return 0; ufree ( smbios_data ); + smbios_data = NULL; err_read: err_version: ufree ( entry ); @@ -116,6 +117,7 @@ static void linux_smbios_shutdown ( int booting __unused ) { /* Free SMBIOS data */ ufree ( smbios_data ); + smbios_data = NULL; } /** SMBIOS shutdown function */ diff --git a/src/interface/smbios/smbios.c b/src/interface/smbios/smbios.c index 89fa4d7ca..3a1a98b17 100644 --- a/src/interface/smbios/smbios.c +++ b/src/interface/smbios/smbios.c @@ -38,26 +38,24 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); /** SMBIOS entry point descriptor */ static struct smbios smbios = { - .address = UNULL, + .address = NULL, }; /** * Calculate SMBIOS entry point structure checksum * * @v start Start address of region - * @v offset Offset of SMBIOS entry point structure * @v len Length of entry point structure * @ret sum Byte checksum */ -static uint8_t smbios_checksum ( userptr_t start, size_t offset, size_t len ) { - size_t end = ( offset + len ); - uint8_t sum; - uint8_t byte; +static uint8_t smbios_checksum ( const void *start, size_t len ) { + const uint8_t *byte = start; + uint8_t sum = 0; + + /* Compute checksum */ + while ( len-- ) + sum += *(byte++); - for ( sum = 0 ; offset < end ; offset++ ) { - copy_from_user ( &byte, start, offset, sizeof ( byte ) ); - sum += byte; - } return sum; } @@ -66,39 +64,45 @@ static uint8_t smbios_checksum ( userptr_t start, size_t offset, size_t len ) { * * @v start Start address of region to scan * @v len Length of region to scan - * @v entry SMBIOS entry point structure to fill in - * @ret rc Return status code + * @ret entry SMBIOS entry point structure, or NULL if not found */ -int find_smbios_entry ( userptr_t start, size_t len, - struct smbios_entry *entry ) { +const struct smbios_entry * find_smbios_entry ( const void *start, + size_t len ) { static size_t offset = 0; /* Avoid repeated attempts to locate SMBIOS */ + const struct smbios_entry *entry; uint8_t sum; /* Try to find SMBIOS */ for ( ; ( offset + sizeof ( *entry ) ) <= len ; offset += 0x10 ) { - /* Read start of header and verify signature */ - copy_from_user ( entry, start, offset, sizeof ( *entry ) ); + /* Verify signature */ + entry = ( start + offset ); if ( entry->signature != SMBIOS_SIGNATURE ) continue; + /* Verify length */ + if ( ( entry->len < sizeof ( *entry ) ) || + ( ( offset + entry->len ) > len ) ) { + DBGC ( &smbios, "SMBIOS at %#08lx has bad length " + "%#02x\n", virt_to_phys ( entry ), entry->len ); + continue; + } + /* Verify checksum */ - if ( ( sum = smbios_checksum ( start, offset, - entry->len ) ) != 0 ) { - DBG ( "SMBIOS at %08lx has bad checksum %02x\n", - virt_to_phys ( start + offset ), sum ); + if ( ( sum = smbios_checksum ( entry, entry->len ) ) != 0 ) { + DBGC ( &smbios, "SMBIOS at %#08lx has bad checksum " + "%#02x\n", virt_to_phys ( entry ), sum ); continue; } /* Fill result structure */ - DBG ( "Found SMBIOS v%d.%d entry point at %08lx\n", - entry->major, entry->minor, - virt_to_phys ( start + offset ) ); - return 0; + DBGC ( &smbios, "Found SMBIOS v%d.%d entry point at %#08lx\n", + entry->major, entry->minor, virt_to_phys ( entry ) ); + return entry; } - DBG ( "No SMBIOS found\n" ); - return -ENODEV; + DBGC ( &smbios, "No SMBIOS found\n" ); + return NULL; } /** @@ -106,39 +110,45 @@ int find_smbios_entry ( userptr_t start, size_t len, * * @v start Start address of region to scan * @v len Length of region to scan - * @v entry SMBIOS entry point structure to fill in - * @ret rc Return status code + * @ret entry SMBIOS entry point structure, or NULL if not found */ -int find_smbios3_entry ( userptr_t start, size_t len, - struct smbios3_entry *entry ) { +const struct smbios3_entry * find_smbios3_entry ( const void *start, + size_t len ) { static size_t offset = 0; /* Avoid repeated attempts to locate SMBIOS */ + const struct smbios3_entry *entry; uint8_t sum; /* Try to find SMBIOS */ for ( ; ( offset + sizeof ( *entry ) ) <= len ; offset += 0x10 ) { - /* Read start of header and verify signature */ - copy_from_user ( entry, start, offset, sizeof ( *entry ) ); + /* Verify signature */ + entry = ( start + offset ); if ( entry->signature != SMBIOS3_SIGNATURE ) continue; + /* Verify length */ + if ( ( entry->len < sizeof ( *entry ) ) || + ( ( offset + entry->len ) > len ) ) { + DBGC ( &smbios, "SMBIOS at %#08lx has bad length " + "%#02x\n", virt_to_phys ( entry ), entry->len ); + continue; + } + /* Verify checksum */ - if ( ( sum = smbios_checksum ( start, offset, - entry->len ) ) != 0 ) { - DBG ( "SMBIOS3 at %08lx has bad checksum %02x\n", - virt_to_phys ( start + offset ), sum ); + if ( ( sum = smbios_checksum ( entry, entry->len ) ) != 0 ) { + DBGC ( &smbios, "SMBIOS3 at %#08lx has bad checksum " + "%#02x\n", virt_to_phys ( entry ), sum ); continue; } /* Fill result structure */ - DBG ( "Found SMBIOS3 v%d.%d entry point at %08lx\n", - entry->major, entry->minor, - virt_to_phys ( start + offset ) ); - return 0; + DBGC ( &smbios, "Found SMBIOS3 v%d.%d entry point at %#08lx\n", + entry->major, entry->minor, virt_to_phys ( entry ) ); + return entry; } - DBG ( "No SMBIOS3 found\n" ); - return -ENODEV; + DBGC ( &smbios, "No SMBIOS3 found\n" ); + return NULL; } /** @@ -148,12 +158,15 @@ int find_smbios3_entry ( userptr_t start, size_t len, * @ret offset Offset to strings terminator, or 0 if not found */ static size_t find_strings_terminator ( size_t offset ) { - size_t max_offset = ( smbios.len - 2 ); - uint16_t nulnul; + const uint16_t *nulnul __attribute__ (( aligned ( 1 ) )); - for ( ; offset <= max_offset ; offset++ ) { - copy_from_user ( &nulnul, smbios.address, offset, 2 ); - if ( nulnul == 0 ) + /* Sanity checks */ + assert ( smbios.address != NULL ); + + /* Check for presence of terminating empty string */ + for ( ; ( offset + sizeof ( *nulnul ) ) <= smbios.len ; offset++ ) { + nulnul = ( smbios.address + offset ); + if ( *nulnul == 0 ) return ( offset + 1 ); } return 0; @@ -164,61 +177,59 @@ static size_t find_strings_terminator ( size_t offset ) { * * @v type Structure type to search for * @v instance Instance of this type of structure - * @v structure SMBIOS structure descriptor to fill in - * @ret rc Return status code + * @ret structure SMBIOS structure header, or NULL if not found */ -int find_smbios_structure ( unsigned int type, unsigned int instance, - struct smbios_structure *structure ) { +const struct smbios_header * smbios_structure ( unsigned int type, + unsigned int instance ) { + const struct smbios_header *structure; unsigned int count = 0; size_t offset = 0; size_t strings_offset; size_t terminator_offset; + size_t strings_len; int rc; /* Find SMBIOS */ - if ( ( smbios.address == UNULL ) && + if ( ( smbios.address == NULL ) && ( ( rc = find_smbios ( &smbios ) ) != 0 ) ) - return rc; - assert ( smbios.address != UNULL ); + return NULL; + assert ( smbios.address != NULL ); /* Scan through list of structures */ - while ( ( ( offset + sizeof ( structure->header ) ) < smbios.len ) && + while ( ( ( offset + sizeof ( *structure ) ) < smbios.len ) && ( ( smbios.count == 0 ) || ( count < smbios.count ) ) ) { - /* Read next SMBIOS structure header */ - copy_from_user ( &structure->header, smbios.address, offset, - sizeof ( structure->header ) ); + /* Access next SMBIOS structure header */ + structure = ( smbios.address + offset ); /* Determine start and extent of strings block */ - strings_offset = ( offset + structure->header.len ); + strings_offset = ( offset + structure->len ); if ( strings_offset > smbios.len ) { - DBG ( "SMBIOS structure at offset %zx with length " - "%x extends beyond SMBIOS\n", offset, - structure->header.len ); - return -ENOENT; + DBGC ( &smbios, "SMBIOS structure at offset %#zx " + "with length %#x extends beyond SMBIOS\n", + offset, structure->len ); + return NULL; } terminator_offset = find_strings_terminator ( strings_offset ); if ( ! terminator_offset ) { - DBG ( "SMBIOS structure at offset %zx has " - "unterminated strings section\n", offset ); - return -ENOENT; + DBGC ( &smbios, "SMBIOS structure at offset %#zx has " + "unterminated strings section\n", offset ); + return NULL; } - structure->strings_len = ( terminator_offset - strings_offset); - - DBG ( "SMBIOS structure at offset %zx has type %d, length %x, " - "strings length %zx\n", offset, structure->header.type, - structure->header.len, structure->strings_len ); + strings_len = ( terminator_offset - strings_offset); + DBGC ( &smbios, "SMBIOS structure at offset %#zx has type %d, " + "length %#x, strings length %#zx\n", offset, + structure->type, structure->len, strings_len ); /* Stop if we have reached an end-of-table marker */ if ( ( smbios.count == 0 ) && - ( structure->header.type == SMBIOS_TYPE_END ) ) + ( structure->type == SMBIOS_TYPE_END ) ) break; /* If this is the structure we want, return */ - if ( ( structure->header.type == type ) && + if ( ( structure->type == type ) && ( instance-- == 0 ) ) { - structure->offset = offset; - return 0; + return structure; } /* Move to next SMBIOS structure */ @@ -226,69 +237,43 @@ int find_smbios_structure ( unsigned int type, unsigned int instance, count++; } - DBG ( "SMBIOS structure type %d not found\n", type ); - return -ENOENT; + DBGC ( &smbios, "SMBIOS structure type %d not found\n", type ); + return NULL; } /** - * Copy SMBIOS structure + * Get indexed string within SMBIOS structure * - * @v structure SMBIOS structure descriptor - * @v data Buffer to hold SMBIOS structure - * @v len Length of buffer - * @ret rc Return status code - */ -int read_smbios_structure ( struct smbios_structure *structure, - void *data, size_t len ) { - - assert ( smbios.address != UNULL ); - - if ( len > structure->header.len ) - len = structure->header.len; - copy_from_user ( data, smbios.address, structure->offset, len ); - return 0; -} - -/** - * Find indexed string within SMBIOS structure - * - * @v structure SMBIOS structure descriptor + * @v structure SMBIOS structure header * @v index String index - * @v data Buffer for string - * @v len Length of string buffer - * @ret rc Length of string, or negative error + * @ret string SMBIOS string, or NULL if not fond */ -int read_smbios_string ( struct smbios_structure *structure, - unsigned int index, void *data, size_t len ) { - size_t strings_start = ( structure->offset + structure->header.len ); - size_t strings_end = ( strings_start + structure->strings_len ); - size_t offset; - size_t string_len; - - assert ( smbios.address != UNULL ); - - /* String numbers start at 1 (0 is used to indicate "no string") */ - if ( ! index ) - return -ENOENT; - - for ( offset = strings_start ; offset < strings_end ; - offset += ( string_len + 1 ) ) { - /* Get string length. This is known safe, since the - * smbios_strings struct is constructed so as to - * always end on a string boundary. +const char * smbios_string ( const struct smbios_header *structure, + unsigned int index ) { + const char *string; + unsigned int i; + size_t len; + + /* Sanity check */ + assert ( smbios.address != NULL ); + + /* Step through strings */ + string = ( ( ( const void * ) structure ) + structure->len ); + for ( i = index ; i-- ; ) { + /* Get string length. This is known safe, since we + * check for the empty-string terminator in + * smbios_structure(). */ - string_len = strlen ( smbios.address + offset ); - if ( --index == 0 ) { - /* Copy string, truncating as necessary. */ - if ( len > string_len ) - len = string_len; - copy_from_user ( data, smbios.address, offset, len ); - return string_len; - } + len = strlen ( string ); + if ( ! len ) + break; + if ( i == 0 ) + return string; + string += ( len + 1 /* NUL */ ); } - DBG ( "SMBIOS string index %d not found\n", index ); - return -ENOENT; + DBGC ( &smbios, "SMBIOS string index %d not found\n", index ); + return NULL; } /** @@ -300,10 +285,10 @@ int smbios_version ( void ) { int rc; /* Find SMBIOS */ - if ( ( smbios.address == UNULL ) && + if ( ( smbios.address == NULL ) && ( ( rc = find_smbios ( &smbios ) ) != 0 ) ) return rc; - assert ( smbios.address != UNULL ); + assert ( smbios.address != NULL ); return smbios.version; } @@ -315,5 +300,5 @@ int smbios_version ( void ) { void smbios_clear ( void ) { /* Clear address */ - smbios.address = UNULL; + smbios.address = NULL; } diff --git a/src/interface/smbios/smbios_settings.c b/src/interface/smbios/smbios_settings.c index 095c35d37..1fe545f38 100644 --- a/src/interface/smbios/smbios_settings.c +++ b/src/interface/smbios/smbios_settings.c @@ -81,15 +81,17 @@ static int smbios_applies ( struct settings *settings __unused, * @v len Length of buffer * @ret len Length of setting data, or negative error */ -static int smbios_fetch ( struct settings *settings __unused, - struct setting *setting, +static int smbios_fetch ( struct settings *settings, struct setting *setting, void *data, size_t len ) { - struct smbios_structure structure; + const struct smbios_header *structure; unsigned int tag_instance; unsigned int tag_type; unsigned int tag_offset; unsigned int tag_len; - int rc; + const void *src; + size_t src_len; + unsigned int string; + union uuid uuid; /* Split tag into instance, type, offset and length */ tag_instance = ( ( setting->tag >> 24 ) & 0xff ); @@ -98,81 +100,75 @@ static int smbios_fetch ( struct settings *settings __unused, tag_len = ( setting->tag & 0xff ); /* Find SMBIOS structure */ - if ( ( rc = find_smbios_structure ( tag_type, tag_instance, - &structure ) ) != 0 ) - return rc; - - { - uint8_t buf[structure.header.len]; - const void *raw; - union uuid uuid; - unsigned int index; - - /* Read SMBIOS structure */ - if ( ( rc = read_smbios_structure ( &structure, buf, - sizeof ( buf ) ) ) != 0 ) - return rc; + structure = smbios_structure ( tag_type, tag_instance ); + if ( ! structure ) + return -ENOENT; + src = structure; + src_len = structure->len; + string = 0; - /* A of zero indicates that the byte at - * contains a string index. An of - * zero indicates that the contains a literal - * string index. - * - * Since the byte at offset zero can never contain a - * string index, and a literal string index can never - * be zero, the combination of both and - * being zero indicates that the entire - * structure is to be read. - */ - if ( ( tag_len == 0 ) && ( tag_offset == 0 ) ) { - tag_len = sizeof ( buf ); - } else if ( ( tag_len == 0 ) || ( tag_offset == 0 ) ) { - index = ( ( tag_offset == 0 ) ? - tag_len : buf[tag_offset] ); - if ( ( rc = read_smbios_string ( &structure, index, - data, len ) ) < 0 ) { - return rc; - } - if ( ! setting->type ) - setting->type = &setting_type_string; - return rc; - } + /* A of zero indicates that the byte at + * contains a string index. An of zero indicates + * that the contains a literal string index. + * + * Since the byte at offset zero can never contain a string + * index, and a literal string index can never be zero, the + * combination of both and being zero + * indicates that the entire structure is to be read. + */ + if ( ( tag_len == 0 ) && ( tag_offset == 0 ) ) { + /* Read whole structure */ + } else if ( ( tag_len == 0 ) || ( tag_offset == 0 ) ) { + /* Read string */ + string = tag_len; + if ( ( string == 0 ) && ( tag_offset < src_len ) ) + string = *( ( uint8_t * ) src + tag_offset ); + src = smbios_string ( structure, string ); + if ( ! src ) + return -ENOENT; + assert ( string > 0 ); + src_len = strlen ( src ); + } else if ( tag_offset > src_len ) { + /* Empty read beyond end of structure */ + src_len = 0; + } else { + /* Read partial structure */ + src += tag_offset; + src_len -= tag_offset; + if ( src_len > tag_len ) + src_len = tag_len; + } - /* Limit length */ - if ( tag_offset > sizeof ( buf ) ) { - tag_len = 0; - } else if ( ( tag_offset + tag_len ) > sizeof ( buf ) ) { - tag_len = ( sizeof ( buf ) - tag_offset ); - } + /* Mangle UUIDs if necessary. iPXE treats UUIDs as being in + * network byte order (big-endian). SMBIOS specification + * version 2.6 states that UUIDs are stored with little-endian + * values in the first three fields; earlier versions did not + * specify an endianness. dmidecode assumes that the byte + * order is little-endian if and only if the SMBIOS version is + * 2.6 or higher; we match this behaviour. + */ + if ( ( ( setting->type == &setting_type_uuid ) || + ( setting->type == &setting_type_guid ) ) && + ( src_len == sizeof ( uuid ) ) && + ( smbios_version() >= SMBIOS_VERSION ( 2, 6 ) ) ) { + DBGC ( settings, "SMBIOS detected mangled UUID\n" ); + memcpy ( &uuid, src, sizeof ( uuid ) ); + uuid_mangle ( &uuid ); + src = &uuid; + } - /* Mangle UUIDs if necessary. iPXE treats UUIDs as - * being in network byte order (big-endian). SMBIOS - * specification version 2.6 states that UUIDs are - * stored with little-endian values in the first three - * fields; earlier versions did not specify an - * endianness. dmidecode assumes that the byte order - * is little-endian if and only if the SMBIOS version - * is 2.6 or higher; we match this behaviour. - */ - raw = &buf[tag_offset]; - if ( ( ( setting->type == &setting_type_uuid ) || - ( setting->type == &setting_type_guid ) ) && - ( tag_len == sizeof ( uuid ) ) && - ( smbios_version() >= SMBIOS_VERSION ( 2, 6 ) ) ) { - DBG ( "SMBIOS detected mangled UUID\n" ); - memcpy ( &uuid, &buf[tag_offset], sizeof ( uuid ) ); - uuid_mangle ( &uuid ); - raw = &uuid; - } + /* Return data */ + if ( len > src_len ) + len = src_len; + memcpy ( data, src, len ); - /* Return data */ - if ( len > tag_len ) - len = tag_len; - memcpy ( data, raw, len ); - if ( ! setting->type ) - setting->type = &setting_type_hex; - return tag_len; + /* Set default type */ + if ( ! setting->type ) { + setting->type = ( string ? &setting_type_string : + &setting_type_hex ); } + + return src_len; } /** SMBIOS settings operations */ @@ -192,12 +188,12 @@ static struct settings smbios_settings = { /** Initialise SMBIOS settings */ static void smbios_init ( void ) { + struct settings *settings = &smbios_settings; int rc; - if ( ( rc = register_settings ( &smbios_settings, NULL, - "smbios" ) ) != 0 ) { - DBG ( "SMBIOS could not register settings: %s\n", - strerror ( rc ) ); + if ( ( rc = register_settings ( settings, NULL, "smbios" ) ) != 0 ) { + DBGC ( settings, "SMBIOS could not register settings: %s\n", + strerror ( rc ) ); return; } }