On 01.04.2025 15:08, Roger Pau Monne wrote:
> --- a/xen/arch/x86/efi/mkreloc.c
> +++ b/xen/arch/x86/efi/mkreloc.c
> @@ -17,6 +17,12 @@
> #define PE_BASE_RELOC_HIGHLOW 3
> #define PE_BASE_RELOC_DIR64 10
>
> +/* The size of a symbol table entry is always 18 bytes. */
> +#define SYM_SIZE 18
> +
> +const char *string_table;
> +unsigned int string_table_size;
The way you use it, imo this wants to be uint32_t.
Both probably want to be static.
> @@ -25,6 +31,28 @@ static void usage(const char *cmd, int rc)
> exit(rc);
> }
>
> +const char *get_name(const char *name)
> +{
> + static char buffer[sizeof(((struct section_header *)NULL)->name) + 1] =
> {};
As this makes things section specific, may I suggest to name the function
e.g. get_section_name()? Also better to be static again.
> + unsigned long offset;
> +
> + if ( name[0] != '/' )
> + {
> + /*
> + * Use a temporary buffer in case the name is 8 characters long, as
> + * then there's no terminating null character in the input string.
> + */
> + strncpy(buffer, name, sizeof(buffer) - 1);
> + return buffer;
> + }
> +
> + offset = strtoul(&name[1], NULL, 10);
Don't you need to nul-terminate the string here, too, to play safe? (Yes,
we don't expect this big a string table.)
> + if ( !string_table || offset < 4 || offset >= string_table_size )
Considering how you reduce string_table_size after having read it from
the image, don't you mean "offset - 4 >= string_table_size" here? Also
below you use sizeof(string_table_size) instead of a literal 4.
> + return name;
> +
> + return &string_table[offset - 4];
Here as well.
> @@ -83,6 +111,31 @@ static unsigned int load(const char *name, int *handle,
> exit(3);
> }
>
> + if ( !string_table && pe_hdr.symbol_table )
> + {
> + char *strings;
> +
> + if ( lseek(in, pe_hdr.symbol_table + pe_hdr.symbols * SYM_SIZE,
> + SEEK_SET) < 0 ||
> + read(in, &string_table_size, sizeof(string_table_size)) !=
> + sizeof(string_table_size) )
> + {
> + perror(name);
> + exit(3);
> + }
> +
> + string_table_size -= sizeof(string_table_size);
You're careful of underflow above; better be careful here, too?
> + strings = malloc(string_table_size);
You check for all other errors, just not here?
Jan
