[PATCH v2 1/5] elf: Validate number of elf section header table entries
In bsdXX.c and multiboot_elfxx.c, e_shnum is used to obtain the number of
section header table entries, but it wasn't being checked if the value was
there.
According to the elf(5) manual page,
"If the number of entries in the section header table is larger than or equal to
SHN_LORESERVE (0xff00), e_shnum holds the value zero and the real number of
entries in the section header table is held in the sh_size member of the intial
entry in section header table. Otherwise, the sh_size member of the initial
entry in the section header table holds the value zero."
Since this check wasn't being made, grub_elfXX_get_shnum() is being added to
elfXX.c to make this check and use whichever member doesn't have a value of
zero. If both are zero, then we must return an error. We also need to make sure
that e_shnum doesn't have a value greater than or equal to SHN_LORESERVE and
sh_size isn't less than SHN_LORESERVE.
In order to get this function to work, the type ElfXX_Shnum is being added where
Elf32_Shnum defines Elf32_Word and Elf64_Shnum defines Elf64_Xword. This new
type is needed because if shnum obtains a value from sh_size, sh_size could be
of type El32_Word for Elf32_Shdr structures or Elf64_Xword for Elf64_Shdr
structures.
Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC macro to define the functions
in elf.h.
For a few smaller changes, changed casts of shnum to match variables being set
as well as dropped casts when unnecessary and fixed spacing errors in bsdXX.c.
Also, shnum is an unsigned integer and is compared to int i in
multiboot_elfxx.c, it should be unsigned to match shnum.
Signed-off-by: Alec Brown
---
grub-core/kern/elf.c | 12 +
grub-core/kern/elfXX.c | 34 +
grub-core/loader/i386/bsdXX.c | 82 ++
grub-core/loader/multiboot_elfxx.c | 49 +++---
include/grub/elf.h | 14 +
5 files changed, 150 insertions(+), 41 deletions(-)
diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 9d7149b38..66b69293e 100644
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -167,11 +167,15 @@ grub_elf_open (const char *name, enum grub_file_type type)
#define grub_elfXX_load_phdrs grub_elf32_load_phdrs
#define ElfXX_Phdr Elf32_Phdr
#define ElfXX_Ehdr Elf32_Ehdr
+#define ElfXX_Shdr Elf32_Shdr
+#define ElfXX_Word Elf32_Word
+#define ElfXX_Shnum Elf32_Shnum
#define grub_uintXX_t grub_uint32_t
#define grub_swap_bytes_addrXX grub_swap_bytes32
#define grub_swap_bytes_offXX grub_swap_bytes32
#define grub_swap_bytes_XwordXX grub_swap_bytes32
#define grub_elfXX_check_endianess_and_bswap_ehdr
grub_elf32_check_endianess_and_bswap_ehdr
+#define grub_elfXX_get_shnum grub_elf32_get_shnum
#include "elfXX.c"
@@ -185,11 +189,15 @@ grub_elf_open (const char *name, enum grub_file_type type)
#undef grub_elfXX_load_phdrs
#undef ElfXX_Phdr
#undef ElfXX_Ehdr
+#undef ElfXX_Shdr
+#undef ElfXX_Word
+#undef ElfXX_Shnum
#undef grub_uintXX_t
#undef grub_swap_bytes_addrXX
#undef grub_swap_bytes_offXX
#undef grub_swap_bytes_XwordXX
#undef grub_elfXX_check_endianess_and_bswap_ehdr
+#undef grub_elfXX_get_shnum
/* 64-bit */
@@ -203,10 +211,14 @@ grub_elf_open (const char *name, enum grub_file_type type)
#define grub_elfXX_load_phdrs grub_elf64_load_phdrs
#define ElfXX_Phdr Elf64_Phdr
#define ElfXX_Ehdr Elf64_Ehdr
+#define ElfXX_Shdr Elf64_Shdr
+#define ElfXX_Word Elf64_Word
+#define ElfXX_Shnum Elf64_Shnum
#define grub_uintXX_t grub_uint64_t
#define grub_swap_bytes_addrXX grub_swap_bytes64
#define grub_swap_bytes_offXX grub_swap_bytes64
#define grub_swap_bytes_XwordXX grub_swap_bytes64
#define grub_elfXX_check_endianess_and_bswap_ehdr
grub_elf64_check_endianess_and_bswap_ehdr
+#define grub_elfXX_get_shnum grub_elf64_get_shnum
#include "elfXX.c"
diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index 1859d1880..48b7745a5 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -205,3 +205,37 @@ grub_elfXX_check_endianess_and_bswap_ehdr (grub_elf_t elf)
return 0;
}
+
+grub_err_t
+grub_elfXX_get_shnum (ElfXX_Ehdr *e, ElfXX_Shnum *shnum)
+{
+ ElfXX_Shdr *s;
+
+ if (shnum == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for
shnum"));
+
+ /* Set *shnum to 0 so that shnum doesn't return junk on error */
+ *shnum = 0;
+
+ if (e == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf
header"));
+
+ *shnum = e->e_shnum;
+ if (*shnum == SHN_UNDEF)
+{
+ if (e->e_shoff == 0)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header
table offset in e_shoff"));
+
+ s = (ElfXX_Shdr *) ((grub_uint8_t *) e + e->e_shoff);
+ *shnum = s->sh_size;
+ if (*shnum < SHN_LORESERVE)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of section
header table entries in sh_size: %" PR
[PATCH v2 3/5] elf: Validate number of elf program header table entries
In bsdXX.c and multiboot_elfxx.c, e_phnum is used to obtain the number of
program header table entries, but it wasn't being checked if the value was
there.
According to the elf(5) manual page,
"If the number of entries in the program header table is larger than or equal to
PN_XNUM (0x), this member holds PN_XNUM (0x) and the real number of
entries in the program header table is held in the sh_info member of the
initial entry in section header table. Otherwise, the sh_info member of the
initial entry contains the value zero."
Since this check wasn't being made, grub_elfXX_get_phnum() is being added to
elfXX.c to make this check and use e_phnum if it doesn't have PN_XNUM as a
value, else use sh_info. We also need to make sure e_phnum isn't greater than
PN_XNUM and sh_info isn't less than PN_XNUM.
Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC macro to define the functions
in elf.h.
Also, changed casts of phnum to match variables being set as well as dropped
casts when unnecessary.
Signed-off-by: Alec Brown
---
grub-core/kern/elf.c | 3 +++
grub-core/kern/elfXX.c | 34 ++
grub-core/loader/i386/bsdXX.c | 11 +++---
grub-core/loader/multiboot_elfxx.c | 19 +++--
include/grub/elf.h | 5 +
5 files changed, 63 insertions(+), 9 deletions(-)
diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index c676db694..077a8500c 100644
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -177,6 +177,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
#define grub_elfXX_check_endianess_and_bswap_ehdr
grub_elf32_check_endianess_and_bswap_ehdr
#define grub_elfXX_get_shnum grub_elf32_get_shnum
#define grub_elfXX_get_shstrndx grub_elf32_get_shstrndx
+#define grub_elfXX_get_phnum grub_elf32_get_phnum
#include "elfXX.c"
@@ -200,6 +201,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
#undef grub_elfXX_check_endianess_and_bswap_ehdr
#undef grub_elfXX_get_shnum
#undef grub_elfXX_get_shstrndx
+#undef grub_elfXX_get_phnum
/* 64-bit */
@@ -223,5 +225,6 @@ grub_elf_open (const char *name, enum grub_file_type type)
#define grub_elfXX_check_endianess_and_bswap_ehdr
grub_elf64_check_endianess_and_bswap_ehdr
#define grub_elfXX_get_shnum grub_elf64_get_shnum
#define grub_elfXX_get_shstrndx grub_elf64_get_shstrndx
+#define grub_elfXX_get_phnum grub_elf64_get_phnum
#include "elfXX.c"
diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index 4e3254fa5..aabf4b9d7 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -272,3 +272,37 @@ grub_elfXX_get_shstrndx (ElfXX_Ehdr *e, ElfXX_Word
*shstrndx)
}
return GRUB_ERR_NONE;
}
+
+grub_err_t
+grub_elfXX_get_phnum (ElfXX_Ehdr *e, ElfXX_Word *phnum)
+{
+ ElfXX_Shdr *s;
+
+ if (phnum == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for
phnum"));
+
+ /* Set *phnum to 0 so that phnum doesn't return junk on error */
+ *phnum = 0;
+
+ if (e == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf
header"));
+
+ *phnum = e->e_phnum;
+ if (*phnum == PN_XNUM)
+{
+ if (e->e_shoff == 0)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header
table offset in e_shoff"));
+
+ s = (ElfXX_Shdr *) ((grub_uint8_t *) e + e->e_shoff);
+ *phnum = s->sh_info;
+ if (*phnum < PN_XNUM)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of program
header table entries in sh_info: %d"), *phnum);
+}
+ else
+{
+ if (*phnum >= PN_XNUM)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of program
header table entries in e_phnum: %d"), *phnum);
+}
+
+ return GRUB_ERR_NONE;
+}
diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index 2d91fd44d..dffa79d56 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -183,6 +183,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator
*relocator,
Elf_Ehdr e;
Elf_Shdr *s, *shdr = NULL;
Elf_Shnum shnum;
+ Elf_Word phnum;
grub_addr_t curload, module;
grub_err_t err;
grub_size_t chunk_size = 0;
@@ -198,6 +199,10 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct
grub_relocator *relocator,
if (err != GRUB_ERR_NONE)
goto out;
+ err = grub_elf_get_phnum (&e, &phnum);
+ if (err != GRUB_ERR_NONE)
+goto out;
+
for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum *
e.e_shentsize);
s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
@@ -212,7 +217,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator
*relocator,
if (chunk_size < sizeof (e))
chunk_size = sizeof (e);
- chunk_size += (grub_uint32_t) e.e_phnum * e.e_phentsize;
+ chunk_size += (grub_size_t) phnum * e.e_phentsize;
chunk_size += (grub_size_t) shnum *
[PATCH v2 4/5] util/grub-module-verifierXX.c: Changed get_shnum() return type
In util/grub-module-verifierXX.c, the function get_shnum() returns the variable
shnum, which is of the type Elf_Word. In the function, shnum can be obtained by
the e_shnum member of an Elf_Ehdr or the sh_size member of an Elf_Shdr. The
sh_size member can either be grub_uint32_t or grub_uint64_t, depending on the
architecture, but Elf_Word is only grub_uint32_t. To account for when sh_size is
grub_uint64_t, we can set shnum to have type Elf_Shnum and have get_shnum()
return an Elf_Shnum.
Signed-off-by: Alec Brown
---
util/grub-module-verifierXX.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index cf3ff0dfa..8e0cd91d9 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -18,6 +18,7 @@
# define Elf_RelElf32_Rel
# define Elf_Word Elf32_Word
# define Elf_Half Elf32_Half
+# define Elf_Shnum Elf32_Shnum
# define Elf_SectionElf32_Section
# define ELF_R_SYM(val)ELF32_R_SYM(val)
# define ELF_R_TYPE(val) ELF32_R_TYPE(val)
@@ -36,6 +37,7 @@
# define Elf_RelElf64_Rel
# define Elf_Word Elf64_Word
# define Elf_Half Elf64_Half
+# define Elf_Shnum Elf64_Shnum
# define Elf_SectionElf64_Section
# define ELF_R_SYM(val)ELF64_R_SYM(val)
# define ELF_R_TYPE(val) ELF64_R_TYPE(val)
@@ -141,11 +143,11 @@ get_shdr (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e, Elf_Word in
index * grub_target_to_host16 (e->e_shentsize));
}
-static Elf_Word
+static Elf_Shnum
get_shnum (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e)
{
Elf_Shdr *s;
- Elf_Word shnum;
+ Elf_Shnum shnum;
shnum = grub_target_to_host16 (e->e_shnum);
if (shnum == SHN_UNDEF)
@@ -153,12 +155,12 @@ get_shnum (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e)
s = get_shdr (arch, e, 0);
shnum = grub_target_to_host (s->sh_size);
if (shnum < SHN_LORESERVE)
- grub_util_error ("Invalid number of section header table entries in
sh_size: %d", shnum);
+ grub_util_error ("Invalid number of section header table entries in
sh_size: %" PRIuGRUB_UINT64_T, (grub_uint64_t) shnum);
}
else
{
if (shnum >= SHN_LORESERVE)
- grub_util_error ("Invalid number of section header table entries in
e_shnum: %d", shnum);
+ grub_util_error ("Invalid number of section header table entries in
e_shnum: %" PRIuGRUB_UINT64_T, (grub_uint64_t) shnum);
}
return shnum;
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 0/5] Fix coverity bugs and add checks for elf values in grub-core
Updates from v1:
- A few patches didn't work on 32-bit platforms. Fixed this by renaming
function definitions in multiboot_elfxx.c. The patches that did work
were
merged with the master branch.
- Added a patch to make error conditionals consistent in comparing to
GRUB_ERR_NONE.
Coverity identified several untrusted loop bounds and untrusted allocation size
bugs in grub-core/loader/i386/bsdXX.c and grub-core/loader/multiboot_elfXX.c.
Upon review of these bugs, I found that specific checks weren't being made to
various elf header values based on the elf manual page. The first version of
the patch series contained the patches fixing the Coverity bugs, but those have
been merged with the master branch. The remaining patches add functions to
check for the correct elf header values, update previous work done in
util/grub-module-verifierXX.c that checks elf header values, and update error
conditionals of the affected files.
Also, I was able to verify that these patches work by using Multiboot and
Multiboot2 to boot a Xen image.
Alec Brown (5):
elf: Validate number of elf section header table entries
elf: Validate elf section header table index for section name string table
elf: Validate number of elf program header table entries
util/grub-module-verifierXX.c: Changed get_shnum() return type
loader: Update error conditionals to use enums
grub-core/kern/elf.c | 18 ++
grub-core/kern/elfXX.c | 101
+
grub-core/loader/i386/bsdXX.c | 131
+++
grub-core/loader/multiboot_elfxx.c | 85
+
include/grub/elf.h | 23 +++
util/grub-module-verifierXX.c | 10 ++
6 files changed, 292 insertions(+), 76 deletions(-)
Interdiff against v1:
diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index 2cc1daa49..09d909f1b 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -52,7 +52,7 @@ read_headers (grub_file_t file, const char *filename,
Elf_Ehdr *e, Elf_Shdr **sh
return grub_error (GRUB_ERR_BAD_OS, N_("invalid arch-dependent ELF
magic"));
err = grub_elf_get_shnum (e, &shnum);
- if (err)
+ if (err != GRUB_ERR_NONE)
return err;
*shdr = grub_calloc (shnum, e->e_shentsize);
@@ -94,7 +94,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct
grub_relocator *relocator,
curload = module = ALIGN_PAGE (*kern_end);
err = read_headers (file, argv[0], &e, &shdr);
- if (err)
+ if (err != GRUB_ERR_NONE)
goto out;
err = grub_elf_get_shnum (&e, &shnum);
@@ -118,7 +118,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct
grub_relocator *relocator,
grub_relocator_chunk_t ch;
err = grub_relocator_alloc_chunk_addr (relocator, &ch,
module, chunk_size);
-if (err)
+if (err != GRUB_ERR_NONE)
goto out;
chunk_src = get_virtual_current_address (ch);
}
@@ -143,7 +143,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct
grub_relocator *relocator,
case SHT_PROGBITS:
err = load (file, argv[0], (grub_uint8_t *) chunk_src + curload -
*kern_end,
s->sh_offset, s->sh_size);
- if (err)
+ if (err != GRUB_ERR_NONE)
goto out;
break;
case SHT_NOBITS:
@@ -159,11 +159,11 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct
grub_relocator *relocator,
err = grub_freebsd_add_meta_module (argv[0], FREEBSD_MODTYPE_ELF_MODULE_OBJ,
argc - 1, argv + 1, module,
curload - module);
- if (! err)
+ if (err == GRUB_ERR_NONE)
err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA
| FREEBSD_MODINFOMD_ELFHDR,
&e, sizeof (e));
- if (! err)
+ if (err == GRUB_ERR_NONE)
err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA
| FREEBSD_MODINFOMD_SHDR,
shdr, shnum * e.e_shentsize);
@@ -192,7 +192,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator
*relocator,
curload = module = ALIGN_PAGE (*kern_end);
err = read_headers (file, argv[0], &e, &shdr);
- if (err)
+ if (err != GRUB_ERR_NONE)
goto out;
err = grub_elf_get_shnum (&e, &shnum);
@@ -225,7 +225,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator
*relocator,
err = grub_relocator_alloc_chunk_addr (relocator, &ch,
module, chunk_size);
-if (err)
+if (err != GRUB_ERR_NONE)
goto out;
chunk_src = get_virtua
[PATCH v2 2/5] elf: Validate elf section header table index for section name string table
In multiboot_elfxx.c, e_shstrndx is used to obtain the section header table
index of the section name string table, but it wasn't being checked if the value
was there.
According to the elf(5) manual page,
"If the index of section name string table section is larger than or equal to
SHN_LORESERVE (0xff00), this member holds SHN_XINDEX (0x) and the real
index of the section name string table section is held in the sh_link member of
the initial entry in section header table. Otherwise, the sh_link member of the
initial entry in section header table contains the value zero."
Since this check wasn't being made, grub_elfXX_get_shstrndx() is being added to
elfXX.c to make this check and use e_shstrndx if it doesn't have SHN_XINDEX as a
value, else use sh_link. We also need to make sure e_shstrndx isn't greater than
or equal to SHN_LORESERVE and sh_link isn't less than SHN_LORESERVE.
Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC macro to define the functions
in elf.h.
Signed-off-by: Alec Brown
---
grub-core/kern/elf.c | 3 +++
grub-core/kern/elfXX.c | 33 ++
grub-core/loader/multiboot_elfxx.c | 13 +++-
include/grub/elf.h | 4
4 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 66b69293e..c676db694 100644
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -176,6 +176,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
#define grub_swap_bytes_XwordXX grub_swap_bytes32
#define grub_elfXX_check_endianess_and_bswap_ehdr
grub_elf32_check_endianess_and_bswap_ehdr
#define grub_elfXX_get_shnum grub_elf32_get_shnum
+#define grub_elfXX_get_shstrndx grub_elf32_get_shstrndx
#include "elfXX.c"
@@ -198,6 +199,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
#undef grub_swap_bytes_XwordXX
#undef grub_elfXX_check_endianess_and_bswap_ehdr
#undef grub_elfXX_get_shnum
+#undef grub_elfXX_get_shstrndx
/* 64-bit */
@@ -220,5 +222,6 @@ grub_elf_open (const char *name, enum grub_file_type type)
#define grub_swap_bytes_XwordXX grub_swap_bytes64
#define grub_elfXX_check_endianess_and_bswap_ehdr
grub_elf64_check_endianess_and_bswap_ehdr
#define grub_elfXX_get_shnum grub_elf64_get_shnum
+#define grub_elfXX_get_shstrndx grub_elf64_get_shstrndx
#include "elfXX.c"
diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index 48b7745a5..4e3254fa5 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -239,3 +239,36 @@ grub_elfXX_get_shnum (ElfXX_Ehdr *e, ElfXX_Shnum *shnum)
return GRUB_ERR_NONE;
}
+
+grub_err_t
+grub_elfXX_get_shstrndx (ElfXX_Ehdr *e, ElfXX_Word *shstrndx)
+{
+ ElfXX_Shdr *s;
+
+ if (shstrndx == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for
shstrndx"));
+
+ /* Set *shstrndx to 0 so that shstrndx doesn't return junk on error */
+ *shstrndx = 0;
+
+ if (e == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf
header"));
+
+ *shstrndx = e->e_shstrndx;
+ if (*shstrndx == SHN_XINDEX)
+{
+ if (e->e_shoff == 0)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header
table offset in e_shoff"));
+
+ s = (ElfXX_Shdr *) ((grub_uint8_t *) e + e->e_shoff);
+ *shstrndx = s->sh_link;
+ if (*shstrndx < SHN_LORESERVE)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header
table index in sh_link: %d"), *shstrndx);
+}
+ else
+{
+ if (*shstrndx >= SHN_LORESERVE)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header
table index in e_shstrndx: %d"), *shstrndx);
+}
+ return GRUB_ERR_NONE;
+}
diff --git a/grub-core/loader/multiboot_elfxx.c
b/grub-core/loader/multiboot_elfxx.c
index ed5a4ae9d..ec4ba3b8d 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -23,8 +23,10 @@
# define Elf_Ehdr Elf32_Ehdr
# define Elf_Phdr Elf32_Phdr
# define Elf_Shdr Elf32_Shdr
+# define Elf_Word Elf32_Word
# define Elf_Shnum Elf32_Shnum
# define grub_multiboot_elf_get_shnum grub_elf32_get_shnum
+# define grub_multiboot_elf_get_shstrndx grub_elf32_get_shstrndx
#elif defined(MULTIBOOT_LOAD_ELF64)
# define XX64
# define E_MACHINE MULTIBOOT_ELF64_MACHINE
@@ -32,8 +34,10 @@
# define Elf_Ehdr Elf64_Ehdr
# define Elf_Phdr Elf64_Phdr
# define Elf_Shdr Elf64_Shdr
+# define Elf_Word Elf64_Word
# define Elf_Shnum Elf64_Shnum
# define grub_multiboot_elf_get_shnum grub_elf64_get_shnum
+# define grub_multiboot_elf_get_shstrndx grub_elf64_get_shstrndx
#else
#error "I'm confused"
#endif
@@ -63,6 +67,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_da
[PATCH v2 5/5] loader: Update error conditionals to use enums
In grub-core/loader/i386/bsdXX.c and grub-core/loader/multiboot_elfxx.c, error conditionals are simplified to statements such as "if (err)". Even though the assumption that non-zero values give errors is correct, it would be clearer and more consistent to compare these conditionals to GRUB_ERR_NONE. Signed-off-by: Alec Brown --- grub-core/loader/i386/bsdXX.c | 38 +++--- grub-core/loader/multiboot_elfxx.c | 6 ++--- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c index dffa79d56..09d909f1b 100644 --- a/grub-core/loader/i386/bsdXX.c +++ b/grub-core/loader/i386/bsdXX.c @@ -94,7 +94,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator, curload = module = ALIGN_PAGE (*kern_end); err = read_headers (file, argv[0], &e, &shdr); - if (err) + if (err != GRUB_ERR_NONE) goto out; err = grub_elf_get_shnum (&e, &shnum); @@ -118,7 +118,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator, grub_relocator_chunk_t ch; err = grub_relocator_alloc_chunk_addr (relocator, &ch, module, chunk_size); -if (err) +if (err != GRUB_ERR_NONE) goto out; chunk_src = get_virtual_current_address (ch); } @@ -143,7 +143,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator, case SHT_PROGBITS: err = load (file, argv[0], (grub_uint8_t *) chunk_src + curload - *kern_end, s->sh_offset, s->sh_size); - if (err) + if (err != GRUB_ERR_NONE) goto out; break; case SHT_NOBITS: @@ -159,11 +159,11 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator, err = grub_freebsd_add_meta_module (argv[0], FREEBSD_MODTYPE_ELF_MODULE_OBJ, argc - 1, argv + 1, module, curload - module); - if (! err) + if (err == GRUB_ERR_NONE) err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA | FREEBSD_MODINFOMD_ELFHDR, &e, sizeof (e)); - if (! err) + if (err == GRUB_ERR_NONE) err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA | FREEBSD_MODINFOMD_SHDR, shdr, shnum * e.e_shentsize); @@ -192,7 +192,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator, curload = module = ALIGN_PAGE (*kern_end); err = read_headers (file, argv[0], &e, &shdr); - if (err) + if (err != GRUB_ERR_NONE) goto out; err = grub_elf_get_shnum (&e, &shnum); @@ -225,7 +225,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator, err = grub_relocator_alloc_chunk_addr (relocator, &ch, module, chunk_size); -if (err) +if (err != GRUB_ERR_NONE) goto out; chunk_src = get_virtual_current_address (ch); @@ -252,7 +252,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator, (grub_uint8_t *) chunk_src + module + s->sh_addr - *kern_end, s->sh_offset, s->sh_size); - if (err) + if (err != GRUB_ERR_NONE) goto out; break; case SHT_NOBITS: @@ -285,7 +285,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator, curload - module); out: grub_free (shdr); - if (err) + if (err != GRUB_ERR_NONE) return err; return SUFFIX (grub_freebsd_load_elf_meta) (relocator, file, argv[0], kern_end); } @@ -313,7 +313,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator *relocator, grub_size_t chunk_size; err = read_headers (file, filename, &e, &shdr); - if (err) + if (err != GRUB_ERR_NONE) goto out; err = grub_elf_get_shnum (&e, &shnum); @@ -323,7 +323,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator *relocator, err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA | FREEBSD_MODINFOMD_ELFHDR, &e, sizeof (e)); - if (err) + if (err != GRUB_ERR_NONE) goto out; for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize); @@ -351,7 +351,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator *relocator, grub_relocator_chunk_t ch; err = grub_relocator_alloc_chunk_addr (relocator, &ch, symtarget, chunk_size); -if (err) +if (err != GRUB_ERR_NONE) goto out; sym_chunk = get_virtual_current_address (ch); } @@ -414,14 +414,14 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator *relocator, err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA | FREEBSD_MODINFOMD_DYNAMIC, &dynam
