Re: [PATCHv2] hurd: Use part: qualifier
On Thu, May 26, 2022 at 12:58:21AM +0200, Samuel Thibault wrote: > When using userland drivers such as rumpdisk, we'd rather make ext2fs use > parted-based libstore partitioning support. That can be used for kernelland > drivers as well, so we can just make grub always use the part: qualifier to > switch ext2fs to it. > > grub_util_find_hurd_root_device then has to understand this syntax and > translate > it into the /dev/ entry name. > > Signed-off-by: Samuel Thibault > > --- > Difference with v2: rebase on master Thank you for rebase! Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3 3/5] cryptodisk: Add options to cryptomount to support keyfiles
On Fri, May 20, 2022 at 02:32:17PM -0500, Glenn Washburn wrote:
> From: John Lane
>
> Add the options --key-file, --keyfile-offset, and --keyfile-size to
> cryptomount and code to put read the requested key file data and pass
> via the cargs struct. Note, key file data is for all intents and purposes
> equivalent to a password given to cryptomount. So there is no need to
> enable support for key files in the various crypto backends (eg. LUKS1)
> because the key data is passed just as if it were a password.
>
> Signed-off-by: John Lane
> gnu...@cyberdimension.org: rebase, patch split, small fixes, commit message
> Signed-off-by: Denis 'GNUtoo' Carikli
> developm...@efficientek.com: rebase and rework to use cryptomount arg passing,
> minor fixes, improve commit message
> Signed-off-by: Glenn Washburn
> ---
> grub-core/disk/cryptodisk.c | 81 -
> include/grub/cryptodisk.h | 2 +
> include/grub/file.h | 2 +
> 3 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 9f5dc7acb..1198cb0e6 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -42,6 +42,9 @@ static const struct grub_arg_option options[] =
> {"all", 'a', 0, N_("Mount all."), 0, 0},
> {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0},
> {"password", 'p', 0, N_("Password to open volumes."), 0,
> ARG_TYPE_STRING},
> +{"key-file", 'k', 0, N_("Key file"), 0, ARG_TYPE_STRING},
> +{"keyfile-offset", 'O', 0, N_("Key file offset (bytes)"), 0,
> ARG_TYPE_INT},
> +{"keyfile-size", 'S', 0, N_("Key file data size (bytes)"), 0,
> ARG_TYPE_INT},
> {0, 0, 0, 0, 0, 0}
>};
>
> @@ -1172,6 +1175,80 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int
> argc, char **args)
>cargs.key_len = grub_strlen (state[3].arg);
> }
>
> + if (state[4].set) /* keyfile */
> +{
> + const char *p = NULL;
> + grub_file_t keyfile;
> + unsigned long long keyfile_offset = 0, keyfile_size = 0;
> +
> + if (state[5].set) /* keyfile-offset */
> + {
> + grub_errno = GRUB_ERR_NONE;
> + keyfile_offset = grub_strtoull (state[5].arg, &p, 0);
> +
> + if (state[5].arg[0] == '\0' || *p != '\0')
> + return grub_error (grub_errno,
> +N_("non-numeric or invalid keyfile offset `%s'"),
> +state[5].arg);
> + }
> +
> + if (state[6].set) /* keyfile-size */
> + {
> + grub_errno = GRUB_ERR_NONE;
> + keyfile_size = grub_strtoull (state[6].arg, &p, 0);
> +
> + if (state[6].arg[0] == '\0' || *p != '\0')
> + return grub_error (grub_errno,
> +N_("non-numeric or invalid keyfile size `%s'"),
> +state[6].arg);
> +
> + if (keyfile_size == 0)
> + return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("key file size is 0"));
> +
> + if (keyfile_size > GRUB_CRYPTODISK_MAX_KEYFILE_SIZE)
> + return grub_error (GRUB_ERR_OUT_OF_RANGE,
> +N_("key file size exceeds maximum (%d)"),
> +GRUB_CRYPTODISK_MAX_KEYFILE_SIZE);
> + }
> +
> + keyfile = grub_file_open (state[4].arg,
> + GRUB_FILE_TYPE_CRYPTODISK_ENCRYPTION_KEY);
> + if (keyfile == NULL)
> + return grub_errno;
> +
> + if (keyfile_offset > keyfile->size)
> + return grub_error (GRUB_ERR_OUT_OF_RANGE,
> +N_("Keyfile offset, %llu, is greater than"
> + "keyfile size, %" PRIuGRUB_UINT64_T),
> +keyfile_offset, keyfile->size);
> +
> + if (grub_file_seek (keyfile, (grub_off_t) keyfile_offset) ==
> (grub_off_t) -1)
> + return grub_errno;
> +
> + if (keyfile_size != 0)
> + {
> + if (keyfile_size > (keyfile->size - keyfile_offset))
> + return grub_error (GRUB_ERR_FILE_READ_ERROR,
> +N_("keyfile is too small: requested %llu bytes,"
> + " but the file only has %" PRIuGRUB_UINT64_T
If you cast subtraction result to grub_size_t below then PRIuGRUB_UINT64_T
should be replaced with PRIuGRUB_SIZE. Or maybe it would be safer if we
do s/grub_size_t/grub_off_t/ instead of changing format string. This does
not matter today but...
Otherwise all patches LGTM -> Reviewed-by: Daniel Kiper
So, I can fix the issue mentioned above for you before push.
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC PATCH v3 1/5] grub-mkconfig linux: Fix quadratic algorithm for sorting menu items
On Fri, May 20, 2022 at 10:37:37AM -0400, Mathieu Desnoyers wrote:
> The current implementation of the 10_linux script implements its menu
> items sorting in bash with a quadratic algorithm, calling "sed", "sort",
> "head", and "grep" to compare versions between individual lines, which
> is annoyingly slow for kernel developers who can easily end up with
> 50-100 kernels in /boot.
>
> As an example, on a Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, running:
>
> /usr/sbin/grub-mkconfig > /dev/null
>
> With 44 kernels in /boot, this command takes 10-15 seconds to complete.
> After this fix, the same command runs in 5 seconds.
>
> With 116 kernels in /boot, this command takes 40 seconds to complete.
> After this fix, the same command runs in 8 seconds.
>
> For reference, the quadratic algorithm here is:
>
> while [ "x$list" != "x" ] ; do <--- outer loop
> linux=`version_find_latest $list`
> version_find_latest()
> for i in "$@" ; do<--- inner loop
> version_test_gt()
> fork+exec sed
> version_test_numeric()
> version_sort
> fork+exec sort
> fork+exec head -n 1
> fork+exec grep
> list=`echo $list | tr ' ' '\n' | fgrep -vx "$linux" | tr '\n' ' '`
> tr
> fgrep
> tr
>
> So all commands executed under version_test_gt() are executed
> O(n^2) times where n is the number of kernel images in /boot.
>
> Here is the improved algorithm proposed:
>
> - Prepare a list with all the relevant information for ordering by a single
> sort(1) execution. This is done by renaming ".old" suffixes by " 1" and
> by suffixing all other files with " 2", thus making sure the ".old" entries
> will follow the non-old entries in reverse-sorted-order.
> - Call version_reverse_sort on the list (sort -r -V): A single execution of
> sort(1). For instance, GNU coreutils' sort will reverse-sort the list in
> O(n*log(n)) with a merge sort.
> - Replace the " 1" suffixes by ".old", and remove the " 2" suffixes.
> - Iterate on the reverse-sorted list to output each menu entry item.
>
> Therefore, the algorithm proposed has O(n*log(n)) complexity with GNU
> coreutils' sort compared to the prior O(n^2) complexity. Moreover, the
> constant time required for each list entry is much less because sorting
> is done within a single execution of sort(1) rather than requiring
> O(n^2) executions of sed(1), sort(1), head(1), and grep(1) in
> sub-shells.
>
> Signed-off-by: Mathieu Desnoyers
> ---
> Changes since v1:
> - Escape the dot from .old in the sed match pattern, thus ensuring it
> matches ".old" rather than "[any character]old".
> - Use "sed" rather than "sed -e" everywhere for consistency.
> - Document the new algorithm in the commit message.
>
> Changes since v2:
> - Rename version_reverse_sort_sort_has_v to version_sort_sort_has_v,
> - Combine multiple sed executions into a single sed -e ... -e ...
>
> Changes since v3:
> - Modify version_sort to expect arguments, and call "version_sort -r",
> rather than copying it as a "version_reverse_sort".
> - Specify that O(n*log(n)) merge sort is specific to GNU coreutils' sort.
> ---
> util/grub-mkconfig_lib.in | 8
> util/grub.d/10_linux.in | 12
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 301d1ac22..fc14afdb3 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -204,16 +204,16 @@ version_sort ()
> {
>case $version_sort_sort_has_v in
> yes)
> - LC_ALL=C sort -V;;
> + LC_ALL=C sort -V $@;;
> no)
> - LC_ALL=C sort -n;;
> + LC_ALL=C sort -n $@;;
> *)
>if sort -V /dev/null 2>&1; then
> version_sort_sort_has_v=yes
> - LC_ALL=C sort -V
> + LC_ALL=C sort -V $@
>else
> version_sort_sort_has_v=no
> -LC_ALL=C sort -n
> +LC_ALL=C sort -n $@
>fi;;
> esac
> }
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index ca068038e..001a97ce3 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -195,9 +195,15 @@ title_correction_code=
> # yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
> submenu_indentation=""
>
> +# Perform a reverse version sort on the entire list.
> +# Temporarily replace the '.old' suffix by ' 1' and append ' 2' for all
> +# other files to order the '.old' files after their non-old counterpart
> +# in reverse-sorted order.
> +
> +reverse_sorted_list=$(echo $list | tr ' ' '\n' | sed -e 's/\.old$/ 1/' -e '/
> 1$/! s/$/ 2/' | version_sort -r | sed -e 's/ 1$/.old/' -e 's/ 2$//')
Nit, I think you can use one "-e" argument for sed, e.g. sed -e 's/\.old$/ 1/;
/ 1$/! s/$/ 2/'.
Otherwise patches LGTM.
Please hold on with rebase. I am going to push one more patch before
your patch series which may potentially conflict with your changes.
I will drop you a line whe
Re: [RFC PATCH v3 0/5] grub-mkconfig: Fix quadratic algorithm for sorting menu items
On Fri, May 20, 2022 at 12:08:05PM -0400, Mathieu Desnoyers wrote: > Sorry, the subject prefix for this patch series should have been [RFC PATCH > v4 n/5]. Next time you can drop RFC from the subject. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3] efidisk: pass buffers with higher alignment
Hey, On Thu, May 19, 2022 at 09:36:41AM +0200, Stefan Agner wrote: > On 2022-05-18 10:59, Stefan Agner wrote: > > Some devices report a IoAlign value of 2, however seem to require a > > buffer with higher alignment. > > After releasing Home Assistant OS 8.0 publicly, some systems still > refuse to boot even with this patch applied. See bug reports collected > in this issue: > https://github.com/home-assistant/operating-system/issues/1912 > > At least in one instance using block size alignment helped. I am going > to change to block size aligned unconditionally, and see how that > behaves in wider deployment. Any updates here? I would be more than happy to get this fixed. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC PATCH v3 1/5] grub-mkconfig linux: Fix quadratic algorithm for sorting menu items
- On May 26, 2022, at 11:13 AM, Daniel Kiper dki...@net-space.pl wrote:
> On Fri, May 20, 2022 at 10:37:37AM -0400, Mathieu Desnoyers wrote:
>> The current implementation of the 10_linux script implements its menu
>> items sorting in bash with a quadratic algorithm, calling "sed", "sort",
>> "head", and "grep" to compare versions between individual lines, which
>> is annoyingly slow for kernel developers who can easily end up with
>> 50-100 kernels in /boot.
>>
>> As an example, on a Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz, running:
>>
>> /usr/sbin/grub-mkconfig > /dev/null
>>
>> With 44 kernels in /boot, this command takes 10-15 seconds to complete.
>> After this fix, the same command runs in 5 seconds.
>>
>> With 116 kernels in /boot, this command takes 40 seconds to complete.
>> After this fix, the same command runs in 8 seconds.
>>
>> For reference, the quadratic algorithm here is:
>>
>> while [ "x$list" != "x" ] ; do <--- outer loop
>> linux=`version_find_latest $list`
>> version_find_latest()
>> for i in "$@" ; do<--- inner loop
>> version_test_gt()
>> fork+exec sed
>> version_test_numeric()
>> version_sort
>> fork+exec sort
>> fork+exec head -n 1
>> fork+exec grep
>> list=`echo $list | tr ' ' '\n' | fgrep -vx "$linux" | tr '\n' ' '`
>> tr
>> fgrep
>> tr
>>
>> So all commands executed under version_test_gt() are executed
>> O(n^2) times where n is the number of kernel images in /boot.
>>
>> Here is the improved algorithm proposed:
>>
>> - Prepare a list with all the relevant information for ordering by a single
>> sort(1) execution. This is done by renaming ".old" suffixes by " 1" and
>> by suffixing all other files with " 2", thus making sure the ".old" entries
>> will follow the non-old entries in reverse-sorted-order.
>> - Call version_reverse_sort on the list (sort -r -V): A single execution of
>> sort(1). For instance, GNU coreutils' sort will reverse-sort the list in
>> O(n*log(n)) with a merge sort.
>> - Replace the " 1" suffixes by ".old", and remove the " 2" suffixes.
>> - Iterate on the reverse-sorted list to output each menu entry item.
>>
>> Therefore, the algorithm proposed has O(n*log(n)) complexity with GNU
>> coreutils' sort compared to the prior O(n^2) complexity. Moreover, the
>> constant time required for each list entry is much less because sorting
>> is done within a single execution of sort(1) rather than requiring
>> O(n^2) executions of sed(1), sort(1), head(1), and grep(1) in
>> sub-shells.
>>
>> Signed-off-by: Mathieu Desnoyers
>> ---
>> Changes since v1:
>> - Escape the dot from .old in the sed match pattern, thus ensuring it
>> matches ".old" rather than "[any character]old".
>> - Use "sed" rather than "sed -e" everywhere for consistency.
>> - Document the new algorithm in the commit message.
>>
>> Changes since v2:
>> - Rename version_reverse_sort_sort_has_v to version_sort_sort_has_v,
>> - Combine multiple sed executions into a single sed -e ... -e ...
>>
>> Changes since v3:
>> - Modify version_sort to expect arguments, and call "version_sort -r",
>> rather than copying it as a "version_reverse_sort".
>> - Specify that O(n*log(n)) merge sort is specific to GNU coreutils' sort.
>> ---
>> util/grub-mkconfig_lib.in | 8
>> util/grub.d/10_linux.in | 12
>> 2 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
>> index 301d1ac22..fc14afdb3 100644
>> --- a/util/grub-mkconfig_lib.in
>> +++ b/util/grub-mkconfig_lib.in
>> @@ -204,16 +204,16 @@ version_sort ()
>> {
>>case $version_sort_sort_has_v in
>> yes)
>> - LC_ALL=C sort -V;;
>> + LC_ALL=C sort -V $@;;
>> no)
>> - LC_ALL=C sort -n;;
>> + LC_ALL=C sort -n $@;;
>> *)
>>if sort -V /dev/null 2>&1; then
>> version_sort_sort_has_v=yes
>> -LC_ALL=C sort -V
>> +LC_ALL=C sort -V $@
>>else
>> version_sort_sort_has_v=no
>> -LC_ALL=C sort -n
>> +LC_ALL=C sort -n $@
>>fi;;
>> esac
>> }
>> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
>> index ca068038e..001a97ce3 100644
>> --- a/util/grub.d/10_linux.in
>> +++ b/util/grub.d/10_linux.in
>> @@ -195,9 +195,15 @@ title_correction_code=
>> # yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
>> submenu_indentation=""
>>
>> +# Perform a reverse version sort on the entire list.
>> +# Temporarily replace the '.old' suffix by ' 1' and append ' 2' for all
>> +# other files to order the '.old' files after their non-old counterpart
>> +# in reverse-sorted order.
>> +
>> +reverse_sorted_list=$(echo $list | tr ' ' '\n' | sed -e 's/\.old$/ 1/' -e '/
>> 1$/! s/$/ 2/' | version_sort -r | sed -e 's/ 1$/.old/' -e 's/ 2$//')
>
> Nit, I think you can use one "-e" argument for sed, e.g. sed -e 's/\.old$/
> 1/; /
> 1$/
Re: [PATCH v3 3/5] cryptodisk: Add options to cryptomount to support keyfiles
On Thu, 26 May 2022 16:24:13 +0200
Daniel Kiper wrote:
> On Fri, May 20, 2022 at 02:32:17PM -0500, Glenn Washburn wrote:
> > From: John Lane
> >
> > Add the options --key-file, --keyfile-offset, and --keyfile-size to
> > cryptomount and code to put read the requested key file data and pass
> > via the cargs struct. Note, key file data is for all intents and purposes
> > equivalent to a password given to cryptomount. So there is no need to
> > enable support for key files in the various crypto backends (eg. LUKS1)
> > because the key data is passed just as if it were a password.
> >
> > Signed-off-by: John Lane
> > gnu...@cyberdimension.org: rebase, patch split, small fixes, commit message
> > Signed-off-by: Denis 'GNUtoo' Carikli
> > developm...@efficientek.com: rebase and rework to use cryptomount arg
> > passing,
> > minor fixes, improve commit message
> > Signed-off-by: Glenn Washburn
> > ---
> > grub-core/disk/cryptodisk.c | 81 -
> > include/grub/cryptodisk.h | 2 +
> > include/grub/file.h | 2 +
> > 3 files changed, 84 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> > index 9f5dc7acb..1198cb0e6 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -42,6 +42,9 @@ static const struct grub_arg_option options[] =
> > {"all", 'a', 0, N_("Mount all."), 0, 0},
> > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0},
> > {"password", 'p', 0, N_("Password to open volumes."), 0,
> > ARG_TYPE_STRING},
> > +{"key-file", 'k', 0, N_("Key file"), 0, ARG_TYPE_STRING},
> > +{"keyfile-offset", 'O', 0, N_("Key file offset (bytes)"), 0,
> > ARG_TYPE_INT},
> > +{"keyfile-size", 'S', 0, N_("Key file data size (bytes)"), 0,
> > ARG_TYPE_INT},
> > {0, 0, 0, 0, 0, 0}
> >};
> >
> > @@ -1172,6 +1175,80 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt,
> > int argc, char **args)
> >cargs.key_len = grub_strlen (state[3].arg);
> > }
> >
> > + if (state[4].set) /* keyfile */
> > +{
> > + const char *p = NULL;
> > + grub_file_t keyfile;
> > + unsigned long long keyfile_offset = 0, keyfile_size = 0;
> > +
> > + if (state[5].set) /* keyfile-offset */
> > + {
> > + grub_errno = GRUB_ERR_NONE;
> > + keyfile_offset = grub_strtoull (state[5].arg, &p, 0);
> > +
> > + if (state[5].arg[0] == '\0' || *p != '\0')
> > + return grub_error (grub_errno,
> > + N_("non-numeric or invalid keyfile offset `%s'"),
> > + state[5].arg);
> > + }
> > +
> > + if (state[6].set) /* keyfile-size */
> > + {
> > + grub_errno = GRUB_ERR_NONE;
> > + keyfile_size = grub_strtoull (state[6].arg, &p, 0);
> > +
> > + if (state[6].arg[0] == '\0' || *p != '\0')
> > + return grub_error (grub_errno,
> > + N_("non-numeric or invalid keyfile size `%s'"),
> > + state[6].arg);
> > +
> > + if (keyfile_size == 0)
> > + return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("key file size is 0"));
> > +
> > + if (keyfile_size > GRUB_CRYPTODISK_MAX_KEYFILE_SIZE)
> > + return grub_error (GRUB_ERR_OUT_OF_RANGE,
> > + N_("key file size exceeds maximum (%d)"),
> > + GRUB_CRYPTODISK_MAX_KEYFILE_SIZE);
> > + }
> > +
> > + keyfile = grub_file_open (state[4].arg,
> > + GRUB_FILE_TYPE_CRYPTODISK_ENCRYPTION_KEY);
> > + if (keyfile == NULL)
> > + return grub_errno;
> > +
> > + if (keyfile_offset > keyfile->size)
> > + return grub_error (GRUB_ERR_OUT_OF_RANGE,
> > + N_("Keyfile offset, %llu, is greater than"
> > + "keyfile size, %" PRIuGRUB_UINT64_T),
> > + keyfile_offset, keyfile->size);
> > +
> > + if (grub_file_seek (keyfile, (grub_off_t) keyfile_offset) ==
> > (grub_off_t) -1)
> > + return grub_errno;
> > +
> > + if (keyfile_size != 0)
> > + {
> > + if (keyfile_size > (keyfile->size - keyfile_offset))
> > + return grub_error (GRUB_ERR_FILE_READ_ERROR,
> > + N_("keyfile is too small: requested %llu bytes,"
> > + " but the file only has %" PRIuGRUB_UINT64_T
>
> If you cast subtraction result to grub_size_t below then PRIuGRUB_UINT64_T
> should be replaced with PRIuGRUB_SIZE. Or maybe it would be safer if we
> do s/grub_size_t/grub_off_t/ instead of changing format string. This does
> not matter today but...
>
> Otherwise all patches LGTM -> Reviewed-by: Daniel Kiper
>
> So, I can fix the issue mentioned above for you before push.
Sounds great. Glad to finally get this series off my plate.
Glenn
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core
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 four patches in this patch series address the coverity bugs, as well as adds functions to check for the correct elf header values. The last two patches adds fixes to previous work done in util/grub-module-verifierXX.c that also relates to making checks of elf header values. The Coverity bugs being addressed are: CID 314018 CID 314030 CID 314031 CID 314039 Alec Brown (6): grub-core/loader/i386/bsdXX.c: Avoid downcasting (char *) to (Elf_Shdr *) 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: Add e_shoff check in get_shdr() util/grub-module-verifierXX.c: Changed get_shnum() return type grub-core/kern/elf.c | 18 ++ grub-core/kern/elfXX.c | 101 + grub-core/loader/i386/bsdXX.c | 142 +- grub-core/loader/multiboot_elfxx.c | 79 ++- include/grub/elf.h | 23 +++ util/grub-module-verifierXX.c | 13 + 6 files changed, 290 insertions(+), 86 deletions(-) ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 5/6] util/grub-module-verifierXX.c: Add e_shoff check in get_shdr()
In util/grub-module-verifierXX.c, the function get_shdr() is used to obtain the
section header at a given index but isn't checking that there is an offset for
the section header table. To validate that there is, we can check that e_shoff
isn't 0.
Signed-off-by: Alec Brown
---
util/grub-module-verifierXX.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index 4e6cf133f..cf3ff0dfa 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -134,6 +134,9 @@ grub_target_to_host_real (const struct
grub_module_verifier_arch *arch, grub_uin
static Elf_Shdr *
get_shdr (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, Elf_Word
index)
{
+ if (grub_target_to_host (e->e_shoff) == 0)
+grub_util_error ("Invalid section header offset");
+
return (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff) +
index * grub_target_to_host16 (e->e_shentsize));
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 6/6] 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 4/6] 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 6e5eb3643..2cc1daa49 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 1/6] grub-core/loader/i386/bsdXX.c: Avoid downcasting (char *) to (Elf_Shdr *)
In bsdXX.c, a couple of untrusted loop bound and untrusted allocation size bugs
were flagged by Coverity in the functions grub_openbsd_find_ramdisk() and
grub_freebsd_load_elfmodule(). These bugs were flagged by coverity because the
variable shdr was downcasting from a char pointer to an Elf_Shdr pointer
whenever it was used to set the base value in for loops. To avoid this, we need
to set shdr as an Elf_Shdr pointer where it is initialized.
In the function read_headers(), the function is reading elf section header data
from a file and passing it to the variable shdr as data for a char pointer. If
we switch the type of shdr to an Elf_Shdr pointer in read_headers() as well as
other functions, then we won't need to downcast to an Elf_Shdr pointer. By doing
this, the issue becomes masked from Coverity's view. In the following patches,
we check limits to ensure the data isn't tainted.
Also, switched use of (char *) to (grub_uint8_t *) to give a better indication
of pointer arithmetic and not suggest use of a C string.
Fixes: CID 314018
Fixes: CID 314030
Fixes: CID 314031
Fixes: CID 314039
Signed-off-by: Alec Brown
---
grub-core/loader/i386/bsdXX.c | 71 ++-
1 file changed, 28 insertions(+), 43 deletions(-)
diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index e4f4aa365..e6edc6fb6 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -24,7 +24,7 @@ load (grub_file_t file, const char *filename, void *where,
grub_off_t off, grub_
}
static inline grub_err_t
-read_headers (grub_file_t file, const char *filename, Elf_Ehdr *e, char **shdr)
+read_headers (grub_file_t file, const char *filename, Elf_Ehdr *e, Elf_Shdr
**shdr)
{
if (grub_file_seek (file, 0) == (grub_off_t) -1)
return grub_errno;
@@ -77,8 +77,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct
grub_relocator *relocator,
char *argv[], grub_addr_t *kern_end)
{
Elf_Ehdr e;
- Elf_Shdr *s;
- char *shdr = 0;
+ Elf_Shdr *s, *shdr = NULL;
grub_addr_t curload, module;
grub_err_t err;
grub_size_t chunk_size = 0;
@@ -90,9 +89,8 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct
grub_relocator *relocator,
if (err)
goto out;
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum *
e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
continue;
@@ -113,9 +111,8 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct
grub_relocator *relocator,
chunk_src = get_virtual_current_address (ch);
}
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum *
e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
continue;
@@ -172,8 +169,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator
*relocator,
grub_addr_t *kern_end)
{
Elf_Ehdr e;
- Elf_Shdr *s;
- char *shdr = 0;
+ Elf_Shdr *s, *shdr = NULL;
grub_addr_t curload, module;
grub_err_t err;
grub_size_t chunk_size = 0;
@@ -185,9 +181,8 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator
*relocator,
if (err)
goto out;
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum *
e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
continue;
@@ -214,9 +209,8 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator
*relocator,
chunk_src = get_virtual_current_address (ch);
}
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum *
e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
continue;
@@ -284,8 +278,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator
*relocator,
{
grub_err_t err;
Elf_Ehdr e;
- Elf_Shdr *s;
- char *shdr = 0;
+ Elf_Shdr *s, *shdr = NULL;
unsigned symoff, stroff, symsize, strsize;
grub_freebsd_addr_t symstart, symend, symentsize, dynamic;
Elf_Sym *sym;
@@ -306,13 +299,11 @@ SUFFIX (grub_freebsd_load_
[PATCH 2/6] 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.
Also, changed casts of shnum to match variables being set as well as dropped
casts when unnecessary and fixed spacing errors in bsdXX.c.
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: %" PRIuGRUB_UINT64_T), (grub_uint64_t) *shnum);
+}
+ else
+{
+ if (*shnum >= SHN_LORESERVE)
+ return grub_error (GRUB_ERR_BAD_
[PATCH 3/6] 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 0d6b6612f..8e0384505 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_elf_get_shnumgrub_elf32_get_shnum
+# define grub_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_elf_get_shnumgrub_elf64_get_shnum
+# define grub_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_data_t *mld)
grub_relocator_chunk_t ch;
grub_uint32_t load_o
Re: [RFC PATCH v3 5/5] Cleanup: grub-mkconfig_lib: remove unused version comparison functions
Mathieu Desnoyers writes: > There are no users left of version_find_latest(), version_test_gt(), and > version_test_numeric(). Remove those unused helper functions. Using > those helper functions is what caused the quadratic sorting performance > issues in the first place, so removing them is a net win. > > Signed-off-by: Mathieu Desnoyers Reviewed-by: Robbie Harwood Be well, --Robbie signature.asc Description: PGP signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC PATCH v3 5/5] Cleanup: grub-mkconfig_lib: remove unused version comparison functions
On Thu, May 26, 2022 at 05:07:11PM -0400, Robbie Harwood wrote: > Mathieu Desnoyers writes: > > > There are no users left of version_find_latest(), version_test_gt(), and > > version_test_numeric(). Remove those unused helper functions. Using > > those helper functions is what caused the quadratic sorting performance > > issues in the first place, so removing them is a net win. > > > > Signed-off-by: Mathieu Desnoyers > > Reviewed-by: Robbie Harwood Hm. This seems to contradict your proposed patch to use distro specific sort by hooking into those functions got removed here. mkconfig: use distro sorts when available https://www.mail-archive.com/grub-devel@gnu.org/msg33357.html I'd like to know more your comments about this as those hooks might still be needed or where to keep distribution's sort ? Thanks, Michael > > Be well, > --Robbie > ___ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
