[SLOF] [PATCH 1/3] elf: Implement elf_get_file_size to determine size of an ELF image
Stefan Berger
stefanb at linux.ibm.com
Thu Apr 2 00:31:30 AEDT 2020
On 4/1/20 12:31 AM, Alexey Kardashevskiy wrote:
>
> On 27/03/2020 07:20, Stefan Berger wrote:
>> From: Stefan Berger <stefanb at linux.ibm.com>
>>
>> Implement elf_get_file_size to determine the size of an ELF image
>> that has been loaded into a buffer much larger than the actual size
>> of the original file. We determine the size by searching for the
>> farthest offset declared by the ELF headers.
>>
>> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
>> ---
>> include/byteorder.h | 14 +++++++++
>> include/helpers.h | 2 ++
>> include/libelf.h | 4 +++
>> lib/libelf/elf.c | 28 +++++++++++++++++
>> lib/libelf/elf32.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
>> lib/libelf/elf64.c | 63 +++++++++++++++++++++++++++++++++++++
>> 6 files changed, 186 insertions(+)
>>
>> diff --git a/include/byteorder.h b/include/byteorder.h
>> index d4a2c8c..d5ebd61 100644
>> --- a/include/byteorder.h
>> +++ b/include/byteorder.h
>> @@ -53,6 +53,20 @@ static inline void bswap_64p (uint64_t *x)
>> *x = __builtin_bswap64(*x);
>> }
>>
>> +static inline uint16_t cond_bswap_16 (uint16_t x, int swap)
>> +{
>> + return swap ? bswap_16(x) : x;
>> +}
>> +
>> +static inline uint32_t cond_bswap_32 (uint32_t x, int swap)
>> +{
>> + return swap ? bswap_32(x) : x;
>> +}
>> +
>> +static inline uint64_t cond_bswap_64 (uint64_t x, int swap)
>> +{
>> + return swap ? bswap_64(x) : x;
>> +}
>>
>> /* gcc defines __BIG_ENDIAN__ on big endian targets */
>> #ifdef __BIG_ENDIAN__
>> diff --git a/include/helpers.h b/include/helpers.h
>> index 47b2674..43122c9 100644
>> --- a/include/helpers.h
>> +++ b/include/helpers.h
>> @@ -51,5 +51,7 @@ extern unsigned long SLOF_get_vtpm_unit(void);
>> const typeof(((type *)0)->member)* struct_ptr = (ptr); \
>> (type *)((char *)struct_ptr - offset_of(type, member)); })
>> #define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0]))
>> +#define ROUNDUP(x,v) ((((x) + (v - 1)) / v) * v)
>
> Put "v" in braces.
Done.
>
>
>> +#define MAX(x,y) ((x) > (y) ? (x) : (y))
>>
>> #endif
>> diff --git a/include/libelf.h b/include/libelf.h
>> index 5fbf279..76bfc27 100644
>> --- a/include/libelf.h
>> +++ b/include/libelf.h
>> @@ -96,4 +96,8 @@ void elf_relocate64(void *file_addr, signed long offset);
>>
>> int elf_forth_claim(void *addr, long size);
>>
>> +int elf_get_file_size(const void *buffer, const long buffer_size);
>> +int elf_get_file_size32(const void *buffer, const long buffer_size);
>> +int elf_get_file_size64(const void *buffer, const long buffer_size);
>> +
> Instead of defining those cond_bswap_XX(), I'd suggest:
>
> #ifdef __BIG_ENDIAN__
> #define elf64_to_cpu(x, ehdr) ((ehdr)->ei_data != ELFDATA2MSB ? (x) :
> bswap_64(x))
> #define elf32_to_cpu(x, ehdr) ((ehdr)->ei_data != ELFDATA2MSB ? (x) :
> bswap_32(x))
> #define elf16_to_cpu(x, ehdr) ((ehdr)->ei_data != ELFDATA2MSB ? (x) :
> bswap_16(x))
> #else
> #define elf64_to_cpu(x, ehdr) ((ehdr)->ei_data != ELFDATA2LSB ?
> bswap_64(x) : (x))
> #define elf32_to_cpu(x, ehdr) ((ehdr)->ei_data != ELFDATA2LSB ?
> bswap_32(x) : (x))
> #define elf16_to_cpu(x, ehdr) ((ehdr)->ei_data != ELFDATA2LSB ?
> bswap_16(x) : (x))
> #endif
The following works correct for me:
#ifdef __BIG_ENDIAN__
#define elf64_to_cpu(x, ehdr) ((ehdr)->ei_data == ELFDATA2MSB ? (x) :
bswap_64(x))
#define elf32_to_cpu(x, ehdr) ((ehdr)->ei_data == ELFDATA2MSB ? (x) :
bswap_32(x))
#define elf16_to_cpu(x, ehdr) ((ehdr)->ei_data == ELFDATA2MSB ? (x) :
bswap_16(x))
#else
#define elf64_to_cpu(x, ehdr) ((ehdr)->ei_data == ELFDATA2LSB ? (x) :
bswap_64(x))
#define elf32_to_cpu(x, ehdr) ((ehdr)->ei_data == ELFDATA2LSB ? (x) :
bswap_32(x))
#define elf16_to_cpu(x, ehdr) ((ehdr)->ei_data == ELFDATA2LSB ? (x) :
bswap_16(x))
#endif
>
> This is more or less how it is done everywhere else for virtio, except
> for virtio it is inline functions but here we can employ macros to
> support both elf32 and elf64.
>
>
>> #endif /* __LIBELF_H */
>> diff --git a/lib/libelf/elf.c b/lib/libelf/elf.c
>> index 5204bc3..91dd3f7 100644
>> --- a/lib/libelf/elf.c
>> +++ b/lib/libelf/elf.c
>> @@ -196,3 +196,31 @@ elf_get_base_addr(void *file_addr)
>>
>> return -1;
>> }
>> +
>> +/**
>> + * Get the file size of the ELF image that has been loaded into a
>> + * buffer larger than the size of the file
>> + * @return The size of the ELF image or < 0 for error
>> + */
>> +int elf_get_file_size(const void *buffer, const long buffer_size)
>> +{
>> + const struct ehdr *ehdr = (const struct ehdr *)buffer;
>> +
>> + if (buffer_size < sizeof(struct ehdr))
>> + return -1;
>> +
>> + /* check if it is an ELF image at all */
>> + if (cpu_to_be32(ehdr->ei_ident) != 0x7f454c46)
>> + return -1;
>> +
>> + switch (ehdr->ei_class) {
>> + case 1:
>> + /* Determine file size of a 32-bit file */
>> + return elf_get_file_size32(buffer, buffer_size);
>> + case 2:
>> + /* Determine file size of a 64-bit file */
>> + return elf_get_file_size64(buffer, buffer_size);
> These 2 comments above are quite useless.
Removed.
>> --- a/lib/libelf/elf64.c
>> +++ b/lib/libelf/elf64.c
>> @@ -20,6 +20,7 @@
>> #include <stdio.h>
>> #include <libelf.h>
>> #include <byteorder.h>
>> +#include <helpers.h>
>>
>> struct ehdr64
>> {
>> @@ -472,3 +473,65 @@ uint32_t elf_get_eflags_64(void *file_addr)
>>
>> return ehdr->e_flags;
>> }
>> +
>> +/*
>> + * Determine the size of an ELF image that has been loaded into
>> + * a buffer larger than its size. We search all program headers
>> + * and sections for the one that shows the farthest extent of the
>> + * file.
>> + * @return Return -1 on error, size of file otherwise.
>> + */
>> +int elf_get_file_size64(const void *buffer, const long buffer_size)
>> +{
>> + const struct ehdr64 *ehdr = (const struct ehdr64 *) buffer;
>> + const void *buffer_end = buffer + buffer_size;
>> + const struct phdr64 *phdr;
>> + const struct shdr64 *shdr;
>> + long elf_size = -1;
>> + uint16_t entsize;
>> + int do_swap;
>> + unsigned i;
>> +
>> + if (buffer_size < sizeof(struct ehdr) || ehdr->e_ehsize != 64)
>> + return -1;
>> +
>> +#ifdef __BIG_ENDIAN__
>> + do_swap = ehdr->ei_data != ELFDATA2MSB;
>> +#else
>> + do_swap = ehdr->ei_data != ELFDATA2LSB;
>> +#endif
>> +
>> + phdr = buffer + cond_bswap_64(ehdr->e_phoff, do_swap);
>> + entsize = cond_bswap_16(ehdr->e_phentsize, do_swap);
>> + for (i = 0; i < cond_bswap_16(ehdr->e_phnum, do_swap); i++) {
>> + if ((void *)phdr + entsize > buffer_end)
>> + return -1;
>> +
>> + elf_size = MAX(cond_bswap_64(phdr->p_offset, do_swap) +
>> + cond_bswap_64(phdr->p_filesz, do_swap),
>> + elf_size);
>> +
>> + /* step to next header */
>> + phdr = (struct phdr64 *)(((uint8_t *)phdr) + entsize);
>> + }
>> +
>> + shdr = buffer + cond_bswap_64(ehdr->e_shoff, do_swap);
>> + entsize = cond_bswap_16(ehdr->e_shentsize, do_swap);
>> + for (i = 0; i < cond_bswap_16(ehdr->e_shnum, do_swap); i++) {
>> + if ((void *)shdr + entsize > buffer_end)
>> + return -1;
>> +
>> + elf_size = MAX(cond_bswap_64(shdr->sh_offset, do_swap) +
>> + cond_bswap_64(shdr->sh_size, do_swap),
>> + elf_size);
>> +
>> + /* step to next header */
>> + shdr = (struct shdr64 *)(((uint8_t *)shdr) + entsize);
>
> Be consistent - "(void *)phdr + entsize" vs. "((uint8_t *)shdr) + entsize".
>
>
> I suggest "(uint8_t *)shdr + entsize".
Fixed.
>
>
>> + }
>> +
>> + elf_size = ROUNDUP(elf_size, 4);
>> + if (elf_size > buffer_size)
>> + return -1;
>> +
>> + return elf_size;
> elf_size is long, the function returns int, make it the same. Thanks,
Fixed.
Thanks!
Stefan
>> +}
>>
More information about the SLOF
mailing list