Re: [PATCH] search: new --efidisk-only option on EFI systems
Renaud Métrich writes: > When using 'search' on EFI systems, we sometimes want to exclude devices > that are not EFI disks (e.g. md, lvm). > This is typically used when wanting to chainload when having a software > raid (md) for EFI partition: > with no option, 'search --file /EFI/redhat/shimx64.efi' sets root envvar > to 'md/boot_efi' which cannot be used for chainloading since there is no > effective EFI device behind. > > Signed-off-by: Renaud Métrich The patch fixes the systems in question, and I don't see a way for it to break other things. 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: [PATCH 0/2] Support plain encryption mode.
--- Original Message --- > > > > Plainmount can work with '(hdX,gptY)' syntax in config or shell (actually, > > this > > > > is the base syntax) and thus it is not limited to GPT paritions, what is > > limited > > > > is the ability to use UUID - currently only on GPT. If partition scheme > > does not > > > > have UUID then UUID as a convenience feature cannot be supported - > > inconvenient, > > > > but technically fair. I will take a look at MBR UUID and see whether they > > can be > > > > supported. Possible situations (under current implementaion) are follows: > > > > a) GPT disk, multi-disk environment, disks map unpredictably: can name > > partitions > > > > by GPT UUID in config file/shell, no problem, ability to name by UUID has > > value > > I agree that searching by partition UUID is useful and desirable. > > However, I don't think this is the right approach. GRUB should have > > generic searching by partition UUID. There is already a patch for > > this[1]. Perhaps you can test/review this patch to help it gain more > > visibility and advocate for it being accepted. > > Glenn > > [1] https://lists.gnu.org/archive/html/grub-devel/2021-04/msg00055.html > Such function (or several functions) should be added into grub 'library', so it can be used to search disk by PART UUID in different places. The patch you refer to seems to add this functionality only to 'search' grub command via 'void grub_search_partuuid' function. Can it be reused on other places? It seems in oder to use it, grub code must call 'search' command and receive the result from grub environment variable which is not convinient for other grub code interested in this feature. I think the proper way to do it is to write some library function which can be used by search, probe (btw I borrowed some details from it - so there is code duplication in search/probe), plainmount commands and other commands in grub. Best regards, Maxim Fomin ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/2] plainmount: Support decryption of devices encrypted in plain mode.
--- Original Message ---
On Tuesday, February 1st, 2022 at 5:30, Glenn Washburn
wrote:
> On Sun, 30 Jan 2022 19:40:43 +
>
> Maxim Fomin ma...@fomin.one wrote:
>
> > This patch introduces support for plain encryption mode (plain dm-crypt) via
> >
> > new module and command named 'plainmount'. The command allows to open
> > devices
> >
> > encrypted in plain mode (without LUKS) with following syntax:
> >
> > plainmount -h hash -c cipher -o offset -s size -k key-size
> >
> > -z sector-size -d keyfile -O keyfile offset -l keyfile-size
> >
> > can be some partition or GPT UUID, keyfile can be /some/path.
>
> I'm not in favor of the keyfile UUID prefix for paths. Why not just use
>
> the normal device syntax?
Normal syntax can be used, it is base case syntax. Not supporting UUID for
keyfile (but supporting for device parameter) significantly limits possible
use cases because keyfile can be located on another drive. For example,
several linux wiki/manuals present one of several setup scenarios where
keyfile is located on a separate disk. This makes having support for UUID
for keyfile is almost surely necessary. However, support for UUID in
plainmount can be temporarily disabled untill the situation with search
command patch is resolved.
> >
> > +static const struct grub_arg_option options[] =
> >
> > - {
> > - /* TRANSLATORS: It's still restricted to this module only. */
>
> It would be nice to allow specifying a password as an argument (-p)
>
> like in cryptomount for consistency. It'll allow tests to no need to
>
> write a keyfile also.
I agree if this option is alternative way of requesting password and the
ability to type password remains.
>
> > - {"hash", 'h', 0, N_("Password hash."), 0, ARG_TYPE_STRING},
> > - {"cipher", 'c', 0, N_("Password cipher."), 0, ARG_TYPE_STRING},
> > - {"offset", 'o', 0, N_("Device offset (512 bit blocks)."), 0,
> > ARG_TYPE_INT},
>
> s/bit/byte/
>
> And why 512-byte blocks? The LUKS2 header stores offset as bytes,
>
> perhaps bytes should be used here too.
I followed cryptsetup command syntax for plain mode - it asks offset in terms of
512 byte segments because it is easier to type such number than number in bytes.
In general offset can be large and also inconvenient to type, but in practice
offset starts from 1-100 MiB, so this number is easier to remember (why it is
done this way in cryptsetup is my guess). Anyway, specifying in this way is more
familiar for cryptsetup users, but this can be changed.
> >
> > +/* Disk iterate callback */
> >
> > +static int grub_plainmount_scan_real (const char *name, void *data)
> >
> > +{
> >
> > - int ret = 0;
> > - struct grub_plainmount_iterate_args *args = data;
> > - grub_disk_t source = NULL, disk = NULL;
> > - struct grub_partition *partition;
> > - struct grub_gpt_partentry entry;
> > - grub_gpt_part_guid_t *guid;
> > - /* UUID format: ---- + '\0' */
> > - char uuid[37] = "";
> >
> > - source = grub_disk_open (name);
> > - if (!source)
> > -goto exit;
> >
> >
> > - if (!source->partition)
> > -goto exit;
> >
> >
> > - partition = source->partition;
> > - if (grub_strcmp (partition->partmap->name, "gpt") != 0)
> > -goto exit;
> >
> >
>
> As noted elsewhere, I'm not in favor of these checks, nor the forcing
>
> of the volume to be on a partition.
I didn't consider the case for disk instead of partition, will think about it.
> >
> >
> > -default:
> >
> >
> > - grub_error (GRUB_ERR_BAD_ARGUMENT,
> >
> >
> > - N_("invalid sector size -z %"PRIuGRUB_SIZE
> >
> >
> > - ", only 512/1024/2048/4096 are allowed"),
> >
> >
> > - cargs->sector_size);
> >
> >
> > - grub_print_error ();
> >
> >
> > - return GRUB_ERR_BAD_ARGUMENT;
> >
> >
>
> Should just set the error and return. Further up the call stack the
>
> error should be printed. So do something like
>
> return grub_error (GRUB_ERR_BAD_ARGUMENT,
>
> N_("invalid sector size -z %"PRIuGRUB_SIZE
>
> ", only 512/1024/2048/4096 are allowed"),
>
> cargs->sector_size);
Meaning move error printing from different places to the near of end of
grub_cmd_plainmount() (and possibly use grub_error_push/grub_error_pop)?
OK, I will take closer look at existing cryptodisk/luks behavior.
> > - cargs->sector_size, NULL);
> >
> >
> >
> > - if (cargs->size)
> > - total_sectors = cargs->size;
> > - else
> > - total_sectors = grub_disk_native_sectors (cargs->disk);
> >
> > - /* Calculate disk sectors in terms of log_sector_size */
> > - total_sectors = grub_convert_sector (total_sectors,
> > GRUB_DISK_SECTOR_BITS,
> > - dev->log_sector_size);
> >
> >
> > - dev->total_sectors = total_sectors - dev->offset_sectors;
> > - grub_dprintf ("plainmount", "log_sector_size=%d,
> > total_sectors=%"PRIuGRUB_SIZE
>
Re: [PATCH v3 0/4] Update gnulib and drop some patches
Glenn Washburn writes:
> On Thu, 27 Jan 2022 14:39:56 -0500
> Robbie Harwood wrote:
>
>> Changes in this version:
>> - Make the version of bootstrap match what it's supposed to
>> - Restore fix-width.patch at dkiper's request
>>
>> Be well,
>> --Robbie
>
> I presume this has been build tested, right?
>
> I'm getting the following compiler error with gcc 10.1.0:
>
> In file included from
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/disk /luks2.c:30:
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/disk/luks2.c: In
> function ‘luks2_verify_key’:
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/disk/luks2.c:398:75:
> error: pointer targets in passing argument 5 of ‘base64_decode_ctx’
> differ in signedness [-Werror=pointer-sign] 398 | if (!base64_decode
> (d->digest, grub_strlen (d->digest), (char *)digest, &digestlen)) |
>
> ^~
> |
> | |
> grub_size_t * {aka long unsigned int *}
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/lib/gnulib/base64.h:59:50:
> note: in definition of macro ‘base64_decode’ 59 |
> base64_decode_ctx (NULL, in, inlen, out, outlen) |
> ^~
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/lib/gnulib/base64.h:52:59:
> note: expected ‘idx_t *’ {aka ‘long int *’} but argument is of type
> ‘grub_size_t *’ {aka ‘long unsigned int *’}
>52 |char *restrict out, idx_t
> *outlen); |
> ~~~^~
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/disk/luks2.c:400:69:
> error: pointer targets in passing argument 5 of ‘base64_decode_ctx’
> differ in signedness [-Werror=pointer-sign] 400 | if (!base64_decode
> (d->salt, grub_strlen (d->salt), (char *)salt, &saltlen)) |
> ^~~~ |
>| |
>grub_size_t
> * {aka long unsigned int *}
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/lib/gnulib/base64.h:59:50:
> note: in definition of macro ‘base64_decode’ 59 |
> base64_decode_ctx (NULL, in, inlen, out, outlen) |
> ^~
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/lib/gnulib/base64.h:52:59:
> note: expected ‘idx_t *’ {aka ‘long int *’} but argument is of type
> ‘grub_size_t *’ {aka ‘long unsigned int *’} 52 |
> char *restrict out, idx_t *outlen); |
> ~~~^~
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/disk/luks2.c: In
> function ‘luks2_decrypt_key’:
> home/g10/grub-tests-uml-update-gnulib/grub/grub-core/disk/luks2.c:439:22:
> error: pointer targets in passing argument 5 of ‘base64_decode_ctx’
> differ in signedness [-Werror=pointer-sign] 439 |(char *)salt,
> &saltlen)) | ^~~~ | |
> | grub_size_t * {aka long unsigned int *}
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/lib/gnulib/base64.h:59:50:
> note: in definition of macro ‘base64_decode’ 59 |
> base64_decode_ctx (NULL, in, inlen, out, outlen) |
> ^~
> /home/g10/grub-tests-uml-update-gnulib/grub/grub-core/lib/gnulib/base64.h:52:59:
> note: expected ‘idx_t *’ {aka ‘long int *’} but argument is of type
> ‘grub_size_t *’ {aka ‘long unsigned int *’} 52 |
> char *restrict out, idx_t *outlen); |
> ~~~^~
> cc1: all warnings being treated as errors
>
> Should be an easy fix. What compiler version are you using?
gcc11-series. I'll see if I can reproduce in a VM and get a fix out.
Be well,
--Robbie
signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 4/4] Update gnulib version and drop most gnulib patches
In addition to the changes carried in our gnulib patches, several
Coverity and code hygiene fixes that were previously downstream are also
included in this 3-year gnulib increment.
Unfortunately, fix-width.patch is retained.
Bump minimum autoconf version from 2.63 to 2.64 as required by gnulib.
Sync bootstrap script itself with gnulib.
Update regexp module for new dynarray dependency.
Fix various new warnings.
Signed-off-by: Robbie Harwood
---
bootstrap | 319 ++
bootstrap.conf| 17 +-
conf/Makefile.extra-dist | 6 -
config.h.in | 15 +
configure.ac | 2 +-
grub-core/Makefile.core.def | 1 +
grub-core/disk/luks2.c| 4 +-
.../lib/gnulib-patches/fix-null-deref.patch | 13 -
.../gnulib-patches/fix-null-state-deref.patch | 12 -
.../fix-regcomp-uninit-token.patch| 15 -
.../fix-regexec-null-deref.patch | 12 -
.../gnulib-patches/fix-uninit-structure.patch | 11 -
.../lib/gnulib-patches/fix-unused-value.patch | 14 -
grub-core/lib/posix_wrap/limits.h | 6 +-
include/grub/compiler.h | 4 +-
include/grub/list.h | 2 +-
16 files changed, 218 insertions(+), 235 deletions(-)
delete mode 100644 grub-core/lib/gnulib-patches/fix-null-deref.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-null-state-deref.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-uninit-structure.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-unused-value.patch
diff --git a/bootstrap b/bootstrap
index 5b08e7e2d..dc2238f4a 100755
--- a/bootstrap
+++ b/bootstrap
@@ -1,10 +1,10 @@
#! /bin/sh
# Print a version string.
-scriptversion=2019-01-04.17; # UTC
+scriptversion=2022-01-26.05; # UTC
# Bootstrap this package from checked-out sources.
-# Copyright (C) 2003-2019 Free Software Foundation, Inc.
+# Copyright (C) 2003-2022 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -47,7 +47,7 @@ PERL="${PERL-perl}"
me=$0
-default_gnulib_url=git://git.sv.gnu.org/gnulib
+default_gnulib_url=https://git.savannah.gnu.org/git/gnulib.git
usage() {
cat .
+This is free software: you are free to change and redistribute it.
+There is NO WARRANTY, to the extent permitted by law."
+
# warnf_ FORMAT-STRING ARG1...
warnf_ ()
{
@@ -154,6 +162,18 @@ gnulib_files=
: ${AUTOPOINT=autopoint}
: ${AUTORECONF=autoreconf}
+# A function to be called for each unrecognized option. Returns 0 if
+# the option in $1 has been processed by the function. Returns 1 if
+# the option has not been processed by the function. Override it via
+# your own definition in bootstrap.conf
+
+bootstrap_option_hook() { return 1; }
+
+# A function to be called in order to print the --help information
+# corresponding to user-defined command-line options.
+
+bootstrap_print_option_usage_hook() { :; }
+
# A function to be called right after gnulib-tool is run.
# Override it via your own definition in bootstrap.conf.
bootstrap_post_import_hook() { :; }
@@ -166,11 +186,11 @@ bootstrap_epilogue() { :; }
# specified directory. Fill in the first %s with the destination
# directory and the second with the domain name.
po_download_command_format=\
-"wget --mirror --level=1 -nd -q -A.po -P '%s' \
+"wget --mirror --level=1 -nd -nv -A.po -P '%s' \
https://translationproject.org/latest/%s/";
# Prefer a non-empty tarname (4th argument of AC_INIT if given), else
-# fall back to the package name (1st argument with munging)
+# fall back to the package name (1st argument with munging).
extract_package_name='
/^AC_INIT(\[*/{
s///
@@ -187,8 +207,11 @@ extract_package_name='
p
}
'
-package=$(sed -n "$extract_package_name" configure.ac) \
- || die 'cannot find package name in configure.ac'
+package=$(${AUTOCONF:-autoconf} --trace AC_INIT:\$4 configure.ac 2>/dev/null)
+if test -z "$package"; then
+ package=$(sed -n "$extract_package_name" configure.ac) \
+ || die 'cannot find package name in configure.ac'
+fi
gnulib_name=lib$package
build_aux=build-aux
@@ -290,62 +313,6 @@ find_tool ()
eval "export $find_tool_envvar"
}
-# Override the default configuration, if necessary.
-# Make sure that bootstrap.conf is sourced from the current directory
-# if we were invoked as "sh bootstrap".
-case "$0" in
- */*) test -r "$0.conf" && . "$0.conf" ;;
- *) test -r "$0.conf" && . ./"$0.conf" ;;
-esac
-
-if test "$vc_ignore" = auto; then
- vc_ignore=
- test -d .git && vc_ignore=.gitignore
- te
[PATCH v4 1/4] Use visual indentation in config.h.in
Signed-off-by: Robbie Harwood --- config.h.in | 56 ++--- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/config.h.in b/config.h.in index 9e8f9911b..1c8c05b9c 100644 --- a/config.h.in +++ b/config.h.in @@ -22,46 +22,46 @@ #define MINILZO_CFG_SKIP_LZO1X_DECOMPRESS 1 #if defined (GRUB_BUILD) -#undef ENABLE_NLS -#define BUILD_SIZEOF_LONG @BUILD_SIZEOF_LONG@ -#define BUILD_SIZEOF_VOID_P @BUILD_SIZEOF_VOID_P@ -#if defined __APPLE__ -# if defined __BIG_ENDIAN__ -# define BUILD_WORDS_BIGENDIAN 1 -# else -# define BUILD_WORDS_BIGENDIAN 0 -# endif -#else -#define BUILD_WORDS_BIGENDIAN @BUILD_WORDS_BIGENDIAN@ -#endif +# undef ENABLE_NLS +# define BUILD_SIZEOF_LONG @BUILD_SIZEOF_LONG@ +# define BUILD_SIZEOF_VOID_P @BUILD_SIZEOF_VOID_P@ +# if defined __APPLE__ +#if defined __BIG_ENDIAN__ +# define BUILD_WORDS_BIGENDIAN 1 +#else +# define BUILD_WORDS_BIGENDIAN 0 +#endif +# else /* !defined __APPLE__ */ +#define BUILD_WORDS_BIGENDIAN @BUILD_WORDS_BIGENDIAN@ +# endif /* !defined __APPLE__ */ #elif defined (GRUB_UTIL) || !defined (GRUB_MACHINE) -#include -#else -#define HAVE_FONT_SOURCE @HAVE_FONT_SOURCE@ +# include +#else /* !defined GRUB_UTIL && defined GRUB_MACHINE */ +# define HAVE_FONT_SOURCE @HAVE_FONT_SOURCE@ /* Define if C symbols get an underscore after compilation. */ -#define HAVE_ASM_USCORE @HAVE_ASM_USCORE@ +# define HAVE_ASM_USCORE @HAVE_ASM_USCORE@ /* Define it to one of __bss_start, edata and _edata. */ -#define BSS_START_SYMBOL @BSS_START_SYMBOL@ +# define BSS_START_SYMBOL @BSS_START_SYMBOL@ /* Define it to either end or _end. */ -#define END_SYMBOL @END_SYMBOL@ +# define END_SYMBOL @END_SYMBOL@ /* Name of package. */ -#define PACKAGE "@PACKAGE@" +# define PACKAGE "@PACKAGE@" /* Version number of package. */ -#define VERSION "@VERSION@" +# define VERSION "@VERSION@" /* Define to the full name and version of this package. */ -#define PACKAGE_STRING "@PACKAGE_STRING@" +# define PACKAGE_STRING "@PACKAGE_STRING@" /* Define to the version of this package. */ -#define PACKAGE_VERSION "@PACKAGE_VERSION@" +# define PACKAGE_VERSION "@PACKAGE_VERSION@" /* Define to the full name of this package. */ -#define PACKAGE_NAME "@PACKAGE_NAME@" +# define PACKAGE_NAME "@PACKAGE_NAME@" /* Define to the address where bug reports for this package should be sent. */ -#define PACKAGE_BUGREPORT "@PACKAGE_BUGREPORT@" +# define PACKAGE_BUGREPORT "@PACKAGE_BUGREPORT@" -#define GRUB_TARGET_CPU "@GRUB_TARGET_CPU@" -#define GRUB_PLATFORM "@GRUB_PLATFORM@" +# define GRUB_TARGET_CPU "@GRUB_TARGET_CPU@" +# define GRUB_PLATFORM "@GRUB_PLATFORM@" -#define RE_ENABLE_I18N 1 +# define RE_ENABLE_I18N 1 -#define _GNU_SOURCE 1 +# define _GNU_SOURCE 1 #endif -- 2.34.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 0/4] Update gnulib and drop some patches
Changes in this version: - Address fix several warnings Glenn pointed out. Note that gnulib isn't clean with our default cflags - they prefer different ones. (I had previously missed these because of how much stuff I turn off to build with our old gnulib copy.) - Update gnulib target version again while we're here Be well, --Robbie Robbie Harwood (4): Use visual indentation in config.h.in Drop gnulib fix-base64.patch Drop gnulib no-abort.patch Update gnulib version and drop most gnulib patches bootstrap | 319 ++ bootstrap.conf| 18 +- conf/Makefile.extra-dist | 8 - config.h.in | 76 +++-- configure.ac | 2 +- grub-core/Makefile.core.def | 1 + grub-core/disk/luks2.c| 4 +- grub-core/lib/gnulib-patches/fix-base64.patch | 21 -- .../lib/gnulib-patches/fix-null-deref.patch | 13 - .../gnulib-patches/fix-null-state-deref.patch | 12 - .../fix-regcomp-uninit-token.patch| 15 - .../fix-regexec-null-deref.patch | 12 - .../gnulib-patches/fix-uninit-structure.patch | 11 - .../lib/gnulib-patches/fix-unused-value.patch | 14 - grub-core/lib/gnulib-patches/no-abort.patch | 26 -- grub-core/lib/posix_wrap/limits.h | 6 +- grub-core/lib/posix_wrap/sys/types.h | 7 +- grub-core/lib/xzembed/xz.h| 5 +- include/grub/compiler.h | 4 +- include/grub/list.h | 2 +- 20 files changed, 256 insertions(+), 320 deletions(-) delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch delete mode 100644 grub-core/lib/gnulib-patches/fix-null-deref.patch delete mode 100644 grub-core/lib/gnulib-patches/fix-null-state-deref.patch delete mode 100644 grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch delete mode 100644 grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch delete mode 100644 grub-core/lib/gnulib-patches/fix-uninit-structure.patch delete mode 100644 grub-core/lib/gnulib-patches/fix-unused-value.patch delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch -- 2.34.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 3/4] Drop gnulib no-abort.patch
Originally added in db7337a3d353a817ffe9eb4a3702120527100be9, this
patched out all relevant invocations of abort() in gnulib. While it was
not documented why at the time, testing suggests that there's no abort()
implementation available for gnulib to use.
gnulib's position is that the use of abort() is correct here, since it
happens when input violates a "shall" from POSIX. Additionally, the
code in question is probably not reachable. Since abort() is more
friendly to user-space, they prefer to make no change, so we can just
carry a define instead. (Suggested by Paul Eggert.)
Signed-off-by: Robbie Harwood
---
bootstrap.conf | 2 +-
conf/Makefile.extra-dist| 1 -
config.h.in | 3 +++
grub-core/lib/gnulib-patches/no-abort.patch | 26 -
4 files changed, 4 insertions(+), 28 deletions(-)
delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index 21a8cf15d..71acbeeb1 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -82,7 +82,7 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
for patchname in fix-null-deref fix-null-state-deref
fix-regcomp-uninit-token \
- fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width
no-abort; do
+ fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width;
do
patch -d grub-core/lib/gnulib -p2 \
< "grub-core/lib/gnulib-patches/$patchname.patch"
done
diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index 15a9b74e9..4ddc3c8f7 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -35,7 +35,6 @@ EXTRA_DIST +=
grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
-EXTRA_DIST += grub-core/lib/gnulib-patches/no-abort.patch
EXTRA_DIST += grub-core/lib/libgcrypt
EXTRA_DIST += grub-core/lib/libgcrypt-grub/mpi/generic
diff --git a/config.h.in b/config.h.in
index 751802f91..66c16abfd 100644
--- a/config.h.in
+++ b/config.h.in
@@ -66,4 +66,7 @@
# define _GL_ATTRIBUTE_CONST
+/* We don't have an abort() for gnulib to call in regexp. */
+# define abort() DEBUG_ASSERT(0)
+
#endif
diff --git a/grub-core/lib/gnulib-patches/no-abort.patch
b/grub-core/lib/gnulib-patches/no-abort.patch
deleted file mode 100644
index e469c4762..0
--- a/grub-core/lib/gnulib-patches/no-abort.patch
+++ /dev/null
@@ -1,26 +0,0 @@
-diff --git a/lib/regcomp.c b/lib/regcomp.c
-index cc85f35ac..de45ebb5c 100644
a/lib/regcomp.c
-+++ b/lib/regcomp.c
-@@ -528,9 +528,9 @@ regerror (int errcode, const regex_t *__restrict preg,
char *__restrict errbuf,
-to this routine. If we are given anything else, or if other regex
-code generates an invalid error code, then the program has a bug.
-Dump core so we can fix it. */
--abort ();
--
-- msg = gettext (__re_error_msgid + __re_error_msgid_idx[errcode]);
-+msg = gettext ("unknown regexp error");
-+ else
-+msg = gettext (__re_error_msgid + __re_error_msgid_idx[errcode]);
-
- msg_size = strlen (msg) + 1; /* Includes the null. */
-
-@@ -1136,7 +1136,7 @@ optimize_utf8 (re_dfa_t *dfa)
- }
- break;
- default:
-- abort ();
-+ break;
- }
-
- if (mb_chars || has_period)
--
2.34.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 2/4] Drop gnulib fix-base64.patch
Originally added in 9fbdec2f6b4fa8b549daa4d49134d1fe89d95ef9 and
subsequently modified in 552c9fd08122a3036c724ce96dfe68aa2f75705f,
fix-base64.patch handled two problems we have using gnulib, which are
exerciesd by the base64 module but not directly caused by it.
First, grub2 defines its own bool type, while gnulib expects the
equivalent of stdbool.h to be present. Rather than patching gnulib,
instead use gnulib's stdbool module to provide a bool type if needed.
(Suggested by Simon Josefsson.)
Second, our config.h doesn't always inherit config-util.h, which is
where gnulib-related options like _GL_ATTRIBUTE_CONST end up.
fix-base64.h worked around this by defining the attribute away, but this
workaround is better placed in config.h itself, not a gnulib patch.
Signed-off-by: Robbie Harwood
---
bootstrap.conf| 3 ++-
conf/Makefile.extra-dist | 1 -
config.h.in | 2 ++
grub-core/lib/gnulib-patches/fix-base64.patch | 21 ---
grub-core/lib/posix_wrap/sys/types.h | 7 +++
grub-core/lib/xzembed/xz.h| 5 +
6 files changed, 8 insertions(+), 31 deletions(-)
delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index 0dd893c5c..21a8cf15d 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -35,6 +35,7 @@ gnulib_modules="
realloc-gnu
regex
save-cwd
+ stdbool
"
gnulib_tool_option_extras="\
@@ -80,7 +81,7 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-base64 fix-null-deref fix-null-state-deref
fix-regcomp-uninit-token \
+ for patchname in fix-null-deref fix-null-state-deref
fix-regcomp-uninit-token \
fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width
no-abort; do
patch -d grub-core/lib/gnulib -p2 \
< "grub-core/lib/gnulib-patches/$patchname.patch"
diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index 8f1485d52..15a9b74e9 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -28,7 +28,6 @@ EXTRA_DIST += grub-core/gensymlist.sh
EXTRA_DIST += grub-core/genemuinit.sh
EXTRA_DIST += grub-core/genemuinitheader.sh
-EXTRA_DIST += grub-core/lib/gnulib-patches/fix-base64.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-state-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
diff --git a/config.h.in b/config.h.in
index 1c8c05b9c..751802f91 100644
--- a/config.h.in
+++ b/config.h.in
@@ -64,4 +64,6 @@
# define _GNU_SOURCE 1
+# define _GL_ATTRIBUTE_CONST
+
#endif
diff --git a/grub-core/lib/gnulib-patches/fix-base64.patch
b/grub-core/lib/gnulib-patches/fix-base64.patch
deleted file mode 100644
index 985db1279..0
--- a/grub-core/lib/gnulib-patches/fix-base64.patch
+++ /dev/null
@@ -1,21 +0,0 @@
-diff --git a/lib/base64.h b/lib/base64.h
-index 9cd0183b8..185a2afa1 100644
a/lib/base64.h
-+++ b/lib/base64.h
-@@ -21,8 +21,14 @@
- /* Get size_t. */
- # include
-
--/* Get bool. */
--# include
-+#ifndef GRUB_POSIX_BOOL_DEFINED
-+typedef enum { false = 0, true = 1 } bool;
-+#define GRUB_POSIX_BOOL_DEFINED 1
-+#endif
-+
-+#ifndef _GL_ATTRIBUTE_CONST
-+# define _GL_ATTRIBUTE_CONST /* empty */
-+#endif
-
- # ifdef __cplusplus
- extern "C" {
diff --git a/grub-core/lib/posix_wrap/sys/types.h
b/grub-core/lib/posix_wrap/sys/types.h
index 854eb0122..eeda543c4 100644
--- a/grub-core/lib/posix_wrap/sys/types.h
+++ b/grub-core/lib/posix_wrap/sys/types.h
@@ -23,11 +23,10 @@
#include
+/* Provided by gnulib if not present. */
+#include
+
typedef grub_ssize_t ssize_t;
-#ifndef GRUB_POSIX_BOOL_DEFINED
-typedef enum { false = 0, true = 1 } bool;
-#define GRUB_POSIX_BOOL_DEFINED 1
-#endif
typedef grub_uint8_t uint8_t;
typedef grub_uint16_t uint16_t;
diff --git a/grub-core/lib/xzembed/xz.h b/grub-core/lib/xzembed/xz.h
index f7b32d800..d1417039a 100644
--- a/grub-core/lib/xzembed/xz.h
+++ b/grub-core/lib/xzembed/xz.h
@@ -29,10 +29,7 @@
#include
#include
#include
-
-#ifndef GRUB_POSIX_BOOL_DEFINED
-typedef enum { false = 0, true = 1 } bool;
-#endif
+#include
/**
* enum xz_ret - Return codes
--
2.34.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] affs: Fix resource leaks
In commit 178ac5107389 (affs: Fix memory leaks), fixes were made to
grub_affs_iterate_dir() to prevent memory leaks from occuring after it returns
without freeing node. However, there were still some instances where node was
causing a memory leak when the function returns after calling
grub_affs_create_node(). In this function, new memory is allocated to node but
doesn't get freed until the hook() function is called near the end. Before
hook() is called, node should be freed in grub_affs_create_node() before
returning out of it.
Fixes: 178ac5107389 (affs: Fix memory leaks)
Fixes: CID 73759
Signed-off-by: Alec Brown
---
grub-core/fs/affs.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/grub-core/fs/affs.c b/grub-core/fs/affs.c
index cafcd0fba..7b9e62064 100644
--- a/grub-core/fs/affs.c
+++ b/grub-core/fs/affs.c
@@ -370,17 +370,26 @@ grub_affs_create_node (grub_fshelp_node_t dir,
GRUB_DISK_SECTOR_SIZE -
GRUB_AFFS_FILE_LOCATION,
sizeof ((*node)->di), (char *) &(*node)->di);
if (err)
- return 1;
+ {
+ grub_free (*node);
+ return 1;
+ }
continue;
}
default:
- return 0;
+ {
+ grub_free (*node);
+ return 0;
+ }
}
break;
}
if (nest == 8)
-return 0;
+{
+ grub_free (*node);
+ return 0;
+}
type |= GRUB_FSHELP_CASE_INSENSITIVE;
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 1/4] util/grub-module-verifierXX.c: Add function to calculate section headers
Added the function get_shdr() which returns the section header at a given index
parameter passed into this function. This helps traverse the section header
table and reduces repeated calls to lengthy equations used to obtain section
headers.
Note that it may look as though the argument *arch isn't being used, it's
actually required in order to use the macros grub_target_to_host*(x) which are
unwinded to grub_target_to_host*_real(arch, (x)) based on defines earlier in the
file.
Signed-off-by: Alec Brown
---
util/grub-module-verifierXX.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index ceb24309a..92f9ae2a4 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -131,6 +131,12 @@ grub_target_to_host_real (const struct
grub_module_verifier_arch *arch, grub_uin
return grub_target_to_host32_real (arch, in);
}
+static Elf_Shdr *
+get_shdr (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, Elf_Word
index)
+{
+ return (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff) +
+ index * grub_target_to_host16 (e->e_shentsize));
+}
static Elf_Shdr *
find_section (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, const
char *name)
@@ -139,12 +145,12 @@ find_section (const struct grub_module_verifier_arch
*arch, Elf_Ehdr *e, const c
const char *str;
unsigned i;
- s = (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff) +
grub_target_to_host16 (e->e_shstrndx) * grub_target_to_host16 (e->e_shentsize));
+ s = get_shdr (arch, e, grub_target_to_host16 (e->e_shstrndx));
str = (char *) e + grub_target_to_host (s->sh_offset);
- for (i = 0, s = (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff));
+ for (i = 0, s = get_shdr (arch, e, 0);
i < grub_target_to_host16 (e->e_shnum);
- i++, s = (Elf_Shdr *) ((char *) s + grub_target_to_host16
(e->e_shentsize)))
+ i++, s = get_shdr (arch, e, i))
if (strcmp (str + grub_target_to_host32 (s->sh_name), name) == 0)
return s;
return NULL;
@@ -166,13 +172,12 @@ static Elf_Sym *
get_symtab (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e,
Elf_Word *size, Elf_Word *entsize)
{
unsigned i;
- Elf_Shdr *s, *sections;
+ Elf_Shdr *s;
Elf_Sym *sym;
- sections = (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff));
- for (i = 0, s = sections;
+ for (i = 0, s = get_shdr (arch, e, 0);
i < grub_target_to_host16 (e->e_shnum);
- i++, s = (Elf_Shdr *) ((char *) s + grub_target_to_host16
(e->e_shentsize)))
+ i++, s = get_shdr (arch, e, i))
if (grub_target_to_host32 (s->sh_type) == SHT_SYMTAB)
break;
@@ -364,9 +369,9 @@ check_relocations (const char * const modname,
Elf_Shdr *s;
unsigned i;
- for (i = 0, s = (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff));
+ for (i = 0, s = get_shdr (arch, e, 0);
i < grub_target_to_host16 (e->e_shnum);
- i++, s = (Elf_Shdr *) ((char *) s + grub_target_to_host16
(e->e_shentsize)))
+ i++, s = get_shdr (arch, e, i))
if (grub_target_to_host32 (s->sh_type) == SHT_REL || grub_target_to_host32
(s->sh_type) == SHT_RELA)
{
Elf_Shdr *ts;
@@ -379,7 +384,7 @@ check_relocations (const char * const modname,
/* Find the target segment. */
if (grub_target_to_host32 (s->sh_info) >= grub_target_to_host16
(e->e_shnum))
grub_util_error ("%s: orphaned reloc section", modname);
- ts = (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff) +
grub_target_to_host32 (s->sh_info) * grub_target_to_host16 (e->e_shentsize));
+ ts = get_shdr (arch, e, grub_target_to_host32 (s->sh_info));
section_check_relocations (modname, arch, e, s, grub_target_to_host
(ts->sh_size));
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 3/4] util/grub-module-verifierXX.c: Validate elf section header table index for section name string table
In grub-module-verifierXX.c, the function find_section() uses the value from
grub_target_to_host16 (e->e_shstrndx) 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, the function get_shstrndx() is being added
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 it may look as though the argument *arch isn't being used, it's
actually required in order to use the macros grub_target_to_host*(x) which are
unwinded to grub_target_to_host*_real(arch, (x)) based on defines earlier in the
file.
Signed-off-by: Alec Brown
---
util/grub-module-verifierXX.c | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index 61b82141f..3a5265aff 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -161,6 +161,29 @@ get_shnum (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e)
return shnum;
}
+static Elf_Word
+get_shstrndx (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e)
+{
+ Elf_Shdr *s;
+ Elf_Word shstrndx;
+
+ shstrndx = grub_target_to_host16 (e->e_shstrndx);
+ if (shstrndx == SHN_XINDEX)
+{
+ s = get_shdr (arch, e, 0);
+ shstrndx = grub_target_to_host (s->sh_link);
+ if (shstrndx < SHN_LORESERVE)
+ grub_util_error ("Invalid section header table index in sh_link: %d",
shstrndx);
+}
+ else
+{
+ if (shstrndx >= SHN_LORESERVE)
+ grub_util_error ("Invalid section header table index in e_shstrndx:
%d", shstrndx);
+}
+
+ return shstrndx;
+}
+
static Elf_Shdr *
find_section (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, const
char *name)
{
@@ -168,7 +191,7 @@ find_section (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e, const c
const char *str;
unsigned i;
- s = get_shdr (arch, e, grub_target_to_host16 (e->e_shstrndx));
+ s = get_shdr (arch, e, get_shstrndx (arch, e));
str = (char *) e + grub_target_to_host (s->sh_offset);
for (i = 0, s = get_shdr (arch, e, 0);
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 0/4] Clean up code and fix coverity bugs in util/grub-module-verifierXX.c
Coverity identified several untrusted loop bounds in util/grub-module-verifierXX.c. This patch series addresses these bugs, cleans up lengthy equations, and makes checks to values based on the elf manual page. The Coverity Bugs being addressed are: CID 314021 CID 314027 CID 314033 Alec Brown (4): util/grub-module-verifierXX.c: Add function to calculate section headers util/grub-module-verifierXX.c: Validate number of elf section header table entries util/grub-module-verifierXX.c: Validate elf section header table index for section name string table util/grub-module-verifierXX.c: Add module_size parameter to functions for sanity checking util/grub-module-verifierXX.c | 124 +--- 1 file changed, 93 insertions(+), 31 deletions(-) ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 4/4] util/grub-module-verifierXX.c: Add module_size parameter to functions for sanity checking
In grub-module-verifierXX.c, the function grub_module_verifyXX() performs an
initial check that the ELF section headers are within the module's size but
doesn't check if the sections being accessed have contents that are within the
module's size. In particular, we need to check that sh_offset and sh_size are
less than the module's size. However, for some section header types we don't
need to make these checks. For the type SHT_NULL, the section header is marked
as inactive and the rest of the members within the section header have undefined
values, so we don't need to check for sh_offset or sh_size. In the case of the
type SHT_NOBITS, sh_offset has a conceptual offset which may be beyond the
module size. Also, this type's sh_size may have a non-zero size, but a section
of this type will take up no space in the module. This can all be checked in the
function get_shdr(), but in order to do so, the parameter module_size must be
added to functions so that the value of the module size can be used in
get_shdr() from grub_module_verifyXX().
Signed-off-by: Alec Brown
---
util/grub-module-verifierXX.c | 69 ---
1 file changed, 40 insertions(+), 29 deletions(-)
diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index 3a5265aff..ac7b6fa78 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -132,10 +132,21 @@ 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)
+get_shdr (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, Elf_Word
index, size_t module_size)
{
- return (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff) +
- index * grub_target_to_host16 (e->e_shentsize));
+ Elf_Shdr *s;
+
+ s = (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff) +
+ index * grub_target_to_host16 (e->e_shentsize));
+ /* Check that the header being accessed isn't outside the module size */
+ if (grub_target_to_host32 (s->sh_type) != SHT_NULL && grub_target_to_host32
(s->sh_type) != SHT_NOBITS)
+{
+ if (grub_target_to_host (s->sh_offset) > module_size)
+ grub_util_error ("Section %d starts after the end of the module",
index);
+ if (grub_target_to_host (s->sh_offset) + grub_target_to_host
(s->sh_size) > module_size)
+ grub_util_error ("Section %d ends after the end of the module", index);
+}
+ return s;
}
static Elf_Word
@@ -147,7 +158,7 @@ get_shnum (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e)
shnum = grub_target_to_host16 (e->e_shnum);
if (shnum == SHN_UNDEF)
{
- s = get_shdr (arch, e, 0);
+ s = get_shdr (arch, e, 0, 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);
@@ -162,7 +173,7 @@ get_shnum (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e)
}
static Elf_Word
-get_shstrndx (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e)
+get_shstrndx (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e,
size_t module_size)
{
Elf_Shdr *s;
Elf_Word shstrndx;
@@ -170,7 +181,7 @@ get_shstrndx (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e)
shstrndx = grub_target_to_host16 (e->e_shstrndx);
if (shstrndx == SHN_XINDEX)
{
- s = get_shdr (arch, e, 0);
+ s = get_shdr (arch, e, 0, 0);
shstrndx = grub_target_to_host (s->sh_link);
if (shstrndx < SHN_LORESERVE)
grub_util_error ("Invalid section header table index in sh_link: %d",
shstrndx);
@@ -185,18 +196,18 @@ get_shstrndx (const struct grub_module_verifier_arch
*arch, Elf_Ehdr *e)
}
static Elf_Shdr *
-find_section (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, const
char *name)
+find_section (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, const
char *name, size_t module_size)
{
Elf_Shdr *s;
const char *str;
unsigned i;
- s = get_shdr (arch, e, get_shstrndx (arch, e));
+ s = get_shdr (arch, e, get_shstrndx (arch, e, module_size), module_size);
str = (char *) e + grub_target_to_host (s->sh_offset);
- for (i = 0, s = get_shdr (arch, e, 0);
+ for (i = 0, s = get_shdr (arch, e, 0, 0);
i < get_shnum (arch, e);
- i++, s = get_shdr (arch, e, i))
+ i++, s = get_shdr (arch, e, i, module_size))
if (strcmp (str + grub_target_to_host32 (s->sh_name), name) == 0)
return s;
return NULL;
@@ -204,9 +215,9 @@ find_section (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e, const c
static void
check_license (const char * const filename,
- const struct grub_module_verifier_arch *arch, Elf_Ehdr *e)
+ const struct grub_module_verifier_arch *arch, Elf_Ehdr *e,
size_t module_size)
{
- Elf_Shdr *s = find_section
[PATCH 2/4] util/grub-module-verifierXX.c: Validate number of elf section header table entries
In grub-module-verifierXX.c, grub_target_to_host16 (e->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, the function get_shnum() is being added 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.
Note that it may look as though the argument *arch isn't being used, it's
actually required in order to use the macros grub_target_to_host*(x) which are
unwinded to grub_target_to_host*_real(arch, (x)) based on defines earlier in the
file.
Fixes: CID 314021
Fixes: CID 314027
Fixes: CID 314033
Signed-off-by: Alec Brown
---
util/grub-module-verifierXX.c | 35 +--
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index 92f9ae2a4..61b82141f 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -138,6 +138,29 @@ 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
+get_shnum (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e)
+{
+ Elf_Shdr *s;
+ Elf_Word shnum;
+
+ shnum = grub_target_to_host16 (e->e_shnum);
+ if (shnum == SHN_UNDEF)
+{
+ 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);
+}
+ else
+{
+ if (shnum >= SHN_LORESERVE)
+ grub_util_error ("Invalid number of section header table entries in
e_shnum: %d", shnum);
+}
+
+ return shnum;
+}
+
static Elf_Shdr *
find_section (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, const
char *name)
{
@@ -149,7 +172,7 @@ find_section (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e, const c
str = (char *) e + grub_target_to_host (s->sh_offset);
for (i = 0, s = get_shdr (arch, e, 0);
- i < grub_target_to_host16 (e->e_shnum);
+ i < get_shnum (arch, e);
i++, s = get_shdr (arch, e, i))
if (strcmp (str + grub_target_to_host32 (s->sh_name), name) == 0)
return s;
@@ -176,12 +199,12 @@ get_symtab (const struct grub_module_verifier_arch *arch,
Elf_Ehdr *e, Elf_Word
Elf_Sym *sym;
for (i = 0, s = get_shdr (arch, e, 0);
- i < grub_target_to_host16 (e->e_shnum);
+ i < get_shnum (arch, e);
i++, s = get_shdr (arch, e, i))
if (grub_target_to_host32 (s->sh_type) == SHT_SYMTAB)
break;
- if (i == grub_target_to_host16 (e->e_shnum))
+ if (i == get_shnum (arch, e))
return NULL;
sym = (Elf_Sym *) ((char *) e + grub_target_to_host (s->sh_offset));
@@ -370,7 +393,7 @@ check_relocations (const char * const modname,
unsigned i;
for (i = 0, s = get_shdr (arch, e, 0);
- i < grub_target_to_host16 (e->e_shnum);
+ i < get_shnum (arch, e);
i++, s = get_shdr (arch, e, i))
if (grub_target_to_host32 (s->sh_type) == SHT_REL || grub_target_to_host32
(s->sh_type) == SHT_RELA)
{
@@ -382,7 +405,7 @@ check_relocations (const char * const modname,
grub_util_error ("%s: unsupported SHT_RELA", modname);
/* Find the target segment. */
- if (grub_target_to_host32 (s->sh_info) >= grub_target_to_host16
(e->e_shnum))
+ if (grub_target_to_host32 (s->sh_info) >= get_shnum (arch, e))
grub_util_error ("%s: orphaned reloc section", modname);
ts = get_shdr (arch, e, grub_target_to_host32 (s->sh_info));
@@ -423,7 +446,7 @@ SUFFIX(grub_module_verify) (const char * const filename,
/* Make sure that every section is within the core. */
if (size < grub_target_to_host (e->e_shoff)
- + (grub_uint32_t) grub_target_to_host16 (e->e_shentsize) *
grub_target_to_host16(e->e_shnum))
+ + (grub_uint32_t) grub_target_to_host16 (e->e_shentsize) * get_shnum
(arch, e))
{
grub_util_error ("%s: ELF sections outside core", filename);
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
