Re: [PATCH] password: Fix backspace in username prompt
I used grub_printf 3 times, because for some reason (line wrapping I
guess) if you print "\b \b" at once the backspace key doesn't work on
the second last character in the terminal line. The visual cursor gets
stuck there and doesn't remove characters anymore, although you can
still type more.
This may be irrelevant because I doubt anyone is using a username longer
than one line, but it solves the problem.
On 3/1/21 8:26 PM, Lennart Sorensen wrote:
On Mon, Mar 01, 2021 at 10:58:40AM +0300, Egor Ignatov wrote:
From: Egor Ignatov
Make backspace work in superuser login prompt.
The problem was that bidi logical to visual ignored BN type,
so you couldn't print control characters.
Use grub_printf() 3 times, because a line wrap will cause
the cursor to get stuck at the end of the terminal line.
Resolves: #60114
Signed-off-by: Egor Ignatov
---
grub-core/normal/auth.c| 4 +++-
grub-core/normal/charset.c | 1 +
grub-core/term/gfxterm.c | 11 +--
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/grub-core/normal/auth.c b/grub-core/normal/auth.c
index 6be678c0d..ffbf6d890 100644
--- a/grub-core/normal/auth.c
+++ b/grub-core/normal/auth.c
@@ -177,7 +177,9 @@ grub_username_get (char buf[], unsigned buf_size)
if (cur_len)
{
cur_len--;
- grub_printf ("\b \b");
+ grub_printf ("\b");
+ grub_printf (" ");
+ grub_printf ("\b");
}
continue;
}
Is this the part that the commit message refers to? I must admit I
am not quite sure why this change makes a difference, but if it does,
perhaps it is important (and non obvious) enough that the code should
actually have a comment explaining it, or someone might come by and
clean it up again later.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] disk/pata: Suppress error message "no device connected"
Dear Glenn,
Am 01.03.21 um 20:36 schrieb Glenn Washburn:
This error message comes from the grub_print_error in
grub_pata_device_initialize, which does not pass on the error, and is
raised in check_device. The function check_device needs to return this as
an error because check_device is also used in grub_pata_open, which does
pass on this error to indicate that the device can not be used.
This is actually not an error when displayed by grub_pata_device_initialize
because it just indicates that there are no pata devices seen. This may be
confusing to end users who do not have pata devices yet are loading the
pata module (perhaps implicitly via nativedisk). This also causes unnessary
output which may need to be accounted for in functional testing.
Instead print to the debug log when check_device raises this "error" and pop
the error from the error stack. If there is another error on the stack then
print the error stack as those should be real errors.
Signed-off-by: Glenn Washburn
---
grub-core/disk/pata.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/grub-core/disk/pata.c b/grub-core/disk/pata.c
index 23eef2be1..b15aeaa13 100644
--- a/grub-core/disk/pata.c
+++ b/grub-core/disk/pata.c
@@ -331,6 +331,12 @@ grub_pata_device_initialize (int port, int device, int
addr)
*devp = dev;
err = check_device (dev);
+ if (err == GRUB_ERR_UNKNOWN_DEVICE)
+{
+ grub_dprintf ("pata", "%s\n", grub_errmsg);
+ grub_error_pop();
+}
+
(The indentation looks wrong in Mozilla Thunderbird, but applying the
patch, everything is fine. The non-changed lines are indented by two
spaces too much.)
if (err)
grub_print_error ();
Acked-by: Paul Menzel
Kind regards,
Paul
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [SECURITY PATCH 116/117] templates: Disable the os-prober by default
Le 02/03/2021 à 19:02, Daniel Kiper a écrit :
From: Alex Burmashev
diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
index 1b91c102f..80685b15f 100644
--- a/util/grub.d/30_os-prober.in
+++ b/util/grub.d/30_os-prober.in
@@ -26,7 +26,8 @@ export TEXTDOMAINDIR="@localedir@"
. "$pkgdatadir/grub-mkconfig_lib"
-if [ "x${GRUB_DISABLE_OS_PROBER}" = "xtrue" ]; then
+if [ "x${GRUB_DISABLE_OS_PROBER}" = "xfalse" ]; then
+ gettext_printf "os-prober will not be executed to detect other bootable
partitions.\nSystems on them will not be added to the GRUB boot configuration.\nCheck
GRUB_DISABLE_OS_PROBER documentation entry.\n"
exit 0
fi
This is confusing: now to get boot entries from os-prober one have to
set:
GRUB_DISABLE_OS_PROBER=true
in /etc/default/grub.
Either revert that, or (better, in my opinion) label the variable
GRUB_ENABLE_OS_PROBER and set it to false by default.
Tested from grub pulled from git master with all patches committed.
Best regards,
Didier
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [SECURITY PATCH 000/117] Multiple GRUB2 vulnerabilities - 2021/03/02 round
Hi Adrian, On Tue, Mar 02, 2021 at 08:37:14PM +0100, John Paul Adrian Glaubitz wrote: > Hi Daniel! > > On 3/2/21 7:00 PM, Daniel Kiper wrote: > > The BootHole vulnerability [1][2] announced last year encouraged many > > people to > > take a closer look at the security of boot process in general and the GRUB > > bootloader in particular. Due to that, during past few months we were > > getting > > reports of, and also discovering various security flaws in the GRUB > > ourselves. > > You can find the list of most severe ones which got CVEs assigned at the > > end of > > this message. The patch bundle fixing all these issues in the upstream GRUB > > contains 117 patches. > > Huge thanks and kudos to everyone involved fixing all these vulnerabilities! > > Given the amount of patches, wouldn't it make sense to push an RC candidate > for 2.06 in the near future so that distributions can start shipping the pre- > release and avoiding to carry this large amount of patches? I am planning to cut 2.06-rc1 in matter of days... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [SECURITY PATCH 000/117] Multiple GRUB2 vulnerabilities - 2021/03/02 round
On 3/2/21 1:37 PM, John Paul Adrian Glaubitz wrote: Hi Daniel! On 3/2/21 7:00 PM, Daniel Kiper wrote: The BootHole vulnerability [1][2] announced last year encouraged many people to take a closer look at the security of boot process in general and the GRUB bootloader in particular. Due to that, during past few months we were getting reports of, and also discovering various security flaws in the GRUB ourselves. You can find the list of most severe ones which got CVEs assigned at the end of this message. The patch bundle fixing all these issues in the upstream GRUB contains 117 patches. Huge thanks and kudos to everyone involved fixing all these vulnerabilities! Given the amount of patches, wouldn't it make sense to push an RC candidate for 2.06 in the near future so that distributions can start shipping the pre- release and avoiding to carry this large amount of patches? It makes sense to not rely on distros to do the job of GRUB. It's time to get on a release schedule and stick to it. Very complex packages like gcc, glibc, and binutils are on a six month schedule. Why not GRUB? From https://ftp.gnu.org/gnu/grub/: grub-1.99.tar.gz2011-05-14 grub-2.00.tar.gz2012-06-27 grub-2.02.tar.gz2017-04-26 grub-2.04.tar.gz2019-07-05 grub-2.06.tar.gz202?-??-?? If you are waiting for perfection, you will never get there. -- Bruce ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [SECURITY PATCH 000/117] Multiple GRUB2 vulnerabilities - 2021/03/02 round
Hi Daniel! On 3/2/21 7:00 PM, Daniel Kiper wrote: > The BootHole vulnerability [1][2] announced last year encouraged many people > to > take a closer look at the security of boot process in general and the GRUB > bootloader in particular. Due to that, during past few months we were getting > reports of, and also discovering various security flaws in the GRUB ourselves. > You can find the list of most severe ones which got CVEs assigned at the end > of > this message. The patch bundle fixing all these issues in the upstream GRUB > contains 117 patches. Huge thanks and kudos to everyone involved fixing all these vulnerabilities! Given the amount of patches, wouldn't it make sense to push an RC candidate for 2.06 in the near future so that distributions can start shipping the pre- release and avoiding to carry this large amount of patches? Thanks, Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 106/117] util/mkimage: Reorder PE optional header fields set-up
From: Peter Jones This makes the PE32 and PE32+ header fields set-up easier to follow by setting them closer to the initialization of their related sections. Signed-off-by: Peter Jones Signed-off-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper --- util/mkimage.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/util/mkimage.c b/util/mkimage.c index a039039db..deaef5666 100644 --- a/util/mkimage.c +++ b/util/mkimage.c @@ -1332,16 +1332,12 @@ grub_install_generate_image (const char *dir, const char *prefix, sections = o64 + 1; } - PE_OHDR (o32, o64, code_size) = grub_host_to_target32 (layout.exec_size); - PE_OHDR (o32, o64, data_size) = grub_host_to_target32 (reloc_addr - layout.exec_size - header_size); + PE_OHDR (o32, o64, header_size) = grub_host_to_target32 (header_size); PE_OHDR (o32, o64, entry_addr) = grub_host_to_target32 (layout.start_address); - PE_OHDR (o32, o64, code_base) = grub_host_to_target32 (header_size); - PE_OHDR (o32, o64, image_base) = 0; + PE_OHDR (o32, o64, image_size) = grub_host_to_target32 (pe_size); PE_OHDR (o32, o64, section_alignment) = grub_host_to_target32 (image_target->section_align); PE_OHDR (o32, o64, file_alignment) = grub_host_to_target32 (GRUB_PE32_FILE_ALIGNMENT); - PE_OHDR (o32, o64, image_size) = grub_host_to_target32 (pe_size); - PE_OHDR (o32, o64, header_size) = grub_host_to_target32 (header_size); PE_OHDR (o32, o64, subsystem) = grub_host_to_target16 (GRUB_PE32_SUBSYSTEM_EFI_APPLICATION); /* Do these really matter? */ @@ -1351,10 +1347,10 @@ grub_install_generate_image (const char *dir, const char *prefix, PE_OHDR (o32, o64, heap_commit_size) = grub_host_to_target32 (0x1); PE_OHDR (o32, o64, num_data_directories) = grub_host_to_target32 (GRUB_PE32_NUM_DATA_DIRECTORIES); - PE_OHDR (o32, o64, base_relocation_table.rva) = grub_host_to_target32 (reloc_addr); - PE_OHDR (o32, o64, base_relocation_table.size) = grub_host_to_target32 (layout.reloc_size); /* The sections. */ + PE_OHDR (o32, o64, code_base) = grub_host_to_target32 (header_size); + PE_OHDR (o32, o64, code_size) = grub_host_to_target32 (layout.exec_size); text_section = sections; strcpy (text_section->name, ".text"); text_section->virtual_size = grub_host_to_target32 (layout.exec_size); @@ -1366,6 +1362,8 @@ grub_install_generate_image (const char *dir, const char *prefix, | GRUB_PE32_SCN_MEM_EXECUTE | GRUB_PE32_SCN_MEM_READ); + PE_OHDR (o32, o64, data_size) = grub_host_to_target32 (reloc_addr - layout.exec_size - header_size); + data_section = text_section + 1; strcpy (data_section->name, ".data"); data_section->virtual_size = grub_host_to_target32 (layout.kernel_size - layout.exec_size); @@ -1388,6 +1386,8 @@ grub_install_generate_image (const char *dir, const char *prefix, | GRUB_PE32_SCN_MEM_READ | GRUB_PE32_SCN_MEM_WRITE); + PE_OHDR (o32, o64, base_relocation_table.rva) = grub_host_to_target32 (reloc_addr); + PE_OHDR (o32, o64, base_relocation_table.size) = grub_host_to_target32 (layout.reloc_size); reloc_section = mods_section + 1; strcpy (reloc_section->name, ".reloc"); reloc_section->virtual_size = grub_host_to_target32 (layout.reloc_size); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 112/117] kern/misc: Split parse_printf_args() into format parsing and va_list handling
From: Thomas Frauendorfer | Miray Software
This patch is preparing for a follow up patch which will use
the format parsing part to compare the arguments in a printf()
format from an external source against a printf() format with
expected arguments.
Signed-off-by: Thomas Frauendorfer | Miray Software
Reviewed-by: Daniel Kiper
---
grub-core/kern/misc.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 430e72340..c58857ca2 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -645,8 +645,7 @@ grub_lltoa (char *str, int c, unsigned long long n)
}
static void
-parse_printf_args (const char *fmt0, struct printf_args *args,
- va_list args_in)
+parse_printf_arg_fmt (const char *fmt0, struct printf_args *args)
{
const char *fmt;
char c;
@@ -804,6 +803,14 @@ parse_printf_args (const char *fmt0, struct printf_args
*args,
break;
}
}
+}
+
+static void
+parse_printf_args (const char *fmt0, struct printf_args *args, va_list args_in)
+{
+ grub_size_t n;
+
+ parse_printf_arg_fmt (fmt0, args);
for (n = 0; n < args->count; n++)
switch (args->ptr[n].type)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 108/117] util/mkimage: Refactor section setup to use a helper
From: Peter Jones
Add a init_pe_section() helper function to setup PE sections. This makes
the code simpler and easier to read.
Signed-off-by: Peter Jones
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
util/mkimage.c | 141 +++--
1 file changed, 76 insertions(+), 65 deletions(-)
diff --git a/util/mkimage.c b/util/mkimage.c
index 853a52179..8b475a691 100644
--- a/util/mkimage.c
+++ b/util/mkimage.c
@@ -817,6 +817,38 @@ grub_install_get_image_targets_string (void)
}
/*
+ * The image_target parameter is used by the grub_host_to_target32() macro.
+ */
+static struct grub_pe32_section_table *
+init_pe_section(const struct grub_install_image_target_desc *image_target,
+ struct grub_pe32_section_table *section,
+ const char * const name,
+ grub_uint32_t *vma, grub_uint32_t vsz, grub_uint32_t valign,
+ grub_uint32_t *rda, grub_uint32_t rsz,
+ grub_uint32_t characteristics)
+{
+ size_t len = strlen (name);
+
+ if (len > sizeof (section->name))
+grub_util_error (_("section name %s length is bigger than %lu"),
+name, (unsigned long) sizeof (section->name));
+
+ memcpy (section->name, name, len);
+
+ section->virtual_address = grub_host_to_target32 (*vma);
+ section->virtual_size = grub_host_to_target32 (vsz);
+ (*vma) = ALIGN_UP (*vma + vsz, valign);
+
+ section->raw_data_offset = grub_host_to_target32 (*rda);
+ section->raw_data_size = grub_host_to_target32 (rsz);
+ (*rda) = ALIGN_UP (*rda + rsz, GRUB_PE32_FILE_ALIGNMENT);
+
+ section->characteristics = grub_host_to_target32 (characteristics);
+
+ return section + 1;
+}
+
+/*
* tmp_ is just here so the compiler knows we'll never derefernce a NULL.
* It should get fully optimized away.
*/
@@ -1257,17 +1289,13 @@ grub_install_generate_image (const char *dir, const
char *prefix,
break;
case IMAGE_EFI:
{
- void *pe_img;
- grub_uint8_t *header;
- void *sections;
+ char *pe_img, *header;
+ struct grub_pe32_section_table *section;
size_t scn_size;
- size_t pe_size;
+ grub_uint32_t vma, raw_data;
+ size_t pe_size, header_size;
struct grub_pe32_coff_header *c;
- struct grub_pe32_section_table *text_section, *data_section;
- struct grub_pe32_section_table *mods_section, *reloc_section;
static const grub_uint8_t stub[] = GRUB_PE32_MSDOS_STUB;
- int header_size;
- int reloc_addr;
struct grub_pe32_optional_header *o32 = NULL;
struct grub_pe64_optional_header *o64 = NULL;
@@ -1276,17 +1304,12 @@ grub_install_generate_image (const char *dir, const
char *prefix,
else
header_size = EFI64_HEADER_SIZE;
- reloc_addr = ALIGN_UP (header_size + core_size,
- GRUB_PE32_FILE_ALIGNMENT);
+ vma = raw_data = header_size;
+ pe_size = ALIGN_UP (header_size + core_size, GRUB_PE32_FILE_ALIGNMENT) +
+ ALIGN_UP (layout.reloc_size, GRUB_PE32_FILE_ALIGNMENT);
+ header = pe_img = xcalloc (1, pe_size);
- pe_size = ALIGN_UP (reloc_addr + layout.reloc_size,
- GRUB_PE32_FILE_ALIGNMENT);
- pe_img = xmalloc (reloc_addr + layout.reloc_size);
- memset (pe_img, 0, header_size);
- memcpy ((char *) pe_img + header_size, core_img, core_size);
- memset ((char *) pe_img + header_size + core_size, 0, reloc_addr -
(header_size + core_size));
- memcpy ((char *) pe_img + reloc_addr, layout.reloc_section,
layout.reloc_size);
- header = pe_img;
+ memcpy (pe_img + raw_data, core_img, core_size);
/* The magic. */
memcpy (header, stub, GRUB_PE32_MSDOS_STUB_SIZE);
@@ -1319,18 +1342,17 @@ grub_install_generate_image (const char *dir, const
char *prefix,
o32->magic = grub_host_to_target16 (GRUB_PE32_PE32_MAGIC);
o32->data_base = grub_host_to_target32 (header_size +
layout.exec_size);
- sections = o32 + 1;
+ section = (struct grub_pe32_section_table *)(o32 + 1);
}
else
{
c->optional_header_size = grub_host_to_target16 (sizeof (struct
grub_pe64_optional_header));
-
o64 = (struct grub_pe64_optional_header *)
(header + GRUB_PE32_MSDOS_STUB_SIZE +
GRUB_PE32_SIGNATURE_SIZE +
sizeof (struct grub_pe32_coff_header));
o64->magic = grub_host_to_target16 (GRUB_PE32_PE64_MAGIC);
- sections = o64 + 1;
+ section = (struct grub_pe32_section_table *)(o64 + 1);
}
PE_OHDR (o32, o64, header_size) = grub_host_to_target32 (header_size);
@@ -1350,58 +1372,47 @@ grub_install_generate_image (const char *dir, const
char *prefix,
PE_OHDR (o32, o64, num_data_directories) = grub_host_to_target32
(GRUB_PE32_NUM_DATA_DIRECTORIES);
/* The
[SECURITY PATCH 105/117] util/mkimage: Unify more of the PE32 and PE32+ header set-up
From: Peter Jones
There's quite a bit of code duplication in the code that sets the optional
header for PE32 and PE32+. The two are very similar with the exception of
a few fields that have type grub_uint64_t instead of grub_uint32_t.
Factor out the common code and add a PE_OHDR() macro that simplifies the
set-up and make the code more readable.
Signed-off-by: Peter Jones
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
util/mkimage.c | 111 ++---
1 file changed, 51 insertions(+), 60 deletions(-)
diff --git a/util/mkimage.c b/util/mkimage.c
index b94bfb781..a039039db 100644
--- a/util/mkimage.c
+++ b/util/mkimage.c
@@ -816,6 +816,21 @@ grub_install_get_image_targets_string (void)
return formats;
}
+/*
+ * tmp_ is just here so the compiler knows we'll never derefernce a NULL.
+ * It should get fully optimized away.
+ */
+#define PE_OHDR(o32, o64, field) (*( \
+{ \
+ __typeof__((o64)->field) tmp_; \
+ __typeof__((o64)->field) *ret_ = _; \
+ if (o32) \
+ret_ = (void *)(&((o32)->field)); \
+ else if (o64)\
+ret_ = (void *)(&((o64)->field)); \
+ ret_;\
+}))
+
void
grub_install_generate_image (const char *dir, const char *prefix,
FILE *out, const char *outname, char *mods[],
@@ -1252,6 +1267,8 @@ grub_install_generate_image (const char *dir, const char
*prefix,
static const grub_uint8_t stub[] = GRUB_PE32_MSDOS_STUB;
int header_size;
int reloc_addr;
+ struct grub_pe32_optional_header *o32 = NULL;
+ struct grub_pe64_optional_header *o64 = NULL;
if (image_target->voidp_sizeof == 4)
header_size = EFI32_HEADER_SIZE;
@@ -1293,76 +1310,50 @@ grub_install_generate_image (const char *dir, const
char *prefix,
/* The PE Optional header. */
if (image_target->voidp_sizeof == 4)
{
- struct grub_pe32_optional_header *o;
-
c->optional_header_size = grub_host_to_target16 (sizeof (struct
grub_pe32_optional_header));
- o = (struct grub_pe32_optional_header *)
- (header + GRUB_PE32_MSDOS_STUB_SIZE + GRUB_PE32_SIGNATURE_SIZE
- + sizeof (struct grub_pe32_coff_header));
- o->magic = grub_host_to_target16 (GRUB_PE32_PE32_MAGIC);
- o->code_size = grub_host_to_target32 (layout.exec_size);
- o->data_size = grub_host_to_target32 (reloc_addr - layout.exec_size
-- header_size);
- o->entry_addr = grub_host_to_target32 (layout.start_address);
- o->code_base = grub_host_to_target32 (header_size);
+ o32 = (struct grub_pe32_optional_header *)
+ (header + GRUB_PE32_MSDOS_STUB_SIZE + GRUB_PE32_SIGNATURE_SIZE +
+ sizeof (struct grub_pe32_coff_header));
+ o32->magic = grub_host_to_target16 (GRUB_PE32_PE32_MAGIC);
+ o32->data_base = grub_host_to_target32 (header_size +
layout.exec_size);
- o->data_base = grub_host_to_target32 (header_size +
layout.exec_size);
-
- o->image_base = 0;
- o->section_alignment = grub_host_to_target32
(image_target->section_align);
- o->file_alignment = grub_host_to_target32
(GRUB_PE32_FILE_ALIGNMENT);
- o->image_size = grub_host_to_target32 (pe_size);
- o->header_size = grub_host_to_target32 (header_size);
- o->subsystem = grub_host_to_target16
(GRUB_PE32_SUBSYSTEM_EFI_APPLICATION);
-
- /* Do these really matter? */
- o->stack_reserve_size = grub_host_to_target32 (0x1);
- o->stack_commit_size = grub_host_to_target32 (0x1);
- o->heap_reserve_size = grub_host_to_target32 (0x1);
- o->heap_commit_size = grub_host_to_target32 (0x1);
-
- o->num_data_directories = grub_host_to_target32
(GRUB_PE32_NUM_DATA_DIRECTORIES);
-
- o->base_relocation_table.rva = grub_host_to_target32 (reloc_addr);
- o->base_relocation_table.size = grub_host_to_target32
(layout.reloc_size);
- sections = o + 1;
+ sections = o32 + 1;
}
else
{
- struct grub_pe64_optional_header *o;
-
c->optional_header_size = grub_host_to_target16 (sizeof (struct
grub_pe64_optional_header));
- o = (struct grub_pe64_optional_header *)
- (header + GRUB_PE32_MSDOS_STUB_SIZE + GRUB_PE32_SIGNATURE_SIZE
- + sizeof (struct grub_pe32_coff_header));
- o->magic = grub_host_to_target16 (GRUB_PE32_PE64_MAGIC);
- o->code_size = grub_host_to_target32 (layout.exec_size);
- o->data_size = grub_host_to_target32 (reloc_addr - layout.exec_size
-
[SECURITY PATCH 104/117] util/mkimage: Always use grub_host_to_target32() to initialize PE stack and heap stuff
From: Peter Jones This change does not impact final result of initialization itself. However, it eases PE code unification in subsequent patches. Signed-off-by: Peter Jones Signed-off-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper --- util/mkimage.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/util/mkimage.c b/util/mkimage.c index 02944f28e..b94bfb781 100644 --- a/util/mkimage.c +++ b/util/mkimage.c @@ -1351,10 +1351,10 @@ grub_install_generate_image (const char *dir, const char *prefix, o->subsystem = grub_host_to_target16 (GRUB_PE32_SUBSYSTEM_EFI_APPLICATION); /* Do these really matter? */ - o->stack_reserve_size = grub_host_to_target64 (0x1); - o->stack_commit_size = grub_host_to_target64 (0x1); - o->heap_reserve_size = grub_host_to_target64 (0x1); - o->heap_commit_size = grub_host_to_target64 (0x1); + o->stack_reserve_size = grub_host_to_target32 (0x1); + o->stack_commit_size = grub_host_to_target32 (0x1); + o->heap_reserve_size = grub_host_to_target32 (0x1); + o->heap_commit_size = grub_host_to_target32 (0x1); o->num_data_directories = grub_host_to_target32 (GRUB_PE32_NUM_DATA_DIRECTORIES); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 098/117] kern/parser: Refactor grub_parser_split_cmdline() cleanup
From: Chris Coulson
Introduce a common function epilogue used for cleaning up on all
return paths, which will simplify additional error handling to be
introduced in a subsequent commit.
Signed-off-by: Chris Coulson
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 35 ---
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index 572c67089..e010eaa1f 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -221,19 +221,13 @@ grub_parser_split_cmdline (const char *cmdline,
if (process_char (*rp, buffer, , varname, , state, argc,
) != GRUB_ERR_NONE)
- {
- if (rd != cmdline)
- grub_free (rd);
- return grub_errno;
- }
+ goto fail;
+
state = newstate;
}
}
while (state != GRUB_PARSER_STATE_TEXT && !check_varstate (state));
- if (rd != cmdline)
-grub_free (rd);
-
/* A special case for when the last character was part of a
variable. */
add_var (varname, , , state, GRUB_PARSER_STATE_TEXT);
@@ -243,20 +237,20 @@ grub_parser_split_cmdline (const char *cmdline,
/* If there are no args, then we're done. */
if (!*argc)
-return 0;
+{
+ grub_errno = GRUB_ERR_NONE;
+ goto out;
+}
/* Reserve memory for the return values. */
args = grub_malloc (bp - buffer);
if (!args)
-return grub_errno;
+goto fail;
grub_memcpy (args, buffer, bp - buffer);
*argv = grub_calloc (*argc + 1, sizeof (char *));
if (!*argv)
-{
- grub_free (args);
- return grub_errno;
-}
+goto fail;
/* The arguments are separated with 0's, setup argv so it points to
the right values. */
@@ -269,7 +263,18 @@ grub_parser_split_cmdline (const char *cmdline,
bp++;
}
- return 0;
+ grub_errno = GRUB_ERR_NONE;
+
+ out:
+ if (rd != cmdline)
+grub_free (rd);
+
+ return grub_errno;
+
+ fail:
+ grub_free (*argv);
+ grub_free (args);
+ goto out;
}
/* Helper for grub_parser_execute. */
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 099/117] kern/buffer: Add variable sized heap buffer
From: Chris Coulson
Add a new variable sized heap buffer type (grub_buffer_t) with simple
operations for appending data, accessing the data and maintaining
a read cursor.
Signed-off-by: Chris Coulson
Reviewed-by: Daniel Kiper
---
grub-core/Makefile.core.def | 1 +
grub-core/kern/buffer.c | 117 +++
include/grub/buffer.h | 144
3 files changed, 262 insertions(+)
create mode 100644 grub-core/kern/buffer.c
create mode 100644 include/grub/buffer.h
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 1f450a98f..8022e1c0a 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -123,6 +123,7 @@ kernel = {
riscv32_efi_startup = kern/riscv/efi/startup.S;
riscv64_efi_startup = kern/riscv/efi/startup.S;
+ common = kern/buffer.c;
common = kern/command.c;
common = kern/corecmd.c;
common = kern/device.c;
diff --git a/grub-core/kern/buffer.c b/grub-core/kern/buffer.c
new file mode 100644
index 0..9f5f8b867
--- /dev/null
+++ b/grub-core/kern/buffer.c
@@ -0,0 +1,117 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2021 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see
[SECURITY PATCH 080/117] fs/nilfs2: Don't search children if provided number is too large
From: Daniel Axtens
NILFS2 reads the number of children a node has from the node. Unfortunately,
that's not trustworthy. Check if it's beyond what the filesystem permits and
reject it if so.
This blocks some OOB reads. I'm not sure how controllable the read is and what
could be done with invalidly read data later on.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/nilfs2.c | 38 +++---
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/grub-core/fs/nilfs2.c b/grub-core/fs/nilfs2.c
index 20053caf5..7963b4ef5 100644
--- a/grub-core/fs/nilfs2.c
+++ b/grub-core/fs/nilfs2.c
@@ -416,14 +416,34 @@ grub_nilfs2_btree_node_get_key (struct
grub_nilfs2_btree_node *node,
}
static inline int
-grub_nilfs2_btree_node_lookup (struct grub_nilfs2_btree_node *node,
+grub_nilfs2_btree_node_nchildren_max (struct grub_nilfs2_data *data,
+ struct grub_nilfs2_btree_node *node)
+{
+ int node_children_max = ((NILFS2_BLOCK_SIZE (data) -
+ sizeof (struct grub_nilfs2_btree_node) -
+ NILFS_BTREE_NODE_EXTRA_PAD_SIZE) /
+ (sizeof (grub_uint64_t) + sizeof (grub_uint64_t)));
+
+ return (node->bn_flags & NILFS_BTREE_NODE_ROOT) ? 3 : node_children_max;
+}
+
+static inline int
+grub_nilfs2_btree_node_lookup (struct grub_nilfs2_data *data,
+ struct grub_nilfs2_btree_node *node,
grub_uint64_t key, int *indexp)
{
grub_uint64_t nkey;
int index, low, high, s;
low = 0;
+
high = grub_le_to_cpu16 (node->bn_nchildren) - 1;
+ if (high >= grub_nilfs2_btree_node_nchildren_max (data, node))
+{
+ grub_error (GRUB_ERR_BAD_FS, "too many children");
+ return 0;
+}
+
index = 0;
s = 0;
while (low <= high)
@@ -459,18 +479,6 @@ grub_nilfs2_btree_node_lookup (struct
grub_nilfs2_btree_node *node,
return s == 0;
}
-static inline int
-grub_nilfs2_btree_node_nchildren_max (struct grub_nilfs2_data *data,
- struct grub_nilfs2_btree_node *node)
-{
- int node_children_max = ((NILFS2_BLOCK_SIZE (data) -
- sizeof (struct grub_nilfs2_btree_node) -
- NILFS_BTREE_NODE_EXTRA_PAD_SIZE) /
- (sizeof (grub_uint64_t) + sizeof (grub_uint64_t)));
-
- return (node->bn_flags & NILFS_BTREE_NODE_ROOT) ? 3 : node_children_max;
-}
-
static inline grub_uint64_t *
grub_nilfs2_btree_node_dptrs (struct grub_nilfs2_data *data,
struct grub_nilfs2_btree_node *node)
@@ -517,7 +525,7 @@ grub_nilfs2_btree_lookup (struct grub_nilfs2_data *data,
node = grub_nilfs2_btree_get_root (inode);
level = grub_nilfs2_btree_get_level (node);
- found = grub_nilfs2_btree_node_lookup (node, key, );
+ found = grub_nilfs2_btree_node_lookup (data, node, key, );
ptr = grub_nilfs2_btree_node_get_ptr (data, node, index);
if (need_translate)
ptr = grub_nilfs2_dat_translate (data, ptr);
@@ -538,7 +546,7 @@ grub_nilfs2_btree_lookup (struct grub_nilfs2_data *data,
}
if (!found)
- found = grub_nilfs2_btree_node_lookup (node, key, );
+ found = grub_nilfs2_btree_node_lookup (data, node, key, );
else
index = 0;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 091/117] disk/lvm: Sanitize rlocn->offset to prevent wild read
From: Daniel Axtens
rlocn->offset is read directly from disk and added to the metadatabuf
pointer to create a pointer to a block of metadata. It's a 64-bit
quantity so as long as you don't overflow you can set subsequent
pointers to point anywhere in memory.
Require that rlocn->offset fits within the metadata buffer size.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 8
1 file changed, 8 insertions(+)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 7efbaec79..4e17b8897 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -248,6 +248,14 @@ grub_lvm_detect (grub_disk_t disk,
}
rlocn = mdah->raw_locns;
+ if (grub_le_to_cpu64 (rlocn->offset) >= grub_le_to_cpu64 (mda_size))
+{
+#ifdef GRUB_UTIL
+ grub_util_info ("metadata offset is beyond end of metadata area");
+#endif
+ goto fail2;
+}
+
if (grub_le_to_cpu64 (rlocn->offset) + grub_le_to_cpu64 (rlocn->size) >
grub_le_to_cpu64 (mdah->size))
{
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 081/117] fs/nilfs2: Properly bail on errors in grub_nilfs2_btree_node_lookup()
From: Daniel Axtens
We just introduced an error return in grub_nilfs2_btree_node_lookup().
Make sure the callers catch it.
At the same time, make sure that grub_nilfs2_btree_node_lookup() always
inits the index pointer passed to it.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/nilfs2.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/grub-core/fs/nilfs2.c b/grub-core/fs/nilfs2.c
index 7963b4ef5..9b76982b3 100644
--- a/grub-core/fs/nilfs2.c
+++ b/grub-core/fs/nilfs2.c
@@ -433,7 +433,7 @@ grub_nilfs2_btree_node_lookup (struct grub_nilfs2_data
*data,
grub_uint64_t key, int *indexp)
{
grub_uint64_t nkey;
- int index, low, high, s;
+ int index = 0, low, high, s;
low = 0;
@@ -441,10 +441,10 @@ grub_nilfs2_btree_node_lookup (struct grub_nilfs2_data
*data,
if (high >= grub_nilfs2_btree_node_nchildren_max (data, node))
{
grub_error (GRUB_ERR_BAD_FS, "too many children");
+ *indexp = index;
return 0;
}
- index = 0;
s = 0;
while (low <= high)
{
@@ -526,6 +526,10 @@ grub_nilfs2_btree_lookup (struct grub_nilfs2_data *data,
level = grub_nilfs2_btree_get_level (node);
found = grub_nilfs2_btree_node_lookup (data, node, key, );
+
+ if (grub_errno != GRUB_ERR_NONE)
+goto fail;
+
ptr = grub_nilfs2_btree_node_get_ptr (data, node, index);
if (need_translate)
ptr = grub_nilfs2_dat_translate (data, ptr);
@@ -550,7 +554,8 @@ grub_nilfs2_btree_lookup (struct grub_nilfs2_data *data,
else
index = 0;
- if (index < grub_nilfs2_btree_node_nchildren_max (data, node))
+ if (index < grub_nilfs2_btree_node_nchildren_max (data, node) &&
+ grub_errno == GRUB_ERR_NONE)
{
ptr = grub_nilfs2_btree_node_get_ptr (data, node, index);
if (need_translate)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 110/117] grub-install-common: Add --sbat option
From: Dimitri John Ledkov
Signed-off-by: Dimitri John Ledkov
Reviewed-by: Daniel Kiper
---
include/grub/util/install.h | 5 -
util/grub-install-common.c | 12 ++--
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 3736a16aa..c14985858 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -63,6 +63,8 @@
/* TRANSLATORS: "embed" is a verb (command description). "*/ \
{ "pubkey", 'k', N_("FILE"), 0,\
N_("embed FILE as public key for signature checking"), 0}, \
+ { "sbat", GRUB_INSTALL_OPTIONS_SBAT, N_("FILE"), 0, \
+ N_("SBAT metadata"), 0 },
\
{ "verbose", 'v', 0, 0, \
N_("print verbose messages."), 1 }
@@ -122,7 +124,8 @@ enum grub_install_options {
GRUB_INSTALL_OPTIONS_THEMES_DIRECTORY,
GRUB_INSTALL_OPTIONS_GRUB_MKIMAGE,
GRUB_INSTALL_OPTIONS_INSTALL_CORE_COMPRESS,
- GRUB_INSTALL_OPTIONS_DTB
+ GRUB_INSTALL_OPTIONS_DTB,
+ GRUB_INSTALL_OPTIONS_SBAT
};
extern char *grub_install_source_directory;
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index 56dcb52bf..89af26c26 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -332,6 +332,7 @@ handle_install_list (struct install_list *il, const char
*val,
static char **pubkeys;
static size_t npubkeys;
+static char *sbat;
static grub_compression_t compression;
int
@@ -362,6 +363,12 @@ grub_install_parse (int key, char *arg)
* (npubkeys + 1));
pubkeys[npubkeys++] = xstrdup (arg);
return 1;
+case GRUB_INSTALL_OPTIONS_SBAT:
+ if (sbat)
+ free (sbat);
+
+ sbat = xstrdup (arg);
+ return 1;
case GRUB_INSTALL_OPTIONS_VERBOSITY:
verbosity++;
@@ -523,9 +530,10 @@ grub_install_make_image_wrap_file (const char *dir, const
char *prefix,
grub_util_info ("grub-mkimage --directory '%s' --prefix '%s'"
" --output '%s' "
" --dtb '%s' "
+ "--sbat '%s' "
"--format '%s' --compression '%s' %s %s\n",
dir, prefix,
- outname, dtb ? : "", mkimage_target,
+ outname, dtb ? : "", sbat ? : "", mkimage_target,
compnames[compression], note ? "--note" : "", s);
free (s);
@@ -536,7 +544,7 @@ grub_install_make_image_wrap_file (const char *dir, const
char *prefix,
grub_install_generate_image (dir, prefix, fp, outname,
modules.entries, memdisk_path,
pubkeys, npubkeys, config_path, tgt,
- note, compression, dtb, NULL);
+ note, compression, dtb, sbat);
while (dc--)
grub_install_pop_module ();
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 085/117] io/gzio: Zero gzio->tl/td in init_dynamic_block() if huft_build() fails
From: Daniel Axtens
If huft_build() fails, gzio->tl or gzio->td could contain pointers that
are no longer valid. Zero them out.
This prevents a double free when grub_gzio_close() comes through and
attempts to free them again.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/io/gzio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 19adebeed..aea86a0a9 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -1010,6 +1010,7 @@ init_dynamic_block (grub_gzio_t gzio)
gzio->bl = lbits;
if (huft_build (ll, nl, 257, cplens, cplext, >tl, >bl) != 0)
{
+ gzio->tl = 0;
grub_error (GRUB_ERR_BAD_COMPRESSED_DATA,
"failed in building a Huffman code table");
return;
@@ -1019,6 +1020,7 @@ init_dynamic_block (grub_gzio_t gzio)
{
huft_free (gzio->tl);
gzio->tl = 0;
+ gzio->td = 0;
grub_error (GRUB_ERR_BAD_COMPRESSED_DATA,
"failed in building a Huffman code table");
return;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 116/117] templates: Disable the os-prober by default
From: Alex Burmashev
The os-prober is enabled by default what may lead to potentially
dangerous use cases and borderline opening attack vectors. This
patch disables the os-prober, adds warning messages and updates
GRUB_DISABLE_OS_PROBER configuration option documentation. This
way we make it clear that the os-prober usage is not recommended.
Simplistic nature of this change allows downstream vendors, who
really want os-prober to be enabled out of the box in their
relevant products, easily revert to it's old behavior.
Reported-by: NyankoSec (, https://twitter.com/NyankoSec),
working with SSD Secure Disclosure
Signed-off-by: Alex Burmashev
Reviewed-by: Daniel Kiper
---
docs/grub.texi | 18 ++
util/grub.d/30_os-prober.in | 5 -
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index 2f2ae2228..cb5fe6e08 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -1519,10 +1519,13 @@ boot sequence. If you have problems, set this option
to @samp{text} and
GRUB will tell Linux to boot in normal text mode.
@item GRUB_DISABLE_OS_PROBER
-Normally, @command{grub-mkconfig} will try to use the external
-@command{os-prober} program, if installed, to discover other operating
-systems installed on the same system and generate appropriate menu entries
-for them. Set this option to @samp{true} to disable this.
+The @command{grub-mkconfig} has a feature to use the external
+@command{os-prober} program to discover other operating systems installed on
+the same machine and generate appropriate menu entries for them. It is disabled
+by default since automatic and silent execution of @command{os-prober}, and
+creating boot entries based on that data, is a potential attack vector. Set
+this option to @samp{false} to enable this feature in the
+@command{grub-mkconfig} command.
@item GRUB_OS_PROBER_SKIP_LIST
List of space-separated FS UUIDs of filesystems to be ignored from os-prober
@@ -1850,10 +1853,9 @@ than zero; otherwise 0.
@section Multi-boot manual config
Currently autogenerating config files for multi-boot environments depends on
-os-prober and has several shortcomings. While fixing it is scheduled for the
-next release, meanwhile you can make use of the power of GRUB syntax and do it
-yourself. A possible configuration is detailed here, feel free to adjust to
your
-needs.
+os-prober and has several shortcomings. Due to that it is disabled by default.
+It is advised to use the power of GRUB syntax and do it yourself. A possible
+configuration is detailed here, feel free to adjust to your needs.
First create a separate GRUB partition, big enough to hold GRUB. Some of the
following entries show how to load OS installer images from this same
partition,
diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
index 1b91c102f..80685b15f 100644
--- a/util/grub.d/30_os-prober.in
+++ b/util/grub.d/30_os-prober.in
@@ -26,7 +26,8 @@ export TEXTDOMAINDIR="@localedir@"
. "$pkgdatadir/grub-mkconfig_lib"
-if [ "x${GRUB_DISABLE_OS_PROBER}" = "xtrue" ]; then
+if [ "x${GRUB_DISABLE_OS_PROBER}" = "xfalse" ]; then
+ gettext_printf "os-prober will not be executed to detect other bootable
partitions.\nSystems on them will not be added to the GRUB boot
configuration.\nCheck GRUB_DISABLE_OS_PROBER documentation entry.\n"
exit 0
fi
@@ -39,6 +40,8 @@ OSPROBED="`os-prober | tr ' ' '^' | paste -s -d ' '`"
if [ -z "${OSPROBED}" ] ; then
# empty os-prober output, nothing doing
exit 0
+else
+ grub_warn "$(gettext_printf "os-prober was executed to detect other bootable
partitions.\nIt's output will be used to detect bootable binaries on them and
create new boot entries.")"
fi
osx_entry() {
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 077/117] fs/jfs: Limit the extents that getblk() can consider
From: Daniel Axtens
getblk() implicitly trusts that treehead->count is an accurate count of
the number of extents. However, that value is read from disk and is not
trustworthy, leading to OOB reads and crashes. I am not sure to what
extent the data read from OOB can influence subsequent program execution.
Require callers to pass in the maximum number of extents for which
they have storage.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/jfs.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/grub-core/fs/jfs.c b/grub-core/fs/jfs.c
index e5bbda61c..804c42d31 100644
--- a/grub-core/fs/jfs.c
+++ b/grub-core/fs/jfs.c
@@ -261,13 +261,15 @@ static grub_err_t grub_jfs_lookup_symlink (struct
grub_jfs_data *data, grub_uint
static grub_int64_t
getblk (struct grub_jfs_treehead *treehead,
struct grub_jfs_tree_extent *extents,
+ int max_extents,
struct grub_jfs_data *data,
grub_uint64_t blk)
{
int found = -1;
int i;
- for (i = 0; i < grub_le_to_cpu16 (treehead->count) - 2; i++)
+ for (i = 0; i < grub_le_to_cpu16 (treehead->count) - 2 &&
+ i < max_extents; i++)
{
if (treehead->flags & GRUB_JFS_TREE_LEAF)
{
@@ -302,7 +304,7 @@ getblk (struct grub_jfs_treehead *treehead,
<< (grub_le_to_cpu16 (data->sblock.log2_blksz)
- GRUB_DISK_SECTOR_BITS), 0,
sizeof (*tree), (char *) tree))
- ret = getblk (>treehead, >extents[0], data, blk);
+ ret = getblk (>treehead, >extents[0], 254, data, blk);
grub_free (tree);
return ret;
}
@@ -316,7 +318,7 @@ static grub_int64_t
grub_jfs_blkno (struct grub_jfs_data *data, struct grub_jfs_inode *inode,
grub_uint64_t blk)
{
- return getblk (>file.tree, >file.extents[0], data, blk);
+ return getblk (>file.tree, >file.extents[0], 16, data, blk);
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 111/117] shim_lock: Only skip loading shim_lock verifier with explicit consent
From: Dimitri John Ledkov
Commit 32ddc42c (efi: Only register shim_lock verifier if shim_lock
protocol is found and SB enabled) reintroduced CVE-2020-15705 which
previously only existed in the out-of-tree linuxefi patches and was
fixed as part of the BootHole patch series.
Under Secure Boot enforce loading shim_lock verifier. Allow skipping
shim_lock verifier if SecureBoot/MokSBState EFI variables indicate
skipping validations, or if GRUB image is built with --disable-shim-lock.
Fixes: 132ddc42c (efi: Only register shim_lock verifier if shim_lock
protocol is found and SB enabled)
Fixes: CVE-2020-15705
Fixes: CVE-2021-3418
Reported-by: Dimitri John Ledkov
Signed-off-by: Dimitri John Ledkov
Reviewed-by: Daniel Kiper
---
docs/grub.texi | 5 -
grub-core/kern/efi/sb.c | 17 -
include/grub/kernel.h | 3 ++-
include/grub/util/install.h | 7 +--
util/grub-install-common.c | 12 +---
util/grub-mkimage.c | 8 +++-
util/mkimage.c | 15 ++-
7 files changed, 57 insertions(+), 10 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index 81b90947a..2f2ae2228 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5950,7 +5950,10 @@ secure boot chain.
The GRUB, except the @command{chainloader} command, works with the UEFI secure
boot and the shim. This functionality is provided by the shim_lock verifier. It
is built into the @file{core.img} and is registered if the UEFI secure boot is
-enabled.
+enabled. The @samp{shim_lock} variable is set to @samp{y} when shim_lock
verifier
+is registered. If it is desired to use UEFI secure boot without shim, one can
+disable shim_lock by disabling shim verification with MokSbState UEFI variable
+or by building grub image with @samp{--disable-shim-lock} option.
All GRUB modules not stored in the @file{core.img}, OS kernels, ACPI tables,
Device Trees, etc. have to be signed, e.g, using PGP. Additionally, the
commands
diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
index 5d7210a82..41dadcd14 100644
--- a/grub-core/kern/efi/sb.c
+++ b/grub-core/kern/efi/sb.c
@@ -21,9 +21,11 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
#include
#include
#include
@@ -160,14 +162,27 @@ struct grub_file_verifier shim_lock_verifier =
void
grub_shim_lock_verifier_setup (void)
{
+ struct grub_module_header *header;
grub_efi_shim_lock_protocol_t *sl =
grub_efi_locate_protocol (_lock_guid, 0);
+ /* shim_lock is missing, check if GRUB image is built with
--disable-shim-lock. */
if (!sl)
-return;
+{
+ FOR_MODULES (header)
+ {
+ if (header->type == OBJ_TYPE_DISABLE_SHIM_LOCK)
+ return;
+ }
+}
+ /* Secure Boot is off. Do not load shim_lock. */
if (grub_efi_get_secureboot () != GRUB_EFI_SECUREBOOT_MODE_ENABLED)
return;
+ /* Enforce shim_lock_verifier. */
grub_verifier_register (_lock_verifier);
+
+ grub_env_set ("shim_lock", "y");
+ grub_env_export ("shim_lock");
}
diff --git a/include/grub/kernel.h b/include/grub/kernel.h
index 133a37c8d..abbca5ea3 100644
--- a/include/grub/kernel.h
+++ b/include/grub/kernel.h
@@ -29,7 +29,8 @@ enum
OBJ_TYPE_CONFIG,
OBJ_TYPE_PREFIX,
OBJ_TYPE_PUBKEY,
- OBJ_TYPE_DTB
+ OBJ_TYPE_DTB,
+ OBJ_TYPE_DISABLE_SHIM_LOCK
};
/* The module header. */
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index c14985858..b93c73caa 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -65,6 +65,8 @@
N_("embed FILE as public key for signature checking"), 0}, \
{ "sbat", GRUB_INSTALL_OPTIONS_SBAT, N_("FILE"), 0, \
N_("SBAT metadata"), 0 },
\
+ { "disable-shim-lock", GRUB_INSTALL_OPTIONS_DISABLE_SHIM_LOCK, 0, 0, \
+ N_("disable shim_lock verifier"), 0 }, \
{ "verbose", 'v', 0, 0, \
N_("print verbose messages."), 1 }
@@ -125,7 +127,8 @@ enum grub_install_options {
GRUB_INSTALL_OPTIONS_GRUB_MKIMAGE,
GRUB_INSTALL_OPTIONS_INSTALL_CORE_COMPRESS,
GRUB_INSTALL_OPTIONS_DTB,
- GRUB_INSTALL_OPTIONS_SBAT
+ GRUB_INSTALL_OPTIONS_SBAT,
+ GRUB_INSTALL_OPTIONS_DISABLE_SHIM_LOCK
};
extern char *grub_install_source_directory;
@@ -187,7 +190,7 @@ grub_install_generate_image (const char *dir, const char
*prefix,
const struct grub_install_image_target_desc
*image_target,
int note,
grub_compression_t comp, const char *dtb_file,
-const char *sbat_path);
+const char *sbat_path, const int
disable_shim_lock);
const struct grub_install_image_target_desc *
grub_install_get_image_target (const char *arg);
diff --git a/util/grub-install-common.c
[SECURITY PATCH 061/117] commands/ls: Require device_name is not NULL before printing
From: Daniel Axtens
This can be triggered with:
ls -l (0 0*)
and causes a NULL deref in grub_normal_print_device_info().
I'm not sure if there's any implication with the IEEE 1275 platform.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/commands/ls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/commands/ls.c b/grub-core/commands/ls.c
index 5b7491aa4..326d2d6b4 100644
--- a/grub-core/commands/ls.c
+++ b/grub-core/commands/ls.c
@@ -196,7 +196,7 @@ grub_ls_list_files (char *dirname, int longlist, int all,
int human)
goto fail;
}
- if (! *path)
+ if (! *path && device_name)
{
if (grub_errno == GRUB_ERR_UNKNOWN_FS)
grub_errno = GRUB_ERR_NONE;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 102/117] util/mkimage: Remove unused code to add BSS section
From: Javier Martinez Canillas The code is compiled out so there is no reason to keep it. Additionally, don't set bss_size field since we do not add a BSS section. Signed-off-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper --- util/mkimage.c | 17 - 1 file changed, 17 deletions(-) diff --git a/util/mkimage.c b/util/mkimage.c index 37d6249f1..32bb8ea68 100644 --- a/util/mkimage.c +++ b/util/mkimage.c @@ -1304,7 +1304,6 @@ grub_install_generate_image (const char *dir, const char *prefix, o->code_size = grub_host_to_target32 (layout.exec_size); o->data_size = grub_cpu_to_le32 (reloc_addr - layout.exec_size - header_size); - o->bss_size = grub_cpu_to_le32 (layout.bss_size); o->entry_addr = grub_cpu_to_le32 (layout.start_address); o->code_base = grub_cpu_to_le32 (header_size); @@ -1342,7 +1341,6 @@ grub_install_generate_image (const char *dir, const char *prefix, o->code_size = grub_host_to_target32 (layout.exec_size); o->data_size = grub_cpu_to_le32 (reloc_addr - layout.exec_size - header_size); - o->bss_size = grub_cpu_to_le32 (layout.bss_size); o->entry_addr = grub_cpu_to_le32 (layout.start_address); o->code_base = grub_cpu_to_le32 (header_size); o->image_base = 0; @@ -1387,21 +1385,6 @@ grub_install_generate_image (const char *dir, const char *prefix, = grub_cpu_to_le32_compile_time (GRUB_PE32_SCN_CNT_INITIALIZED_DATA | GRUB_PE32_SCN_MEM_READ | GRUB_PE32_SCN_MEM_WRITE); - -#if 0 - bss_section = data_section + 1; - strcpy (bss_section->name, ".bss"); - bss_section->virtual_size = grub_cpu_to_le32 (layout.bss_size); - bss_section->virtual_address = grub_cpu_to_le32 (header_size + layout.kernel_size); - bss_section->raw_data_size = 0; - bss_section->raw_data_offset = 0; - bss_section->characteristics - = grub_cpu_to_le32_compile_time (GRUB_PE32_SCN_MEM_READ - | GRUB_PE32_SCN_MEM_WRITE - | GRUB_PE32_SCN_ALIGN_64BYTES - | GRUB_PE32_SCN_CNT_INITIALIZED_DATA - | 0x80); -#endif mods_section = data_section + 1; strcpy (mods_section->name, "mods"); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 060/117] script/execute: Fix NULL dereference in grub_script_execute_cmdline()
From: Daniel Axtens
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/script/execute.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/script/execute.c b/grub-core/script/execute.c
index ce83edd4b..3ad468fce 100644
--- a/grub-core/script/execute.c
+++ b/grub-core/script/execute.c
@@ -940,7 +940,7 @@ grub_script_execute_cmdline (struct grub_script_cmd *cmd)
struct grub_script_argv argv = { 0, 0, 0 };
/* Lookup the command. */
- if (grub_script_arglist_to_argv (cmdline->arglist, ) || ! argv.args[0])
+ if (grub_script_arglist_to_argv (cmdline->arglist, ) || ! argv.args ||
! argv.args[0])
return grub_errno;
for (i = 0; i < argv.argc; i++)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 103/117] util/mkimage: Use grub_host_to_target32() instead of grub_cpu_to_le32()
From: Peter Jones The latter doesn't take into account the target image endianness. There is a grub_cpu_to_le32_compile_time() but no compile time variant for function grub_host_to_target32(). So, let's keep using the other one for this case. Signed-off-by: Peter Jones Signed-off-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper --- util/mkimage.c | 44 ++-- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/util/mkimage.c b/util/mkimage.c index 32bb8ea68..02944f28e 100644 --- a/util/mkimage.c +++ b/util/mkimage.c @@ -1302,10 +1302,10 @@ grub_install_generate_image (const char *dir, const char *prefix, + sizeof (struct grub_pe32_coff_header)); o->magic = grub_host_to_target16 (GRUB_PE32_PE32_MAGIC); o->code_size = grub_host_to_target32 (layout.exec_size); - o->data_size = grub_cpu_to_le32 (reloc_addr - layout.exec_size + o->data_size = grub_host_to_target32 (reloc_addr - layout.exec_size - header_size); - o->entry_addr = grub_cpu_to_le32 (layout.start_address); - o->code_base = grub_cpu_to_le32 (header_size); + o->entry_addr = grub_host_to_target32 (layout.start_address); + o->code_base = grub_host_to_target32 (header_size); o->data_base = grub_host_to_target32 (header_size + layout.exec_size); @@ -1339,10 +1339,10 @@ grub_install_generate_image (const char *dir, const char *prefix, + sizeof (struct grub_pe32_coff_header)); o->magic = grub_host_to_target16 (GRUB_PE32_PE64_MAGIC); o->code_size = grub_host_to_target32 (layout.exec_size); - o->data_size = grub_cpu_to_le32 (reloc_addr - layout.exec_size + o->data_size = grub_host_to_target32 (reloc_addr - layout.exec_size - header_size); - o->entry_addr = grub_cpu_to_le32 (layout.start_address); - o->code_base = grub_cpu_to_le32 (header_size); + o->entry_addr = grub_host_to_target32 (layout.start_address); + o->code_base = grub_host_to_target32 (header_size); o->image_base = 0; o->section_alignment = grub_host_to_target32 (image_target->section_align); o->file_alignment = grub_host_to_target32 (GRUB_PE32_FILE_ALIGNMENT); @@ -1366,10 +1366,10 @@ grub_install_generate_image (const char *dir, const char *prefix, /* The sections. */ text_section = sections; strcpy (text_section->name, ".text"); - text_section->virtual_size = grub_cpu_to_le32 (layout.exec_size); - text_section->virtual_address = grub_cpu_to_le32 (header_size); - text_section->raw_data_size = grub_cpu_to_le32 (layout.exec_size); - text_section->raw_data_offset = grub_cpu_to_le32 (header_size); + text_section->virtual_size = grub_host_to_target32 (layout.exec_size); + text_section->virtual_address = grub_host_to_target32 (header_size); + text_section->raw_data_size = grub_host_to_target32 (layout.exec_size); + text_section->raw_data_offset = grub_host_to_target32 (header_size); text_section->characteristics = grub_cpu_to_le32_compile_time ( GRUB_PE32_SCN_CNT_CODE | GRUB_PE32_SCN_MEM_EXECUTE @@ -1377,10 +1377,10 @@ grub_install_generate_image (const char *dir, const char *prefix, data_section = text_section + 1; strcpy (data_section->name, ".data"); - data_section->virtual_size = grub_cpu_to_le32 (layout.kernel_size - layout.exec_size); - data_section->virtual_address = grub_cpu_to_le32 (header_size + layout.exec_size); - data_section->raw_data_size = grub_cpu_to_le32 (layout.kernel_size - layout.exec_size); - data_section->raw_data_offset = grub_cpu_to_le32 (header_size + layout.exec_size); + data_section->virtual_size = grub_host_to_target32 (layout.kernel_size - layout.exec_size); + data_section->virtual_address = grub_host_to_target32 (header_size + layout.exec_size); + data_section->raw_data_size = grub_host_to_target32 (layout.kernel_size - layout.exec_size); + data_section->raw_data_offset = grub_host_to_target32 (header_size + layout.exec_size); data_section->characteristics = grub_cpu_to_le32_compile_time (GRUB_PE32_SCN_CNT_INITIALIZED_DATA | GRUB_PE32_SCN_MEM_READ @@ -1388,10 +1388,10 @@ grub_install_generate_image (const char *dir, const char *prefix, mods_section = data_section + 1; strcpy (mods_section->name, "mods"); - mods_section->virtual_size = grub_cpu_to_le32 (reloc_addr - layout.kernel_size - header_size); - mods_section->virtual_address = grub_cpu_to_le32 (header_size + layout.kernel_size + layout.bss_size); - mods_section->raw_data_size
[SECURITY PATCH 059/117] util/glue-efi: Fix incorrect use of a possibly negative value
From: Darren Kenny
It is possible for the ftell() function to return a negative value,
although it is fairly unlikely here, we should be checking for
a negative value before we assign it to an unsigned value.
Fixes: CID 73744
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
util/glue-efi.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/util/glue-efi.c b/util/glue-efi.c
index 68f53168b..de0fa6d33 100644
--- a/util/glue-efi.c
+++ b/util/glue-efi.c
@@ -39,13 +39,23 @@ write_fat (FILE *in32, FILE *in64, FILE *out, const char
*out_filename,
struct grub_macho_fat_header head;
struct grub_macho_fat_arch arch32, arch64;
grub_uint32_t size32, size64;
+ long size;
char *buf;
fseek (in32, 0, SEEK_END);
- size32 = ftell (in32);
+ size = ftell (in32);
+ if (size < 0)
+grub_util_error ("cannot get end of input file '%s': %s",
+name32, strerror (errno));
+ size32 = (grub_uint32_t) size;
fseek (in32, 0, SEEK_SET);
+
fseek (in64, 0, SEEK_END);
- size64 = ftell (in64);
+ size = ftell (in64);
+ if (size < 0)
+grub_util_error ("cannot get end of input file '%s': %s",
+name64, strerror (errno));
+ size64 = (grub_uint64_t) size;
fseek (in64, 0, SEEK_SET);
head.magic = grub_cpu_to_le32_compile_time (GRUB_MACHO_FAT_EFI_MAGIC);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 054/117] loader/xnu: Fix memory leak
From: Darren Kenny The code here is finished with the memory stored in name, but it only frees it if there curvalue is valid, while it could actually free it regardless. The fix is a simple relocation of the grub_free() to before the test of curvalue. Fixes: CID 96646 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/loader/xnu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grub-core/loader/xnu.c b/grub-core/loader/xnu.c index 44fd5a979..eb1446251 100644 --- a/grub-core/loader/xnu.c +++ b/grub-core/loader/xnu.c @@ -1391,9 +1391,9 @@ grub_xnu_fill_devicetree (void) name[len] = 0; curvalue = grub_xnu_create_value (curkey, name); -if (!curvalue) - return grub_errno; grub_free (name); +if (!curvalue) + return grub_errno; data = grub_malloc (grub_strlen (var->value) + 1); if (!data) -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 092/117] disk/lvm: Do not allow a LV to be it's own segment's node's LV
From: Daniel Axtens
This prevents infinite recursion in the diskfilter verification code.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 4e17b8897..8257159b3 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -981,9 +981,13 @@ grub_lvm_detect (grub_disk_t disk,
}
if (lv1->segments[i].nodes[j].pv == NULL)
for (lv2 = vg->lvs; lv2; lv2 = lv2->next)
- if (grub_strcmp (lv2->name,
-lv1->segments[i].nodes[j].name) == 0)
- lv1->segments[i].nodes[j].lv = lv2;
+ {
+ if (lv1 == lv2)
+ continue;
+ if (grub_strcmp (lv2->name,
+ lv1->segments[i].nodes[j].name) == 0)
+ lv1->segments[i].nodes[j].lv = lv2;
+ }
}
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 074/117] fs/hfs: Disable under lockdown
From: Daniel Axtens
HFS has issues such as infinite mutual recursion that are simply too
complex to fix for such a legacy format. So simply do not permit
it to be loaded under lockdown.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/hfs.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/grub-core/fs/hfs.c b/grub-core/fs/hfs.c
index 3fe842b4d..9a5b7bbe9 100644
--- a/grub-core/fs/hfs.c
+++ b/grub-core/fs/hfs.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
GRUB_MOD_LICENSE ("GPLv3+");
@@ -1433,11 +1434,13 @@ static struct grub_fs grub_hfs_fs =
GRUB_MOD_INIT(hfs)
{
- grub_fs_register (_hfs_fs);
+ if (!grub_is_lockdown ())
+grub_fs_register (_hfs_fs);
my_mod = mod;
}
GRUB_MOD_FINI(hfs)
{
- grub_fs_unregister (_hfs_fs);
+ if (!grub_is_lockdown())
+grub_fs_unregister (_hfs_fs);
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 050/117] video/fb/video_fb: Fix possible integer overflow
From: Darren Kenny
It is minimal possibility that the values being used here will overflow.
So, change the code to use the safemath function grub_mul() to ensure
that doesn't happen.
Fixes: CID 73761
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/video/fb/video_fb.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/grub-core/video/fb/video_fb.c b/grub-core/video/fb/video_fb.c
index 1c9a138dc..ae6b89f9a 100644
--- a/grub-core/video/fb/video_fb.c
+++ b/grub-core/video/fb/video_fb.c
@@ -1537,7 +1537,13 @@ doublebuf_pageflipping_init (struct grub_video_mode_info
*mode_info,
volatile void *page1_ptr)
{
grub_err_t err;
- grub_size_t page_size = mode_info->pitch * mode_info->height;
+ grub_size_t page_size = 0;
+
+ if (grub_mul (mode_info->pitch, mode_info->height, _size))
+{
+ /* Shouldn't happen, but if it does we've a bug. */
+ return GRUB_ERR_BUG;
+}
framebuffer.offscreen_buffer = grub_malloc (page_size);
if (! framebuffer.offscreen_buffer)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 046/117] commands/probe: Fix a resource leak when probing disks
From: Darren Kenny
Every other return statement in this code is calling grub_device_close()
to clean up dev before returning. This one should do that too.
Fixes: CID 292443
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/commands/probe.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
index 573bdf603..e53b61178 100644
--- a/grub-core/commands/probe.c
+++ b/grub-core/commands/probe.c
@@ -111,7 +111,11 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, char
**args)
grub_disk_t disk = grub_disk_open(dev->disk->name);
if (!disk)
- return grub_errno;
+ {
+ grub_device_close (dev);
+ return grub_errno;
+ }
+
if (grub_strcmp(dev->disk->partition->partmap->name, "gpt") == 0)
{
struct grub_gpt_partentry entry;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 093/117] fs/btrfs: Validate the number of stripes/parities in RAID5/6
From: Daniel Axtens This prevents a divide by zero if nstripes == nparities, and also prevents propagation of invalid values if nstripes ends up less than nparities. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/fs/btrfs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c index 27339bdb3..c4ba5f110 100644 --- a/grub-core/fs/btrfs.c +++ b/grub-core/fs/btrfs.c @@ -1083,6 +1083,9 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, grub_disk_addr_t addr, * stripen is computed without the parities * (0 for A0, A1, A2, 1 for B0, B1, B2, etc.). */ + if (nparities >= nstripes) + return grub_error (GRUB_ERR_BAD_FS, + "invalid RAID5/6: nparities >= nstripes"); high = grub_divmod64 (stripe_nr, nstripes - nparities, ); /* -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 042/117] libgcrypt/mpi: Fix possible NULL dereference
From: Darren Kenny The code in gcry_mpi_scan() assumes that buffer is not NULL, but there is no explicit check for that, so we add one. Fixes: CID 73757 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/lib/libgcrypt/mpi/mpicoder.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/grub-core/lib/libgcrypt/mpi/mpicoder.c b/grub-core/lib/libgcrypt/mpi/mpicoder.c index 7ecad27b2..6fe389165 100644 --- a/grub-core/lib/libgcrypt/mpi/mpicoder.c +++ b/grub-core/lib/libgcrypt/mpi/mpicoder.c @@ -379,6 +379,9 @@ gcry_mpi_scan (struct gcry_mpi **ret_mpi, enum gcry_mpi_format format, unsigned int len; int secure = (buffer && gcry_is_secure (buffer)); + if (!buffer) +return gcry_error (GPG_ERR_INV_ARG); + if (format == GCRYMPI_FMT_SSH) len = 0; else -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 088/117] disk/lvm: Bail on missing PV list
From: Daniel Axtens
There's an if block for the presence of "physical_volumes {", but if
that block is absent, then p remains NULL and a NULL-deref will result
when looking for logical volumes.
It doesn't seem like LVM makes sense without physical volumes, so error
out rather than crashing.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index b70d7d79f..31bbc9acc 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -409,6 +409,8 @@ grub_lvm_detect (grub_disk_t disk,
goto fail4;
}
}
+ else
+goto fail4;
p = grub_strstr (p, "logical_volumes {");
if (p)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 044/117] normal/completion: Fix leaking of memory when processing a completion
From: Darren Kenny
It is possible for the code to reach the end of the function without
freeing the memory allocated to argv and argc still to be 0.
We should always call grub_free(argv). The grub_free() will handle
a NULL argument correctly if it reaches that code without the memory
being allocated.
Fixes: CID 96672
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/normal/completion.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/grub-core/normal/completion.c b/grub-core/normal/completion.c
index c07100a8d..18cadfa85 100644
--- a/grub-core/normal/completion.c
+++ b/grub-core/normal/completion.c
@@ -401,8 +401,8 @@ char *
grub_normal_do_completion (char *buf, int *restore,
void (*hook) (const char *, grub_completion_type_t,
int))
{
- int argc;
- char **argv;
+ int argc = 0;
+ char **argv = NULL;
/* Initialize variables. */
match = 0;
@@ -517,10 +517,8 @@ grub_normal_do_completion (char *buf, int *restore,
fail:
if (argc != 0)
-{
- grub_free (argv[0]);
- grub_free (argv);
-}
+grub_free (argv[0]);
+ grub_free (argv);
grub_free (match);
grub_errno = GRUB_ERR_NONE;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 086/117] disk/lvm: Don't go beyond the end of the data we read from disk
From: Daniel Axtens
We unconditionally trusted offset_xl from the LVM label header, even if
it told us that the PV header/disk locations were way off past the end
of the data we read from disk.
Require that the offset be sane, fixing an OOB read and crash.
Fixes: CID 314367, CID 314371
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 753545146..6988fe492 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -178,6 +178,20 @@ grub_lvm_detect (grub_disk_t disk,
goto fail;
}
+ /*
+ * We read a grub_lvm_pv_header and then 2 grub_lvm_disk_locns that
+ * immediately follow the PV header. Make sure we have space for both.
+ */
+ if (grub_le_to_cpu32 (lh->offset_xl) >=
+ GRUB_LVM_LABEL_SIZE - sizeof (struct grub_lvm_pv_header) -
+ 2 * sizeof (struct grub_lvm_disk_locn))
+{
+#ifdef GRUB_UTIL
+ grub_util_info ("LVM PV header/disk locations are beyond the end of the
block");
+#endif
+ goto fail;
+}
+
pvh = (struct grub_lvm_pv_header *) (buf + grub_le_to_cpu32(lh->offset_xl));
for (i = 0, j = 0; i < GRUB_LVM_ID_LEN; i++)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 037/117] zfs: Fix resource leaks while constructing path
From: Paulo Flabiano Smorigo
There are several exit points in dnode_get_path() that are causing possible
memory leaks.
In the while(1) the correct exit mechanism should not be to do a direct return,
but to instead break out of the loop, setting err first if it is not already
set.
The reason behind this is that the dnode_path is a linked list, and while doing
through this loop, it is being allocated and built up - the only way to
correctly unravel it is to traverse it, which is what is being done at the end
of the function outside of the loop.
Several of the existing exit points correctly did a break, but not all so this
change makes that more consistent and should resolve the leaking of memory as
found by Coverity.
Fixes: CID 73741
Signed-off-by: Paulo Flabiano Smorigo
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/zfs/zfs.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 695e6ea30..ca82eb878 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -2839,8 +2839,8 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
if (dnode_path->dn.dn.dn_type != DMU_OT_DIRECTORY_CONTENTS)
{
- grub_free (path_buf);
- return grub_error (GRUB_ERR_BAD_FILE_TYPE, N_("not a directory"));
+ err = grub_error (GRUB_ERR_BAD_FILE_TYPE, N_("not a directory"));
+ break;
}
err = zap_lookup (&(dnode_path->dn), cname, ,
data, subvol->case_insensitive);
@@ -2882,11 +2882,18 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
<< SPA_MINBLOCKSHIFT);
if (blksz == 0)
- return grub_error(GRUB_ERR_BAD_FS, "0-sized block");
+{
+ err = grub_error (GRUB_ERR_BAD_FS, "0-sized block");
+ break;
+}
sym_value = grub_malloc (sym_sz);
if (!sym_value)
- return grub_errno;
+ {
+ err = grub_errno;
+ break;
+ }
+
for (block = 0; block < (sym_sz + blksz - 1) / blksz; block++)
{
void *t;
@@ -2896,7 +2903,7 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
if (err)
{
grub_free (sym_value);
- return err;
+ break;
}
movesize = sym_sz - block * blksz;
@@ -2906,6 +2913,8 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
grub_memcpy (sym_value + block * blksz, t, movesize);
grub_free (t);
}
+ if (err)
+ break;
free_symval = 1;
}
path = path_buf = grub_malloc (sym_sz + grub_strlen (oldpath) + 1);
@@ -2914,7 +2923,8 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
grub_free (oldpathbuf);
if (free_symval)
grub_free (sym_value);
- return grub_errno;
+ err = grub_errno;
+ break;
}
grub_memcpy (path, sym_value, sym_sz);
if (free_symval)
@@ -2952,11 +2962,12 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
err = zio_read (bp, dnode_path->dn.endian, , NULL, data);
if (err)
- return err;
+ break;
}
else
{
- return grub_error (GRUB_ERR_BAD_FS, "filesystem is corrupt");
+ err = grub_error (GRUB_ERR_BAD_FS, "filesystem is corrupt");
+ break;
}
hdrsize = SA_HDR_SIZE (((sa_hdr_phys_t *) sahdrp));
@@ -2977,7 +2988,8 @@ dnode_get_path (struct subvolume *subvol, const char
*path_in, dnode_end_t *dn,
if (!path_buf)
{
grub_free (oldpathbuf);
- return grub_errno;
+ err = grub_errno;
+ break;
}
grub_memcpy (path, sym_value, sym_sz);
path [sym_sz] = 0;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 084/117] io/gzio: Catch missing values in huft_build() and bail
From: Daniel Axtens
In huft_build(), "v" is a table of values in order of bit length.
The code later (when setting up table entries in "r") assumes that all
elements of this array corresponding to a code are initialized and less
than N_MAX. However, it doesn't enforce this.
With sufficiently manipulated inputs (e.g. from fuzzing), there can be
elements of "v" that are not filled. Therefore a lookup into "e" or "d"
will use an uninitialized value. This can lead to an invalid/OOB read on
those values, often leading to a crash.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/io/gzio.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 4236f0fd4..19adebeed 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -507,6 +507,7 @@ huft_build (unsigned *b,/* code lengths in bits (all
assumed <= BMAX) */
}
/* Make a table of values in order of bit lengths */
+ grub_memset (v, N_MAX, ARRAY_SIZE (v));
p = b;
i = 0;
do
@@ -588,11 +589,18 @@ huft_build (unsigned *b, /* code lengths in bits (all
assumed <= BMAX) */
r.v.n = (ush) (*p); /* simple code is just the value */
p++; /* one compiler does not like *p++ */
}
- else
+ else if (*p < N_MAX)
{
r.e = (uch) e[*p - s];/* non-simple--look up in lists */
r.v.n = d[*p++ - s];
}
+ else
+ {
+ /* Detected an uninitialised value, abort. */
+ if (h)
+ huft_free (u[0]);
+ return 2;
+ }
/* fill code-like entries with r */
f = 1 << (k - w);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 034/117] disk/cryptodisk: Fix potential integer overflow
From: Darren Kenny The encrypt and decrypt functions expect a grub_size_t. So, we need to ensure that the constant bit shift is using grub_size_t rather than unsigned int when it is performing the shift. Fixes: CID 307788 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/disk/cryptodisk.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c index b62835acc..41866c62d 100644 --- a/grub-core/disk/cryptodisk.c +++ b/grub-core/disk/cryptodisk.c @@ -325,10 +325,10 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev, case GRUB_CRYPTODISK_MODE_CBC: if (do_encrypt) err = grub_crypto_cbc_encrypt (dev->cipher, data + i, data + i, - (1U << log_sector_size), iv); + ((grub_size_t) 1 << log_sector_size), iv); else err = grub_crypto_cbc_decrypt (dev->cipher, data + i, data + i, - (1U << log_sector_size), iv); + ((grub_size_t) 1 << log_sector_size), iv); if (err) return err; break; @@ -336,10 +336,10 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev, case GRUB_CRYPTODISK_MODE_PCBC: if (do_encrypt) err = grub_crypto_pcbc_encrypt (dev->cipher, data + i, data + i, - (1U << log_sector_size), iv); + ((grub_size_t) 1 << log_sector_size), iv); else err = grub_crypto_pcbc_decrypt (dev->cipher, data + i, data + i, - (1U << log_sector_size), iv); + ((grub_size_t) 1 << log_sector_size), iv); if (err) return err; break; -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 078/117] fs/jfs: Catch infinite recursion
From: Daniel Axtens
It's possible with a fuzzed filesystem for JFS to keep getblk()-ing
the same data over and over again, leading to stack exhaustion.
Check if we'd be calling the function with exactly the same data as
was passed in, and if so abort.
I'm not sure what the performance impact of this is and am open to
better ideas.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/jfs.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/jfs.c b/grub-core/fs/jfs.c
index 804c42d31..6f7c43904 100644
--- a/grub-core/fs/jfs.c
+++ b/grub-core/fs/jfs.c
@@ -304,7 +304,16 @@ getblk (struct grub_jfs_treehead *treehead,
<< (grub_le_to_cpu16 (data->sblock.log2_blksz)
- GRUB_DISK_SECTOR_BITS), 0,
sizeof (*tree), (char *) tree))
- ret = getblk (>treehead, >extents[0], 254, data, blk);
+ {
+ if (grub_memcmp (>treehead, treehead, sizeof (struct
grub_jfs_treehead)) ||
+ grub_memcmp (>extents, extents, 254 * sizeof (struct
grub_jfs_tree_extent)))
+ ret = getblk (>treehead, >extents[0], 254, data, blk);
+ else
+ {
+ grub_error (GRUB_ERR_BAD_FS, "jfs: infinite recursion detected");
+ ret = -1;
+ }
+ }
grub_free (tree);
return ret;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 082/117] io/gzio: Bail if gzio->tl/td is NULL
From: Daniel Axtens
This is an ugly fix that doesn't address why gzio->tl comes to be NULL.
However, it seems to be sufficient to patch up a bunch of NULL derefs.
It would be good to revisit this in future and see if we can have
a cleaner solution that addresses some of the causes of the unexpected
NULL pointers.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/io/gzio.c | 20
1 file changed, 20 insertions(+)
diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 43d98a7bd..4a8eaeae2 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -669,6 +669,13 @@ inflate_codes_in_window (grub_gzio_t gzio)
{
if (! gzio->code_state)
{
+
+ if (gzio->tl == NULL)
+ {
+ grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "NULL gzio->tl");
+ return 1;
+ }
+
NEEDBITS ((unsigned) gzio->bl);
if ((e = (t = gzio->tl + ((unsigned) b & ml))->e) > 16)
do
@@ -707,6 +714,12 @@ inflate_codes_in_window (grub_gzio_t gzio)
n = t->v.n + ((unsigned) b & mask_bits[e]);
DUMPBITS (e);
+ if (gzio->td == NULL)
+ {
+ grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "NULL gzio->td");
+ return 1;
+ }
+
/* decode distance of block to copy */
NEEDBITS ((unsigned) gzio->bd);
if ((e = (t = gzio->td + ((unsigned) b & md))->e) > 16)
@@ -917,6 +930,13 @@ init_dynamic_block (grub_gzio_t gzio)
n = nl + nd;
m = mask_bits[gzio->bl];
i = l = 0;
+
+ if (gzio->tl == NULL)
+{
+ grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "NULL gzio->tl");
+ return;
+}
+
while ((unsigned) i < n)
{
NEEDBITS ((unsigned) gzio->bl);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 113/117] kern/misc: Add STRING type for internal printf() format handling
From: Thomas Frauendorfer | Miray Software
Set printf() argument type for "%s" to new type STRING. This is in
preparation for a follow up patch to compare a printf() format string
against an expected printf() format string.
For "%s" the corresponding printf() argument is dereferenced as pointer
while all other argument types are defined as integer value. However,
when validating a printf() format it is necessary to differentiate "%s"
from "%p" and other integers. So, let's do that.
Signed-off-by: Thomas Frauendorfer | Miray Software
Reviewed-by: Daniel Kiper
---
grub-core/kern/misc.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index c58857ca2..074728b2b 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -33,7 +33,8 @@ union printf_arg
enum
{
INT, LONG, LONGLONG,
- UNSIGNED_INT = 3, UNSIGNED_LONG, UNSIGNED_LONGLONG
+ UNSIGNED_INT = 3, UNSIGNED_LONG, UNSIGNED_LONGLONG,
+ STRING
} type;
long long ll;
};
@@ -791,12 +792,14 @@ parse_printf_arg_fmt (const char *fmt0, struct
printf_args *args)
args->ptr[curn].type = INT + longfmt;
break;
case 'p':
- case 's':
if (sizeof (void *) == sizeof (long long))
args->ptr[curn].type = UNSIGNED_LONGLONG;
else
args->ptr[curn].type = UNSIGNED_INT;
break;
+ case 's':
+ args->ptr[curn].type = STRING;
+ break;
case 'C':
case 'c':
args->ptr[curn].type = INT;
@@ -831,6 +834,12 @@ parse_printf_args (const char *fmt0, struct printf_args
*args, va_list args_in)
case UNSIGNED_LONGLONG:
args->ptr[n].ll = va_arg (args_in, long long);
break;
+ case STRING:
+ if (sizeof (void *) == sizeof (long long))
+ args->ptr[n].ll = va_arg (args_in, long long);
+ else
+ args->ptr[n].ll = va_arg (args_in, unsigned int);
+ break;
}
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 019/117] net/tftp: Fix dangling memory pointer
From: Darren Kenny The static code analysis tool, Parfait, reported that the valid of file->data was left referencing memory that was freed by the call to grub_free(data) where data was initialized from file->data. To ensure that there is no unintentional access to this memory referenced by file->data we should set the pointer to NULL. Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/net/tftp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c index 7e659a78b..1f3793ed8 100644 --- a/grub-core/net/tftp.c +++ b/grub-core/net/tftp.c @@ -442,6 +442,7 @@ tftp_close (struct grub_file *file) grub_net_udp_close (data->sock); } grub_free (data); + file->data = NULL; return GRUB_ERR_NONE; } -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 071/117] fs/fshelp: Catch impermissibly large block sizes in read helper
From: Daniel Axtens
A fuzzed HFS+ filesystem had log2blocksize = 22. This gave
log2blocksize + GRUB_DISK_SECTOR_BITS = 31. 1 << 31 = 0x8000,
which is -1 as an int. This caused some wacky behavior later on in
the function, leading to out-of-bounds writes on the destination buffer.
Catch log2blocksize + GRUB_DISK_SECTOR_BITS >= 31. We could be stricter,
but this is the minimum that will prevent integer size weirdness.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/fshelp.c | 12
1 file changed, 12 insertions(+)
diff --git a/grub-core/fs/fshelp.c b/grub-core/fs/fshelp.c
index 4c902adf3..a2d0d297a 100644
--- a/grub-core/fs/fshelp.c
+++ b/grub-core/fs/fshelp.c
@@ -362,6 +362,18 @@ grub_fshelp_read_file (grub_disk_t disk,
grub_fshelp_node_t node,
grub_disk_addr_t i, blockcnt;
int blocksize = 1 << (log2blocksize + GRUB_DISK_SECTOR_BITS);
+ /*
+ * Catch blatantly invalid log2blocksize. We could be a lot stricter, but
+ * this is the most permissive we can be before we start to see integer
+ * overflow/underflow issues.
+ */
+ if (log2blocksize + GRUB_DISK_SECTOR_BITS >= 31)
+{
+ grub_error (GRUB_ERR_OUT_OF_RANGE,
+ N_("blocksize too large"));
+ return -1;
+}
+
if (pos > filesize)
{
grub_error (GRUB_ERR_OUT_OF_RANGE,
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 117/117] kern/mm: Fix grub_debug_calloc() compilation error
From: Marco A Benatto
Fix compilation error due to missing parameter to
grub_printf() when MM_DEBUG is defined.
Fixes: 64e26162e (calloc: Make sure we always have an overflow-checking
calloc() available)
Signed-off-by: Marco A Benatto
Reviewed-by: Daniel Kiper
---
grub-core/kern/mm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index f2822a836..c070afc62 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -594,7 +594,7 @@ grub_debug_calloc (const char *file, int line, grub_size_t
nmemb, grub_size_t si
if (grub_mm_debug)
grub_printf ("%s:%d: calloc (0x%" PRIxGRUB_SIZE ", 0x%" PRIxGRUB_SIZE ") =
",
-file, line, size);
+file, line, nmemb, size);
ptr = grub_calloc (nmemb, size);
if (grub_mm_debug)
grub_printf ("%p\n", ptr);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 021/117] kern/efi: Fix memory leak on failure
From: Darren Kenny
Free the memory allocated to name before returning on failure.
Fixes: CID 296222
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/kern/efi/efi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 2942b8e35..67394bb6f 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -410,6 +410,7 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0)
{
grub_error (GRUB_ERR_OUT_OF_RANGE,
"malformed EFI Device Path node has length=%d", len);
+ grub_free (name);
return NULL;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 067/117] video/readers/jpeg: Catch files with unsupported quantization or Huffman tables
From: Daniel Axtens Our decoder only supports 2 quantization tables. If a file asks for a quantization table with index > 1, reject it. Similarly, our decoder only supports 4 Huffman tables. If a file asks for a Huffman table with index > 3, reject it. This fixes some out of bounds reads. It's not clear what degree of control over subsequent execution could be gained by someone who can carefully set up the contents of memory before loading an invalid JPEG file. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/video/readers/jpeg.c | 8 1 file changed, 8 insertions(+) diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c index 0b6ce3cee..23f919aa0 100644 --- a/grub-core/video/readers/jpeg.c +++ b/grub-core/video/readers/jpeg.c @@ -333,7 +333,11 @@ grub_jpeg_decode_sof (struct grub_jpeg_data *data) else if (ss != JPEG_SAMPLING_1x1) return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: sampling method not supported"); + data->comp_index[id][0] = grub_jpeg_get_byte (data); + if (data->comp_index[id][0] > 1) + return grub_error (GRUB_ERR_BAD_FILE_TYPE, + "jpeg: too many quantization tables"); } if (data->file->offset != next_marker) @@ -602,6 +606,10 @@ grub_jpeg_decode_sos (struct grub_jpeg_data *data) ht = grub_jpeg_get_byte (data); data->comp_index[id][1] = (ht >> 4); data->comp_index[id][2] = (ht & 0xF) + 2; + + if ((data->comp_index[id][1] < 0) || (data->comp_index[id][1] > 3) || + (data->comp_index[id][2] < 0) || (data->comp_index[id][2] > 3)) + return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid hufftable index"); } grub_jpeg_get_byte (data); /* Skip 3 unused bytes. */ -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 114/117] kern/misc: Add function to check printf() format against expected format
From: Thomas Frauendorfer | Miray Software
The grub_printf_fmt_check() function parses the arguments of an untrusted
printf() format and an expected printf() format and then compares the
arguments counts and arguments types. The arguments count in the untrusted
format string must be less or equal to the arguments count in the expected
format string and both arguments types must match.
To do this the parse_printf_arg_fmt() helper function is extended in the
following way:
1. Add a return value to report errors to the grub_printf_fmt_check().
2. Add the fmt_check argument to enable stricter format verification:
- the function expects that arguments definitions are always
terminated by a supported conversion specifier.
- positional parameters, "$", are not allowed, as they cannot be
validated correctly with the current implementation. For example
"%s%1$d" would assign the first args entry twice while leaving the
second one unchanged.
- Return an error if preallocated space in args is too small and
allocation fails for the needed size. The grub_printf_fmt_check()
should verify all arguments. So, if validation is not possible for
any reason it should return an error.
This also adds a case entry to handle "%%", which is the escape
sequence to print "%" character.
3. Add the max_args argument to check for the maximum allowed arguments
count in a printf() string. This should be set to the arguments count
of the expected format. Then the parse_printf_arg_fmt() function will
return an error if the arguments count is exceeded.
The two additional arguments allow us to use parse_printf_arg_fmt() in
printf() and grub_printf_fmt_check() calls.
When parse_printf_arg_fmt() is used by grub_printf_fmt_check() the
function parse user provided untrusted format string too. So, in
that case it is better to be too strict than too lenient.
Signed-off-by: Thomas Frauendorfer | Miray Software
Reviewed-by: Daniel Kiper
---
grub-core/kern/misc.c | 82 ---
include/grub/misc.h | 16 ++
2 files changed, 94 insertions(+), 4 deletions(-)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 074728b2b..3af336ee2 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -645,8 +645,26 @@ grub_lltoa (char *str, int c, unsigned long long n)
return p;
}
-static void
-parse_printf_arg_fmt (const char *fmt0, struct printf_args *args)
+/*
+ * Parse printf() fmt0 string into args arguments.
+ *
+ * The parsed arguments are either used by a printf() function to format the
fmt0
+ * string or they are used to compare a format string from an untrusted source
+ * against a format string with expected arguments.
+ *
+ * When the fmt_check is set to !0, e.g. 1, then this function is executed in
+ * printf() format check mode. This enforces stricter rules for parsing the
+ * fmt0 to limit exposure to possible errors in printf() handling. It also
+ * disables positional parameters, "$", because some formats, e.g "%s%1$d",
+ * cannot be validated with the current implementation.
+ *
+ * The max_args allows to set a maximum number of accepted arguments. If the
fmt0
+ * string defines more arguments than the max_args then the
parse_printf_arg_fmt()
+ * function returns an error. This is currently used for format check only.
+ */
+static grub_err_t
+parse_printf_arg_fmt (const char *fmt0, struct printf_args *args,
+ int fmt_check, grub_size_t max_args)
{
const char *fmt;
char c;
@@ -673,7 +691,12 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args
*args)
fmt++;
if (*fmt == '$')
- fmt++;
+ {
+ if (fmt_check)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT,
+ "positional arguments are not supported");
+ fmt++;
+ }
if (*fmt =='-')
fmt++;
@@ -705,9 +728,19 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args
*args)
case 's':
args->count++;
break;
+ case '%':
+ /* "%%" is the escape sequence to output "%". */
+ break;
+ default:
+ if (fmt_check)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT, "unexpected format");
+ break;
}
}
+ if (fmt_check && args->count > max_args)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, "too many arguments");
+
if (args->count <= ARRAY_SIZE (args->prealloc))
args->ptr = args->prealloc;
else
@@ -715,6 +748,9 @@ parse_printf_arg_fmt (const char *fmt0, struct printf_args
*args)
args->ptr = grub_calloc (args->count, sizeof (args->ptr[0]));
if (!args->ptr)
{
+ if (fmt_check)
+ return grub_errno;
+
grub_errno = GRUB_ERR_NONE;
args->ptr = args->prealloc;
args->count = ARRAY_SIZE (args->prealloc);
@@ -806,6 +842,8 @@
[SECURITY PATCH 014/117] docs: Document the cutmem command
From: Javier Martinez Canillas
The command is not present in the docs/grub.texi user documentation.
Reported-by: Daniel Kiper
Signed-off-by: Javier Martinez Canillas
Signed-off-by: Daniel Kiper
Reviewed-by: Javier Martinez Canillas
---
docs/grub.texi | 21 +
1 file changed, 21 insertions(+)
diff --git a/docs/grub.texi b/docs/grub.texi
index 91666781b..c5fc3904f 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3981,6 +3981,7 @@ you forget a command, you can run the command
@command{help}
* cpuid:: Check for CPU features
* crc:: Compute or check CRC32 checksums
* cryptomount:: Mount a crypto device
+* cutmem:: Remove memory regions
* date::Display or set current date and time
* devicetree:: Load a device tree blob
* distrust::Remove a pubkey from trusted keys
@@ -4139,6 +4140,8 @@ this page is to be filtered. This syntax makes it easy
to represent patterns
that are often result of memory damage, due to physical distribution of memory
cells.
+The command is similar to @command{cutmem} command.
+
Note: The command is not allowed when lockdown is enforced (@pxref{Lockdown}).
This prevents removing EFI memory regions to potentially subvert the
security mechanisms provided by the UEFI secure boot.
@@ -4303,6 +4306,24 @@ modules (@var{luks}, @var{luks2} and @var{geli}) have to
be loaded manually
before this command can be used.
@end deffn
+@node cutmem
+@subsection cutmem
+
+@deffn Command cutmem from[K|M|G] to[K|M|G]
+Remove any memory regions in specified range.
+@end deffn
+
+This command notifies the memory manager that specified regions of RAM ought to
+be filtered out. This remains in effect after a payload kernel has been loaded
+by GRUB, as long as the loaded kernel obtains its memory map from GRUB. Kernels
+that support this include Linux, GNU Mach, the kernel of FreeBSD and Multiboot
+kernels in general.
+
+The command is similar to @command{badram} command.
+
+Note: The command is not allowed when lockdown is enforced (@pxref{Lockdown}).
+ This prevents removing EFI memory regions to potentially subvert the
+ security mechanisms provided by the UEFI secure boot.
@node date
@subsection date
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 062/117] script/execute: Avoid crash when using "$#" outside a function scope
From: Daniel Axtens
"$#" represents the number of arguments to a function. It is only
defined in a function scope, where "scope" is non-NULL. Currently,
if we attempt to evaluate "$#" outside a function scope, "scope" will
be NULL and we will crash with a NULL pointer dereference.
Do not attempt to count arguments for "$#" if "scope" is NULL. This
will result in "$#" being interpreted as an empty string if evaluated
outside a function scope.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/script/execute.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/script/execute.c b/grub-core/script/execute.c
index 3ad468fce..b88765f1d 100644
--- a/grub-core/script/execute.c
+++ b/grub-core/script/execute.c
@@ -485,7 +485,7 @@ gettext_putvar (const char *str, grub_size_t len,
return 0;
/* Enough for any number. */
- if (len == 1 && str[0] == '#')
+ if (len == 1 && str[0] == '#' && scope != NULL)
{
grub_snprintf (*ptr, 30, "%u", scope->argv.argc);
*ptr += grub_strlen (*ptr);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 115/117] gfxmenu/gui: Check printf() format in the gui_progress_bar and gui_label
From: Thomas Frauendorfer | Miray Software
The gui_progress_bar and gui_label components can display the timeout
value. The format string can be set through a theme file. This patch
adds a validation step to the format string.
If a user loads a theme file into the GRUB without this patch then
a GUI label with the following settings
+ label {
...
id = "__timeout__"
text = "%s"
}
will interpret the current timeout value as string pointer and print the
memory at that position on the screen. It is not desired behavior.
Signed-off-by: Thomas Frauendorfer | Miray Software
Reviewed-by: Daniel Kiper
---
grub-core/gfxmenu/gui_label.c| 4
grub-core/gfxmenu/gui_progress_bar.c | 3 +++
2 files changed, 7 insertions(+)
diff --git a/grub-core/gfxmenu/gui_label.c b/grub-core/gfxmenu/gui_label.c
index a4c817891..1c190542a 100644
--- a/grub-core/gfxmenu/gui_label.c
+++ b/grub-core/gfxmenu/gui_label.c
@@ -193,6 +193,10 @@ label_set_property (void *vself, const char *name, const
char *value)
else if (grub_strcmp (value, "@KEYMAP_SHORT@") == 0)
value = _("enter: boot, `e': options, `c': cmd-line");
/* FIXME: Add more templates here if needed. */
+
+ if (grub_printf_fmt_check(value, "%d") != GRUB_ERR_NONE)
+ value = ""; /* Unsupported format. */
+
self->template = grub_strdup (value);
self->text = grub_xasprintf (value, self->value);
}
diff --git a/grub-core/gfxmenu/gui_progress_bar.c
b/grub-core/gfxmenu/gui_progress_bar.c
index b128f0866..ace85a125 100644
--- a/grub-core/gfxmenu/gui_progress_bar.c
+++ b/grub-core/gfxmenu/gui_progress_bar.c
@@ -348,6 +348,9 @@ progress_bar_set_property (void *vself, const char *name,
const char *value)
Please use the shortest form available in you language. */
value = _("%ds");
+ if (grub_printf_fmt_check(value, "%d") != GRUB_ERR_NONE)
+ value = ""; /* Unsupported format. */
+
self->template = grub_strdup (value);
}
else if (grub_strcmp (name, "font") == 0)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 012/117] gdb: Restrict GDB access when locked down
From: Javier Martinez Canillas
The gdbstub* commands allow to start and control a GDB stub running on
local host that can be used to connect from a remote debugger. Restrict
this functionality when the GRUB is locked down.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/gdb/gdb.c | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/grub-core/gdb/gdb.c b/grub-core/gdb/gdb.c
index 847a1e1e3..1818cb6f8 100644
--- a/grub-core/gdb/gdb.c
+++ b/grub-core/gdb/gdb.c
@@ -75,20 +75,24 @@ static grub_command_t cmd, cmd_stop, cmd_break;
GRUB_MOD_INIT (gdb)
{
grub_gdb_idtinit ();
- cmd = grub_register_command ("gdbstub", grub_cmd_gdbstub,
- N_("PORT"),
- /* TRANSLATORS: GDB stub is a small part of
- GDB functionality running on local host
- which allows remote debugger to
- connect to it. */
- N_("Start GDB stub on given port"));
- cmd_break = grub_register_command ("gdbstub_break", grub_cmd_gdb_break,
-/* TRANSLATORS: this refers to triggering
- a breakpoint so that the user will land
- into GDB. */
-0, N_("Break into GDB"));
- cmd_stop = grub_register_command ("gdbstub_stop", grub_cmd_gdbstop,
- 0, N_("Stop GDB stub"));
+ cmd = grub_register_command_lockdown ("gdbstub", grub_cmd_gdbstub,
+ N_("PORT"),
+ /*
+* TRANSLATORS: GDB stub is a small
part of
+* GDB functionality running on local
host
+* which allows remote debugger to
+* connect to it.
+*/
+ N_("Start GDB stub on given port"));
+ cmd_break = grub_register_command_lockdown ("gdbstub_break",
grub_cmd_gdb_break,
+ /*
+ * TRANSLATORS: this refers to
triggering
+ * a breakpoint so that the user
will land
+ * into GDB.
+ */
+ 0, N_("Break into GDB"));
+ cmd_stop = grub_register_command_lockdown ("gdbstub_stop", grub_cmd_gdbstop,
+0, N_("Stop GDB stub"));
}
GRUB_MOD_FINI (gdb)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 058/117] util/grub-editenv: Fix incorrect casting of a signed value
From: Darren Kenny
The return value of ftell() may be negative (-1) on error. While it is
probably unlikely to occur, we should not blindly cast to an unsigned
value without first testing that it is not negative.
Fixes: CID 73856
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
util/grub-editenv.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index f3662c95b..db6f187cc 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -125,6 +125,7 @@ open_envblk_file (const char *name)
{
FILE *fp;
char *buf;
+ long loc;
size_t size;
grub_envblk_t envblk;
@@ -143,7 +144,12 @@ open_envblk_file (const char *name)
grub_util_error (_("cannot seek `%s': %s"), name,
strerror (errno));
- size = (size_t) ftell (fp);
+ loc = ftell (fp);
+ if (loc < 0)
+grub_util_error (_("cannot get file location `%s': %s"), name,
+strerror (errno));
+
+ size = (size_t) loc;
if (fseek (fp, 0, SEEK_SET) < 0)
grub_util_error (_("cannot seek `%s': %s"), name,
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 107/117] util/mkimage: Improve data_size value calculation
From: Peter Jones According to "Microsoft Portable Executable and Common Object File Format Specification", the Optional Header SizeOfInitializedData field contains: Size of the initialized data section, or the sum of all such sections if there are multiple data sections. Make this explicit by adding the GRUB kernel data size to the sum of all the modules sizes. The ALIGN_UP() is not required by the PE spec but do it to avoid alignment issues. Signed-off-by: Peter Jones Signed-off-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper --- util/mkimage.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/util/mkimage.c b/util/mkimage.c index deaef5666..853a52179 100644 --- a/util/mkimage.c +++ b/util/mkimage.c @@ -1260,6 +1260,7 @@ grub_install_generate_image (const char *dir, const char *prefix, void *pe_img; grub_uint8_t *header; void *sections; + size_t scn_size; size_t pe_size; struct grub_pe32_coff_header *c; struct grub_pe32_section_table *text_section, *data_section; @@ -1362,7 +1363,10 @@ grub_install_generate_image (const char *dir, const char *prefix, | GRUB_PE32_SCN_MEM_EXECUTE | GRUB_PE32_SCN_MEM_READ); - PE_OHDR (o32, o64, data_size) = grub_host_to_target32 (reloc_addr - layout.exec_size - header_size); + scn_size = ALIGN_UP (layout.kernel_size - layout.exec_size, GRUB_PE32_FILE_ALIGNMENT); + PE_OHDR (o32, o64, data_size) = grub_host_to_target32 (scn_size + + ALIGN_UP (total_module_size, + GRUB_PE32_FILE_ALIGNMENT)); data_section = text_section + 1; strcpy (data_section->name, ".data"); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 010/117] commands/setpci: Restrict setpci command when locked down
From: Javier Martinez Canillas
This command can set PCI devices register values, which makes it dangerous
in a locked down configuration. Restrict it so can't be used on this setup.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/commands/setpci.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/grub-core/commands/setpci.c b/grub-core/commands/setpci.c
index e966af080..8a0c91f02 100644
--- a/grub-core/commands/setpci.c
+++ b/grub-core/commands/setpci.c
@@ -328,10 +328,10 @@ static grub_extcmd_t cmd;
GRUB_MOD_INIT(setpci)
{
- cmd = grub_register_extcmd ("setpci", grub_cmd_setpci, 0,
- N_("[-s POSITION] [-d DEVICE] [-v VAR] "
-"REGISTER[=VALUE[:MASK]]"),
- N_("Manipulate PCI devices."), options);
+ cmd = grub_register_extcmd_lockdown ("setpci", grub_cmd_setpci, 0,
+ N_("[-s POSITION] [-d DEVICE] [-v VAR] "
+ "REGISTER[=VALUE[:MASK]]"),
+ N_("Manipulate PCI devices."), options);
}
GRUB_MOD_FINI(setpci)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 057/117] util/grub-install: Fix NULL pointer dereferences
Two grub_device_open() calls does not have associated NULL checks
for returned values. Fix that and appease the Coverity.
Fixes: CID 314583
Signed-off-by: Daniel Kiper
Reviewed-by: Javier Martinez Canillas
---
util/grub-install.c | 4
1 file changed, 4 insertions(+)
diff --git a/util/grub-install.c b/util/grub-install.c
index 79fc4b937..a0babe3ef 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1784,6 +1784,8 @@ main (int argc, char *argv[])
fill_core_services (core_services);
ins_dev = grub_device_open (install_drive);
+ if (ins_dev == NULL)
+ grub_util_error ("%s", grub_errmsg);
bless (ins_dev, core_services, 0);
@@ -1884,6 +1886,8 @@ main (int argc, char *argv[])
fill_core_services(core_services);
ins_dev = grub_device_open (install_drive);
+ if (ins_dev == NULL)
+ grub_util_error ("%s", grub_errmsg);
bless (ins_dev, boot_efi, 1);
if (!removable && update_nvram)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 090/117] disk/lvm: Do not overread metadata
From: Daniel Axtens
We could reach the end of valid metadata and not realize, leading to
some buffer overreads. Check if we have reached the end and bail.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 35 +--
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 201097fda..7efbaec79 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -352,17 +352,23 @@ grub_lvm_detect (grub_disk_t disk,
while (1)
{
grub_ssize_t s;
- while (grub_isspace (*p))
+ while (grub_isspace (*p) && p < mda_end)
p++;
+ if (p == mda_end)
+ goto fail4;
+
if (*p == '}')
break;
pv = grub_zalloc (sizeof (*pv));
q = p;
- while (*q != ' ')
+ while (*q != ' ' && q < mda_end)
q++;
+ if (q == mda_end)
+ goto pvs_fail_noname;
+
s = q - p;
pv->name = grub_malloc (s + 1);
grub_memcpy (pv->name, p, s);
@@ -405,6 +411,7 @@ grub_lvm_detect (grub_disk_t disk,
continue;
pvs_fail:
grub_free (pv->name);
+ pvs_fail_noname:
grub_free (pv);
goto fail4;
}
@@ -426,18 +433,24 @@ grub_lvm_detect (grub_disk_t disk,
struct grub_diskfilter_segment *seg;
int is_pvmove;
- while (grub_isspace (*p))
+ while (grub_isspace (*p) && p < mda_end)
p++;
+ if (p == mda_end)
+ goto fail4;
+
if (*p == '}')
break;
lv = grub_zalloc (sizeof (*lv));
q = p;
- while (*q != ' ')
+ while (*q != ' ' && q < mda_end)
q++;
+ if (q == mda_end)
+ goto lvs_fail;
+
s = q - p;
lv->name = grub_strndup (p, s);
if (!lv->name)
@@ -609,9 +622,12 @@ grub_lvm_detect (grub_disk_t disk,
if (p == NULL)
goto lvs_segment_fail2;
q = ++p;
- while (*q != '"')
+ while (q < mda_end && *q != '"')
q++;
+ if (q == mda_end)
+ goto lvs_segment_fail2;
+
s = q - p;
stripe->name = grub_malloc (s + 1);
@@ -668,9 +684,12 @@ grub_lvm_detect (grub_disk_t disk,
if (p == NULL)
goto lvs_segment_fail2;
q = ++p;
- while (*q != '"')
+ while (q < mda_end && *q != '"')
q++;
+ if (q == mda_end)
+ goto lvs_segment_fail2;
+
s = q - p;
lvname = grub_malloc (s + 1);
@@ -826,6 +845,8 @@ grub_lvm_detect (grub_disk_t disk,
goto cache_lv_fail;
p3 = ++p2;
+ if (p3 == mda_end)
+ goto cache_lv_fail;
p3 = grub_strchr (p3, '"');
if (!p3)
goto cache_lv_fail;
@@ -847,6 +868,8 @@ grub_lvm_detect (grub_disk_t disk,
goto cache_lv_fail;
p3 = ++p2;
+ if (p3 == mda_end)
+ goto cache_lv_fail;
p3 = grub_strchr (p3, '"');
if (!p3)
goto cache_lv_fail;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 003/117] kern: Add lockdown support
From: Javier Martinez Canillas
When the GRUB starts on a secure boot platform, some commands can be
used to subvert the protections provided by the verification mechanism and
could lead to booting untrusted system.
To prevent that situation, allow GRUB to be locked down. That way the code
may check if GRUB has been locked down and further restrict the commands
that are registered or what subset of their functionality could be used.
The lockdown support adds the following components:
* The grub_lockdown() function which can be used to lockdown GRUB if,
e.g., UEFI Secure Boot is enabled.
* The grub_is_lockdown() function which can be used to check if the GRUB
was locked down.
* A verifier that flags OS kernels, the GRUB modules, Device Trees and ACPI
tables as GRUB_VERIFY_FLAGS_DEFER_AUTH to defer verification to other
verifiers. These files are only successfully verified if another registered
verifier returns success. Otherwise, the whole verification process fails.
For example, PE/COFF binaries verification can be done by the shim_lock
verifier which validates the signatures using the shim_lock protocol.
However, the verification is not deferred directly to the shim_lock verifier.
The shim_lock verifier is hooked into the verification process instead.
* A set of grub_{command,extcmd}_lockdown functions that can be used by
code registering command handlers, to only register unsafe commands if
the GRUB has not been locked down.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
conf/Makefile.common| 2 ++
docs/grub-dev.texi | 27 +++
docs/grub.texi | 8 +
grub-core/Makefile.am | 5 ++-
grub-core/Makefile.core.def | 1 +
grub-core/commands/extcmd.c | 23 +
grub-core/kern/command.c| 24 ++
grub-core/kern/lockdown.c | 80 +
include/grub/command.h | 5 +++
include/grub/extcmd.h | 7
include/grub/lockdown.h | 44 +
11 files changed, 225 insertions(+), 1 deletion(-)
create mode 100644 grub-core/kern/lockdown.c
create mode 100644 include/grub/lockdown.h
diff --git a/conf/Makefile.common b/conf/Makefile.common
index 6cd71cbb2..2a1a886f6 100644
--- a/conf/Makefile.common
+++ b/conf/Makefile.common
@@ -84,7 +84,9 @@ CPPFLAGS_PARTTOOL_LIST =
-Dgrub_parttool_register=PARTTOOL_LIST_MARKER
CPPFLAGS_TERMINAL_LIST =
'-Dgrub_term_register_input(...)=INPUT_TERMINAL_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_TERMINAL_LIST +=
'-Dgrub_term_register_output(...)=OUTPUT_TERMINAL_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_COMMAND_LIST =
'-Dgrub_register_command(...)=COMMAND_LIST_MARKER(__VA_ARGS__)'
+CPPFLAGS_COMMAND_LIST +=
'-Dgrub_register_command_lockdown(...)=COMMAND_LOCKDOWN_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_COMMAND_LIST +=
'-Dgrub_register_extcmd(...)=EXTCOMMAND_LIST_MARKER(__VA_ARGS__)'
+CPPFLAGS_COMMAND_LIST +=
'-Dgrub_register_extcmd_lockdown(...)=EXTCOMMAND_LOCKDOWN_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_COMMAND_LIST +=
'-Dgrub_register_command_p1(...)=P1COMMAND_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_FDT_LIST :=
'-Dgrub_fdtbus_register(...)=FDT_DRIVER_LIST_MARKER(__VA_ARGS__)'
CPPFLAGS_MARKER = $(CPPFLAGS_FS_LIST) $(CPPFLAGS_VIDEO_LIST) \
diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
index 24d17b8ec..a834b3a9c 100644
--- a/docs/grub-dev.texi
+++ b/docs/grub-dev.texi
@@ -86,6 +86,7 @@ This edition documents version @value{VERSION}.
* PFF2 Font File Format::
* Graphical Menu Software Design::
* Verifiers framework::
+* Lockdown framework::
* Copying This Manual:: Copying This Manual
* Index::
@end menu
@@ -2123,6 +2124,32 @@ Optionally at the end of the file @samp{fini}, if it
exists, is called with just
the context. If you return no error during any of @samp{init}, @samp{write} and
@samp{fini} then the file is considered as having succeded verification.
+@node Lockdown framework
+@chapter Lockdown framework
+
+The GRUB can be locked down, which is a restricted mode where some operations
+are not allowed. For instance, some commands cannot be used when the GRUB is
+locked down.
+
+The function
+@code{grub_lockdown()} is used to lockdown GRUB and the function
+@code{grub_is_lockdown()} function can be used to check whether lockdown is
+enabled or not. When enabled, the function returns @samp{GRUB_LOCKDOWN_ENABLED}
+and @samp{GRUB_LOCKDOWN_DISABLED} when is not enabled.
+
+The following functions can be used to register the commands that can only be
+used when lockdown is disabled:
+
+@itemize
+
+@item @code{grub_cmd_lockdown()} registers command which should not run when
the
+GRUB is in lockdown mode.
+
+@item @code{grub_cmd_lockdown()} registers extended command which should not
run
+when the GRUB is in lockdown mode.
+
+@end itemize
+
@node Copying This Manual
@appendix Copying This Manual
diff --git a/docs/grub.texi b/docs/grub.texi
index e3de2a4dc..fd3c78054 100644
[SECURITY PATCH 056/117] loader/xnu: Check if pointer is NULL before using it
From: Paulo Flabiano Smorigo
Fixes: CID 73654
Signed-off-by: Paulo Flabiano Smorigo
Reviewed-by: Daniel Kiper
---
grub-core/loader/xnu.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/grub-core/loader/xnu.c b/grub-core/loader/xnu.c
index 1a590dbc0..1c0cf6a43 100644
--- a/grub-core/loader/xnu.c
+++ b/grub-core/loader/xnu.c
@@ -670,6 +670,9 @@ grub_xnu_load_driver (char *infoplistname, grub_file_t
binaryfile,
char *name, *nameend;
int namelen;
+ if (infoplistname == NULL)
+return grub_error (GRUB_ERR_BAD_FILENAME, N_("missing p-list filename"));
+
name = get_name_ptr (infoplistname);
nameend = grub_strchr (name, '/');
@@ -701,10 +704,7 @@ grub_xnu_load_driver (char *infoplistname, grub_file_t
binaryfile,
else
macho = 0;
- if (infoplistname)
-infoplist = grub_file_open (infoplistname, GRUB_FILE_TYPE_XNU_INFO_PLIST);
- else
-infoplist = 0;
+ infoplist = grub_file_open (infoplistname, GRUB_FILE_TYPE_XNU_INFO_PLIST);
grub_errno = GRUB_ERR_NONE;
if (infoplist)
{
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 097/117] kern/parser: Introduce terminate_arg() helper
From: Chris Coulson
process_char() and grub_parser_split_cmdline() use similar code for
terminating the most recent argument. Add a helper function for this.
Signed-off-by: Chris Coulson
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index 0d3582bd8..572c67089 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -129,6 +129,16 @@ add_var (char *varname, char **bp, char **vp,
*((*bp)++) = *val;
}
+static void
+terminate_arg (char *buffer, char **bp, int *argc)
+{
+ if (*bp != buffer && *((*bp) - 1) != '\0')
+{
+ *((*bp)++) = '\0';
+ (*argc)++;
+}
+}
+
static grub_err_t
process_char (char c, char *buffer, char **bp, char *varname, char **vp,
grub_parser_state_t state, int *argc,
@@ -157,11 +167,7 @@ process_char (char c, char *buffer, char **bp, char
*varname, char **vp,
* Don't add more than one argument if multiple
* spaces are used.
*/
- if (*bp != buffer && *((*bp) - 1) != '\0')
- {
- *((*bp)++) = '\0';
- (*argc)++;
- }
+ terminate_arg (buffer, bp, argc);
}
else if (use)
*((*bp)++) = use;
@@ -232,11 +238,8 @@ grub_parser_split_cmdline (const char *cmdline,
variable. */
add_var (varname, , , state, GRUB_PARSER_STATE_TEXT);
- if (bp != buffer && *(bp - 1))
-{
- *(bp++) = '\0';
- (*argc)++;
-}
+ /* Ensure that the last argument is terminated. */
+ terminate_arg (buffer, , argc);
/* If there are no args, then we're done. */
if (!*argc)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 004/117] kern/lockdown: Set a variable if the GRUB is locked down
From: Javier Martinez Canillas
It may be useful for scripts to determine whether the GRUB is locked
down or not. Add the lockdown variable which is set to "y" when the GRUB
is locked down.
Suggested-by: Dimitri John Ledkov
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi| 3 +++
grub-core/kern/lockdown.c | 4
2 files changed, 7 insertions(+)
diff --git a/docs/grub.texi b/docs/grub.texi
index fd3c78054..4c980d356 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5966,6 +5966,9 @@ The GRUB can be locked down when booted on a secure boot
environment, for exampl
if the UEFI secure boot is enabled. On a locked down configuration, the GRUB
will
be restricted and some operations/commands cannot be executed.
+The @samp{lockdown} variable is set to @samp{y} when the GRUB is locked down.
+Otherwise it does not exit.
+
@node Platform limitations
@chapter Platform limitations
diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c
index 1e56c0b80..0bc70fd42 100644
--- a/grub-core/kern/lockdown.c
+++ b/grub-core/kern/lockdown.c
@@ -18,6 +18,7 @@
*/
#include
+#include
#include
#include
#include
@@ -71,6 +72,9 @@ grub_lockdown (void)
lockdown = GRUB_LOCKDOWN_ENABLED;
grub_verifier_register (_verifier);
+
+ grub_env_set ("lockdown", "y");
+ grub_env_export ("lockdown");
}
int
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 053/117] loader/bsd: Check for NULL arg up-front
From: Darren Kenny
The code in the next block suggests that it is possible for .set to be
true but .arg may still be NULL.
This code assumes that it is never NULL, yet later is testing if it is
NULL - that is inconsistent.
So we should check first if .arg is not NULL, and remove this check that
is being flagged by Coverity since it is no longer required.
Fixes: CID 292471
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/loader/i386/bsd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
index f5bf7f89e..d89ff0a7a 100644
--- a/grub-core/loader/i386/bsd.c
+++ b/grub-core/loader/i386/bsd.c
@@ -1605,7 +1605,7 @@ grub_cmd_openbsd (grub_extcmd_context_t ctxt, int argc,
char *argv[])
kernel_type = KERNEL_TYPE_OPENBSD;
bootflags = grub_bsd_parse_flags (ctxt->state, openbsd_flags);
- if (ctxt->state[OPENBSD_ROOT_ARG].set)
+ if (ctxt->state[OPENBSD_ROOT_ARG].set && ctxt->state[OPENBSD_ROOT_ARG].arg
!= NULL)
{
const char *arg = ctxt->state[OPENBSD_ROOT_ARG].arg;
unsigned type, unit, part;
@@ -1622,7 +1622,7 @@ grub_cmd_openbsd (grub_extcmd_context_t ctxt, int argc,
char *argv[])
"unknown disk type name");
unit = grub_strtoul (arg, , 10);
- if (! (arg && *arg >= 'a' && *arg <= 'z'))
+ if (! (*arg >= 'a' && *arg <= 'z'))
return grub_error (GRUB_ERR_BAD_ARGUMENT,
"only device specifications of form "
" are supported");
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 096/117] kern/parser: Introduce process_char() helper
From: Chris Coulson
grub_parser_split_cmdline() iterates over each command line character.
In order to add error checking and to simplify the subsequent error
handling, split the character processing in to a separate function.
Signed-off-by: Chris Coulson
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 74 ++---
1 file changed, 46 insertions(+), 28 deletions(-)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index 39e4df65b..0d3582bd8 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -1,7 +1,7 @@
/* parser.c - the part of the parser that can return partial tokens */
/*
* GRUB -- GRand Unified Bootloader
- * Copyright (C) 2005,2007,2009 Free Software Foundation, Inc.
+ * Copyright (C) 2005,2007,2009,2021 Free Software Foundation, Inc.
*
* GRUB is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -129,6 +129,46 @@ add_var (char *varname, char **bp, char **vp,
*((*bp)++) = *val;
}
+static grub_err_t
+process_char (char c, char *buffer, char **bp, char *varname, char **vp,
+ grub_parser_state_t state, int *argc,
+ grub_parser_state_t *newstate)
+{
+ char use;
+
+ *newstate = grub_parser_cmdline_state (state, c, );
+
+ /*
+ * If a variable was being processed and this character does
+ * not describe the variable anymore, write the variable to
+ * the buffer.
+ */
+ add_var (varname, bp, vp, state, *newstate);
+
+ if (check_varstate (*newstate))
+{
+ if (use)
+ *((*vp)++) = use;
+}
+ else if (*newstate == GRUB_PARSER_STATE_TEXT &&
+ state != GRUB_PARSER_STATE_ESC && grub_isspace (use))
+{
+ /*
+ * Don't add more than one argument if multiple
+ * spaces are used.
+ */
+ if (*bp != buffer && *((*bp) - 1) != '\0')
+ {
+ *((*bp)++) = '\0';
+ (*argc)++;
+ }
+}
+ else if (use)
+*((*bp)++) = use;
+
+ return GRUB_ERR_NONE;
+}
+
grub_err_t
grub_parser_split_cmdline (const char *cmdline,
grub_reader_getline_t getline, void *getline_data,
@@ -172,35 +212,13 @@ grub_parser_split_cmdline (const char *cmdline,
for (; *rp != '\0'; rp++)
{
grub_parser_state_t newstate;
- char use;
- newstate = grub_parser_cmdline_state (state, *rp, );
-
- /* If a variable was being processed and this character does
-not describe the variable anymore, write the variable to
-the buffer. */
- add_var (varname, , , state, newstate);
-
- if (check_varstate (newstate))
- {
- if (use)
- *(vp++) = use;
- }
- else
+ if (process_char (*rp, buffer, , varname, , state, argc,
+ ) != GRUB_ERR_NONE)
{
- if (newstate == GRUB_PARSER_STATE_TEXT
- && state != GRUB_PARSER_STATE_ESC && grub_isspace (use))
- {
- /* Don't add more than one argument if multiple
-spaces are used. */
- if (bp != buffer && *(bp - 1))
- {
- *(bp++) = '\0';
- (*argc)++;
- }
- }
- else if (use)
- *(bp++) = use;
+ if (rd != cmdline)
+ grub_free (rd);
+ return grub_errno;
}
state = newstate;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 109/117] util/mkimage: Add an option to import SBAT metadata into a .sbat section
From: Peter Jones
Add a --sbat option to the grub-mkimage tool which allows us to import
an SBAT metadata formatted as a CSV file into a .sbat section of the
EFI binary.
Signed-off-by: Peter Jones
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi | 19 +++
include/grub/util/install.h | 3 ++-
include/grub/util/mkimage.h | 1 +
util/grub-install-common.c | 2 +-
util/grub-mkimage.c | 15 ++-
util/mkimage.c | 43 ---
6 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index c5fc3904f..81b90947a 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5784,6 +5784,7 @@ environment variables and commands are listed in the same
order.
* Authentication and authorisation:: Users and access control
* Using digital signatures:: Booting digitally signed code
* UEFI secure boot and shim::Booting digitally signed PE files
+* Secure Boot Advanced Targeting:: Embedded information for generation
number based revocation
* Measured Boot::Measuring boot components
* Lockdown:: Lockdown when booting on a secure setup
@end menu
@@ -5958,6 +5959,24 @@ and @command{memrw} will not be available when the UEFI
secure boot is enabled.
This is done for security reasons and are enforced by the GRUB Lockdown
mechanism
(@pxref{Lockdown}).
+@node Secure Boot Advanced Targeting
+@section Embedded information for generation number based revocation
+
+The Secure Boot Advanced Targeting (SBAT) is a mechanism to allow the
revocation
+of components in the boot path by using generation numbers embedded into the
EFI
+binaries. The SBAT metadata is located in an .sbat data section that has set of
+UTF-8 strings as comma-separated values (CSV). See
+@uref{https://github.com/rhboot/shim/blob/main/SBAT.md} for more details.
+
+To add a data section containing the SBAT information into the binary, the
+@option{--sbat} option of @command{grub-mkimage} command should be used. The
content
+of a CSV file, encoded with UTF-8, is copied as is to the .sbat data section
into
+the generated EFI binary. The CSV file can be stored anywhere on the file
system.
+
+@example
+grub-mkimage -O x86_64-efi -o grubx64.efi -p '(tftp)/grub' --sbat sbat.csv
efinet tftp
+@end example
+
@node Measured Boot
@section Measuring boot components
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 8cbbc4139..3736a16aa 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -183,7 +183,8 @@ grub_install_generate_image (const char *dir, const char
*prefix,
char *config_path,
const struct grub_install_image_target_desc
*image_target,
int note,
-grub_compression_t comp, const char *dtb_file);
+grub_compression_t comp, const char *dtb_file,
+const char *sbat_path);
const struct grub_install_image_target_desc *
grub_install_get_image_target (const char *arg);
diff --git a/include/grub/util/mkimage.h b/include/grub/util/mkimage.h
index ba9f568f6..3819a6744 100644
--- a/include/grub/util/mkimage.h
+++ b/include/grub/util/mkimage.h
@@ -24,6 +24,7 @@ struct grub_mkimage_layout
size_t exec_size;
size_t kernel_size;
size_t bss_size;
+ size_t sbat_size;
grub_uint64_t start_address;
void *reloc_section;
size_t reloc_size;
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index 08d3eb668..56dcb52bf 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -536,7 +536,7 @@ grub_install_make_image_wrap_file (const char *dir, const
char *prefix,
grub_install_generate_image (dir, prefix, fp, outname,
modules.entries, memdisk_path,
pubkeys, npubkeys, config_path, tgt,
- note, compression, dtb);
+ note, compression, dtb, NULL);
while (dc--)
grub_install_pop_module ();
}
diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
index 912564e36..75b884710 100644
--- a/util/grub-mkimage.c
+++ b/util/grub-mkimage.c
@@ -81,6 +81,7 @@ static struct argp_option options[] = {
{"output", 'o', N_("FILE"), 0, N_("output a generated image to FILE
[default=stdout]"), 0},
{"format", 'O', N_("FORMAT"), 0, 0, 0},
{"compression", 'C', "(xz|none|auto)", 0, N_("choose the compression to use
for core image"), 0},
+ {"sbat", 's', N_("FILE"), 0, N_("SBAT metadata"), 0},
{"verbose", 'v', 0, 0, N_("print verbose messages."), 0},
{ 0, 0, 0, 0, 0, 0 }
};
@@ -123,6 +124,7 @@ struct arguments
size_t npubkeys;
char *font;
char *config;
+ char *sbat;
int note;
const struct grub_install_image_target_desc
[SECURITY PATCH 002/117] efi: Move the shim_lock verifier to the GRUB core
From: Marco A Benatto
Move the shim_lock verifier from its own module into the core image. The
Secure Boot lockdown mechanism has the intent to prevent the load of any
unsigned code or binary when Secure Boot is enabled.
The reason is that GRUB must be able to prevent executing untrusted code
if UEFI Secure Boot is enabled, without depending on external modules.
Signed-off-by: Marco A Benatto
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
docs/grub.texi | 9 +--
grub-core/Makefile.core.def| 6 --
grub-core/commands/efi/shim_lock.c | 133 -
grub-core/kern/efi/init.c | 4 ++
grub-core/kern/efi/sb.c| 105 +
include/grub/efi/sb.h | 3 +
6 files changed, 117 insertions(+), 143 deletions(-)
delete mode 100644 grub-core/commands/efi/shim_lock.c
diff --git a/docs/grub.texi b/docs/grub.texi
index eeac9b2ce..e3de2a4dc 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5910,15 +5910,16 @@ secure boot chain.
@section UEFI secure boot and shim support
The GRUB, except the @command{chainloader} command, works with the UEFI secure
-boot and the shim. This functionality is provided by the shim_lock module. It
-is recommend to build in this and other required modules into the
@file{core.img}.
+boot and the shim. This functionality is provided by the shim_lock verifier. It
+is built into the @file{core.img} and is registered if the UEFI secure boot is
+enabled.
+
All modules not stored in the @file{core.img} and the ACPI tables for the
@command{acpi} command have to be signed, e.g. using PGP. Additionally, the
@command{iorw}, the @command{memrw} and the @command{wrmsr} commands are
prohibited if the UEFI secure boot is enabled. This is done due to
security reasons. All above mentioned requirements are enforced by the
-shim_lock module. And itself it is a persistent module which means that
-it cannot be unloaded if it was loaded into the memory.
+shim_lock verifier logic.
@node Measured Boot
@section Measuring boot components
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 0cecc3875..2b98fe1fc 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -945,12 +945,6 @@ module = {
};
module = {
- name = shim_lock;
- common = commands/efi/shim_lock.c;
- enable = efi;
-};
-
-module = {
name = hdparm;
common = commands/hdparm.c;
enable = pci;
diff --git a/grub-core/commands/efi/shim_lock.c
b/grub-core/commands/efi/shim_lock.c
deleted file mode 100644
index f7f3109d6..0
--- a/grub-core/commands/efi/shim_lock.c
+++ /dev/null
@@ -1,133 +0,0 @@
-/*
- * GRUB -- GRand Unified Bootloader
- * Copyright (C) 2017 Free Software Foundation, Inc.
- *
- * GRUB is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * GRUB is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with GRUB. If not, see
[SECURITY PATCH 047/117] video/efi_gop: Remove unnecessary return value of grub_video_gop_fill_mode_info()
From: Darren Kenny
The return value of grub_video_gop_fill_mode_info() is never able to be
anything other than GRUB_ERR_NONE. So, rather than continue to return
a value and checking it each time, it is more correct to redefine the
function to not return anything and remove checks of its return value
altogether.
Fixes: CID 96701
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/video/efi_gop.c | 25 ++---
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/grub-core/video/efi_gop.c b/grub-core/video/efi_gop.c
index 7fe0cdabf..b7590dc6c 100644
--- a/grub-core/video/efi_gop.c
+++ b/grub-core/video/efi_gop.c
@@ -235,7 +235,7 @@ grub_video_gop_fill_real_mode_info (unsigned mode,
return GRUB_ERR_NONE;
}
-static grub_err_t
+static void
grub_video_gop_fill_mode_info (unsigned mode,
struct grub_efi_gop_mode_info *in,
struct grub_video_mode_info *out)
@@ -260,8 +260,6 @@ grub_video_gop_fill_mode_info (unsigned mode,
out->blit_format = GRUB_VIDEO_BLIT_FORMAT_BGRA_;
out->mode_type |= (GRUB_VIDEO_MODE_TYPE_DOUBLE_BUFFERED
| GRUB_VIDEO_MODE_TYPE_UPDATING_SWAP);
-
- return GRUB_ERR_NONE;
}
static int
@@ -274,7 +272,6 @@ grub_video_gop_iterate (int (*hook) (const struct
grub_video_mode_info *info, vo
grub_efi_uintn_t size;
grub_efi_status_t status;
struct grub_efi_gop_mode_info *info = NULL;
- grub_err_t err;
struct grub_video_mode_info mode_info;
status = efi_call_4 (gop->query_mode, gop, mode, , );
@@ -285,12 +282,7 @@ grub_video_gop_iterate (int (*hook) (const struct
grub_video_mode_info *info, vo
continue;
}
- err = grub_video_gop_fill_mode_info (mode, info, _info);
- if (err)
- {
- grub_errno = GRUB_ERR_NONE;
- continue;
- }
+ grub_video_gop_fill_mode_info (mode, info, _info);
if (hook (_info, hook_arg))
return 1;
}
@@ -474,13 +466,8 @@ grub_video_gop_setup (unsigned int width, unsigned int
height,
info = gop->mode->info;
- err = grub_video_gop_fill_mode_info (gop->mode->mode, info,
- _info);
- if (err)
-{
- grub_dprintf ("video", "GOP: couldn't fill mode info\n");
- return err;
-}
+ grub_video_gop_fill_mode_info (gop->mode->mode, info,
+_info);
framebuffer.ptr = (void *) (grub_addr_t) gop->mode->fb_base;
framebuffer.offscreen
@@ -494,8 +481,8 @@ grub_video_gop_setup (unsigned int width, unsigned int
height,
{
grub_dprintf ("video", "GOP: couldn't allocate shadow\n");
grub_errno = 0;
- err = grub_video_gop_fill_mode_info (gop->mode->mode, info,
- _info);
+ grub_video_gop_fill_mode_info (gop->mode->mode, info,
+_info);
buffer = framebuffer.ptr;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 101/117] kern/efi: Add initial stack protector implementation
From: Chris Coulson
It works only on UEFI platforms but can be quite easily extended to
others architectures and platforms if needed.
Signed-off-by: Chris Coulson
Signed-off-by: Daniel Kiper
Reviewed-by: Marco A Benatto
Reviewed-by: Javier Martinez Canillas
---
acinclude.m4 | 38 +++--
configure.ac | 44 ++
grub-core/Makefile.am | 1 +
grub-core/kern/efi/init.c | 54 ++
include/grub/efi/api.h | 19 +++
include/grub/stack_protector.h | 30 +++
6 files changed, 179 insertions(+), 7 deletions(-)
create mode 100644 include/grub/stack_protector.h
diff --git a/acinclude.m4 b/acinclude.m4
index 78cdf6e1d..6e14bb553 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -305,9 +305,9 @@ fi
])
-dnl Check if the C compiler supports `-fstack-protector'.
+dnl Check if the C compiler supports the stack protector
AC_DEFUN([grub_CHECK_STACK_PROTECTOR],[
-[# Smashing stack protector.
+[# Stack smashing protector.
ssp_possible=yes]
AC_MSG_CHECKING([whether `$CC' accepts `-fstack-protector'])
# Is this a reliable test case?
@@ -324,6 +324,40 @@ else
ssp_possible=no]
AC_MSG_RESULT([no])
[fi]
+[# Strong stack smashing protector.
+ssp_strong_possible=yes]
+AC_MSG_CHECKING([whether `$CC' accepts `-fstack-protector-strong'])
+# Is this a reliable test case?
+AC_LANG_CONFTEST([AC_LANG_SOURCE([[
+void foo (void) { volatile char a[8]; a[3]; }
+]])])
+[# `$CC -c -o ...' might not be portable. But, oh, well... Is calling
+# `ac_compile' like this correct, after all?
+if eval "$ac_compile -S -fstack-protector-strong -o conftest.s" 2> /dev/null;
then]
+ AC_MSG_RESULT([yes])
+ [# Should we clear up other files as well, having called `AC_LANG_CONFTEST'?
+ rm -f conftest.s
+else
+ ssp_strong_possible=no]
+ AC_MSG_RESULT([no])
+[fi]
+[# Global stack smashing protector.
+ssp_global_possible=yes]
+AC_MSG_CHECKING([whether `$CC' accepts `-mstack-protector-guard=global'])
+# Is this a reliable test case?
+AC_LANG_CONFTEST([AC_LANG_SOURCE([[
+void foo (void) { volatile char a[8]; a[3]; }
+]])])
+[# `$CC -c -o ...' might not be portable. But, oh, well... Is calling
+# `ac_compile' like this correct, after all?
+if eval "$ac_compile -S -fstack-protector -mstack-protector-guard=global -o
conftest.s" 2> /dev/null; then]
+ AC_MSG_RESULT([yes])
+ [# Should we clear up other files as well, having called `AC_LANG_CONFTEST'?
+ rm -f conftest.s
+else
+ ssp_global_possible=no]
+ AC_MSG_RESULT([no])
+[fi]
])
dnl Check if the C compiler supports `-mstack-arg-probe' (Cygwin).
diff --git a/configure.ac b/configure.ac
index 7c10a4db7..676137deb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1312,12 +1312,41 @@ fi]
CFLAGS="$TARGET_CFLAGS"
-# Smashing stack protector.
+# Stack smashing protector.
grub_CHECK_STACK_PROTECTOR
-# Need that, because some distributions ship compilers that include
-# `-fstack-protector' in the default specs.
-if test "x$ssp_possible" = xyes; then
- TARGET_CFLAGS="$TARGET_CFLAGS -fno-stack-protector"
+AC_ARG_ENABLE([stack-protector],
+ AS_HELP_STRING([--enable-stack-protector],
+[enable the stack protector]),
+ [],
+ [enable_stack_protector=no])
+if test "x$enable_stack_protector" = xno; then
+ if test "x$ssp_possible" = xyes; then
+# Need that, because some distributions ship compilers that include
+# `-fstack-protector' in the default specs.
+TARGET_CFLAGS="$TARGET_CFLAGS -fno-stack-protector"
+ fi
+elif test "x$platform" != xefi; then
+ AC_MSG_ERROR([--enable-stack-protector is only supported on EFI platforms])
+elif test "x$ssp_global_possible" != xyes; then
+ AC_MSG_ERROR([--enable-stack-protector is not supported (compiler doesn't
support -mstack-protector-guard=global)])
+else
+ TARGET_CFLAGS="$TARGET_CFLAGS -mstack-protector-guard=global"
+ if test "x$enable_stack_protector" = xyes; then
+if test "x$ssp_possible" != xyes; then
+ AC_MSG_ERROR([--enable-stack-protector is not supported (compiler
doesn't support -fstack-protector)])
+fi
+TARGET_CFLAGS="$TARGET_CFLAGS -fstack-protector"
+ elif test "x$enable_stack_protector" = xstrong; then
+if test "x$ssp_strong_possible" != xyes; then
+ AC_MSG_ERROR([--enable-stack-protector=strong is not supported (compiler
doesn't support -fstack-protector-strong)])
+fi
+TARGET_CFLAGS="$TARGET_CFLAGS -fstack-protector-strong"
+ else
+# Note, -fstack-protector-all requires that the protector is disabled for
+# functions that appear in the call stack when the canary is initialized.
+AC_MSG_ERROR([invalid value $enable_stack_protector for
--enable-stack-protector])
+ fi
+ TARGET_CPPFLAGS="$TARGET_CPPFLAGS -DGRUB_STACK_PROTECTOR=1"
fi
CFLAGS="$TARGET_CFLAGS"
@@ -2132,5 +2161,10 @@ echo "Without liblzma (no support for
[SECURITY PATCH 089/117] disk/lvm: Do not crash if an expected string is not found
From: Daniel Axtens
Clean up a bunch of cases where we could have strstr() fail and lead to
us dereferencing NULL.
We'll still leak memory in some cases (loops don't clean up allocations
from earlier iterations if a later iteration fails) but at least we're
not crashing.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 31bbc9acc..201097fda 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -578,7 +578,16 @@ grub_lvm_detect (grub_disk_t disk,
}
if (seg->node_count != 1)
- seg->stripe_size = grub_lvm_getvalue (, "stripe_size
= ");
+ {
+ seg->stripe_size = grub_lvm_getvalue (,
"stripe_size = ");
+ if (p == NULL)
+ {
+#ifdef GRUB_UTIL
+ grub_util_info ("unknown stripe_size");
+#endif
+ goto lvs_segment_fail;
+ }
+ }
seg->nodes = grub_calloc (seg->node_count,
sizeof (*stripe));
@@ -598,7 +607,7 @@ grub_lvm_detect (grub_disk_t disk,
{
p = grub_strchr (p, '"');
if (p == NULL)
- continue;
+ goto lvs_segment_fail2;
q = ++p;
while (*q != '"')
q++;
@@ -617,7 +626,10 @@ grub_lvm_detect (grub_disk_t disk,
stripe->start = grub_lvm_getvalue (, ",")
* vg->extent_size;
if (p == NULL)
- continue;
+ {
+ grub_free (stripe->name);
+ goto lvs_segment_fail2;
+ }
stripe++;
}
@@ -654,7 +666,7 @@ grub_lvm_detect (grub_disk_t disk,
p = grub_strchr (p, '"');
if (p == NULL)
- continue;
+ goto lvs_segment_fail2;
q = ++p;
while (*q != '"')
q++;
@@ -742,7 +754,7 @@ grub_lvm_detect (grub_disk_t disk,
p = p ? grub_strchr (p + 1, '"') : 0;
p = p ? grub_strchr (p + 1, '"') : 0;
if (p == NULL)
- continue;
+ goto lvs_segment_fail2;
q = ++p;
while (*q != '"')
q++;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 001/117] verifiers: Move verifiers API to kernel image
From: Marco A Benatto
Move verifiers API from a module to the kernel image, so it can be
used there as well. There are no functional changes in this patch.
Signed-off-by: Marco A Benatto
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/Makefile.am| 1 +
grub-core/Makefile.core.def | 6 +-
grub-core/kern/main.c| 4
grub-core/{commands => kern}/verifiers.c | 8 ++--
include/grub/verify.h| 9 ++---
5 files changed, 14 insertions(+), 14 deletions(-)
rename grub-core/{commands => kern}/verifiers.c (97%)
diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
index c6ba5b2d7..cc6fc7dfa 100644
--- a/grub-core/Makefile.am
+++ b/grub-core/Makefile.am
@@ -91,6 +91,7 @@ KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/parser.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/partition.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/term.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/time.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/verify.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/mm_private.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/net.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/memory.h
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 68b9e9f68..0cecc3875 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -140,6 +140,7 @@ kernel = {
common = kern/rescue_parser.c;
common = kern/rescue_reader.c;
common = kern/term.c;
+ common = kern/verifiers.c;
noemu = kern/compiler-rt.c;
noemu = kern/mm.c;
@@ -944,11 +945,6 @@ module = {
};
module = {
- name = verifiers;
- common = commands/verifiers.c;
-};
-
-module = {
name = shim_lock;
common = commands/efi/shim_lock.c;
enable = efi;
diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
index 9cad0c448..73967e2f5 100644
--- a/grub-core/kern/main.c
+++ b/grub-core/kern/main.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#ifdef GRUB_MACHINE_PCBIOS
#include
@@ -274,6 +275,9 @@ grub_main (void)
grub_printf ("Welcome to GRUB!\n\n");
grub_setcolorstate (GRUB_TERM_COLOR_STANDARD);
+ /* Init verifiers API. */
+ grub_verifiers_init ();
+
grub_load_config ();
grub_boot_time ("Before loading embedded modules.");
diff --git a/grub-core/commands/verifiers.c b/grub-core/kern/verifiers.c
similarity index 97%
rename from grub-core/commands/verifiers.c
rename to grub-core/kern/verifiers.c
index aae4d84bb..75d7994cf 100644
--- a/grub-core/commands/verifiers.c
+++ b/grub-core/kern/verifiers.c
@@ -221,12 +221,8 @@ grub_verify_string (char *str, enum
grub_verify_string_type type)
return GRUB_ERR_NONE;
}
-GRUB_MOD_INIT(verifiers)
+void
+grub_verifiers_init (void)
{
grub_file_filter_register (GRUB_FILE_FILTER_VERIFY, grub_verifiers_open);
}
-
-GRUB_MOD_FINI(verifiers)
-{
- grub_file_filter_unregister (GRUB_FILE_FILTER_VERIFY);
-}
diff --git a/include/grub/verify.h b/include/grub/verify.h
index ea0491433..cd129c398 100644
--- a/include/grub/verify.h
+++ b/include/grub/verify.h
@@ -64,7 +64,10 @@ struct grub_file_verifier
grub_err_t (*verify_string) (char *str, enum grub_verify_string_type type);
};
-extern struct grub_file_verifier *grub_file_verifiers;
+extern struct grub_file_verifier *EXPORT_VAR (grub_file_verifiers);
+
+extern void
+grub_verifiers_init (void);
static inline void
grub_verifier_register (struct grub_file_verifier *ver)
@@ -78,7 +81,7 @@ grub_verifier_unregister (struct grub_file_verifier *ver)
grub_list_remove (GRUB_AS_LIST (ver));
}
-grub_err_t
-grub_verify_string (char *str, enum grub_verify_string_type type);
+extern grub_err_t
+EXPORT_FUNC (grub_verify_string) (char *str, enum grub_verify_string_type
type);
#endif /* ! GRUB_VERIFY_HEADER */
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 100/117] kern/parser: Fix a stack buffer overflow
From: Chris Coulson
grub_parser_split_cmdline() expands variable names present in the supplied
command line in to their corresponding variable contents and uses a 1 kiB
stack buffer for temporary storage without sufficient bounds checking. If
the function is called with a command line that references a variable with
a sufficiently large payload, it is possible to overflow the stack
buffer via tab completion, corrupt the stack frame and potentially
control execution.
Fixes: CVE-2020-27749
Reported-by: Chris Coulson
Signed-off-by: Chris Coulson
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 110 +---
1 file changed, 67 insertions(+), 43 deletions(-)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index e010eaa1f..6ab7aa427 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -18,6 +18,7 @@
*/
#include
+#include
#include
#include
#include
@@ -107,8 +108,8 @@ check_varstate (grub_parser_state_t s)
}
-static void
-add_var (char *varname, char **bp, char **vp,
+static grub_err_t
+add_var (grub_buffer_t varname, grub_buffer_t buf,
grub_parser_state_t state, grub_parser_state_t newstate)
{
const char *val;
@@ -116,31 +117,41 @@ add_var (char *varname, char **bp, char **vp,
/* Check if a variable was being read in and the end of the name
was reached. */
if (!(check_varstate (state) && !check_varstate (newstate)))
-return;
+return GRUB_ERR_NONE;
- *((*vp)++) = '\0';
- val = grub_env_get (varname);
- *vp = varname;
+ if (grub_buffer_append_char (varname, '\0') != GRUB_ERR_NONE)
+return grub_errno;
+
+ val = grub_env_get ((const char *) grub_buffer_peek_data (varname));
+ grub_buffer_reset (varname);
if (!val)
-return;
+return GRUB_ERR_NONE;
/* Insert the contents of the variable in the buffer. */
- for (; *val; val++)
-*((*bp)++) = *val;
+ return grub_buffer_append_data (buf, val, grub_strlen (val));
}
-static void
-terminate_arg (char *buffer, char **bp, int *argc)
+static grub_err_t
+terminate_arg (grub_buffer_t buffer, int *argc)
{
- if (*bp != buffer && *((*bp) - 1) != '\0')
-{
- *((*bp)++) = '\0';
- (*argc)++;
-}
+ grub_size_t unread = grub_buffer_get_unread_bytes (buffer);
+
+ if (unread == 0)
+return GRUB_ERR_NONE;
+
+ if (*(const char *) grub_buffer_peek_data_at (buffer, unread - 1) == '\0')
+return GRUB_ERR_NONE;
+
+ if (grub_buffer_append_char (buffer, '\0') != GRUB_ERR_NONE)
+return grub_errno;
+
+ (*argc)++;
+
+ return GRUB_ERR_NONE;
}
static grub_err_t
-process_char (char c, char *buffer, char **bp, char *varname, char **vp,
+process_char (char c, grub_buffer_t buffer, grub_buffer_t varname,
grub_parser_state_t state, int *argc,
grub_parser_state_t *newstate)
{
@@ -153,12 +164,13 @@ process_char (char c, char *buffer, char **bp, char
*varname, char **vp,
* not describe the variable anymore, write the variable to
* the buffer.
*/
- add_var (varname, bp, vp, state, *newstate);
+ if (add_var (varname, buffer, state, *newstate) != GRUB_ERR_NONE)
+return grub_errno;
if (check_varstate (*newstate))
{
if (use)
- *((*vp)++) = use;
+return grub_buffer_append_char (varname, use);
}
else if (*newstate == GRUB_PARSER_STATE_TEXT &&
state != GRUB_PARSER_STATE_ESC && grub_isspace (use))
@@ -167,10 +179,10 @@ process_char (char c, char *buffer, char **bp, char
*varname, char **vp,
* Don't add more than one argument if multiple
* spaces are used.
*/
- terminate_arg (buffer, bp, argc);
+ return terminate_arg (buffer, argc);
}
else if (use)
-*((*bp)++) = use;
+return grub_buffer_append_char (buffer, use);
return GRUB_ERR_NONE;
}
@@ -181,19 +193,22 @@ grub_parser_split_cmdline (const char *cmdline,
int *argc, char ***argv)
{
grub_parser_state_t state = GRUB_PARSER_STATE_TEXT;
- /* XXX: Fixed size buffer, perhaps this buffer should be dynamically
- allocated. */
- char buffer[1024];
- char *bp = buffer;
+ grub_buffer_t buffer, varname;
char *rd = (char *) cmdline;
char *rp = rd;
- char varname[200];
- char *vp = varname;
- char *args;
int i;
*argc = 0;
*argv = NULL;
+
+ buffer = grub_buffer_new (1024);
+ if (buffer == NULL)
+return grub_errno;
+
+ varname = grub_buffer_new (200);
+ if (varname == NULL)
+goto fail;
+
do
{
if (rp == NULL || *rp == '\0')
@@ -219,7 +234,7 @@ grub_parser_split_cmdline (const char *cmdline,
{
grub_parser_state_t newstate;
- if (process_char (*rp, buffer, , varname, , state, argc,
+ if (process_char (*rp, buffer, varname, state, argc,
) != GRUB_ERR_NONE)
goto fail;
@@ -230,10 +245,12 @@ grub_parser_split_cmdline (const char *cmdline,
[SECURITY PATCH 040/117] affs: Fix memory leaks
From: Darren Kenny
The node structure reference is being allocated but not freed if it
reaches the end of the function. If any of the hooks had returned
a non-zero value, then node would have been copied in to the context
reference, but otherwise node is not stored and should be freed.
Similarly, the call to grub_affs_create_node() replaces the allocated
memory in node with a newly allocated structure, leaking the existing
memory pointed by node.
Finally, when dir->parent is set, then we again replace node with newly
allocated memory, which seems unnecessary when we copy in the values
from dir->parent immediately after.
Fixes: CID 73759
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/affs.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/grub-core/fs/affs.c b/grub-core/fs/affs.c
index 220b3712f..230e26af0 100644
--- a/grub-core/fs/affs.c
+++ b/grub-core/fs/affs.c
@@ -400,12 +400,12 @@ grub_affs_iterate_dir (grub_fshelp_node_t dir,
{
unsigned int i;
struct grub_affs_file file;
- struct grub_fshelp_node *node = 0;
+ struct grub_fshelp_node *node, *orig_node;
struct grub_affs_data *data = dir->data;
grub_uint32_t *hashtable;
/* Create the directory entries for `.' and `..'. */
- node = grub_zalloc (sizeof (*node));
+ node = orig_node = grub_zalloc (sizeof (*node));
if (!node)
return 1;
@@ -414,9 +414,6 @@ grub_affs_iterate_dir (grub_fshelp_node_t dir,
return 1;
if (dir->parent)
{
- node = grub_zalloc (sizeof (*node));
- if (!node)
- return 1;
*node = *dir->parent;
if (hook ("..", GRUB_FSHELP_DIR, node, hook_data))
return 1;
@@ -456,17 +453,18 @@ grub_affs_iterate_dir (grub_fshelp_node_t dir,
if (grub_affs_create_node (dir, hook, hook_data, , ,
next, ))
- return 1;
+ {
+ /* Node has been replaced in function. */
+ grub_free (orig_node);
+ return 1;
+ }
next = grub_be_to_cpu32 (file.next);
}
}
- grub_free (hashtable);
- return 0;
-
fail:
- grub_free (node);
+ grub_free (orig_node);
grub_free (hashtable);
return 0;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 075/117] fs/sfs: Fix over-read of root object name
From: Daniel Axtens There's a read of the name of the root object that assumes that the name is nul-terminated within the root block. This isn't guaranteed - it seems SFS would require you to read multiple blocks to get a full name in general, but maybe that doesn't apply to the root object. Either way, figure out how much space is left in the root block and don't over-read it. This fixes some OOB reads. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/fs/sfs.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/grub-core/fs/sfs.c b/grub-core/fs/sfs.c index de2b107a4..983e88008 100644 --- a/grub-core/fs/sfs.c +++ b/grub-core/fs/sfs.c @@ -373,6 +373,7 @@ grub_sfs_mount (grub_disk_t disk) struct grub_sfs_objc *rootobjc; char *rootobjc_data = 0; grub_uint32_t blk; + unsigned int max_len; data = grub_malloc (sizeof (*data)); if (!data) @@ -421,7 +422,13 @@ grub_sfs_mount (grub_disk_t disk) data->diropen.data = data; data->diropen.cache = 0; data->disk = disk; - data->label = grub_strdup ((char *) (rootobjc->objects[0].filename)); + + /* We only read 1 block of data, so truncate the name if needed. */ + max_len = ((GRUB_DISK_SECTOR_SIZE << data->log_blocksize) +- 24/* offsetof (struct grub_sfs_objc, objects) */ +- 25); /* offsetof (struct grub_sfs_obj, filename) */ + data->label = grub_zalloc (max_len + 1); + grub_strncpy (data->label, (char *) rootobjc->objects[0].filename, max_len); grub_free (rootobjc_data); return data; -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 094/117] fs/btrfs: Squash some uninitialized reads
From: Daniel Axtens
We need to check errors before calling into a function that uses the result.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/btrfs.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index c4ba5f110..63203034d 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -383,9 +383,9 @@ next (struct grub_btrfs_data *data,
err = grub_btrfs_read_logical (data, grub_le_to_cpu64 (node.addr),
, sizeof (head), 0);
+ if (err)
+ return -err;
check_btrfs_header (data, , grub_le_to_cpu64 (node.addr));
- if (err)
- return -err;
save_ref (desc, grub_le_to_cpu64 (node.addr), 0,
grub_le_to_cpu32 (head.nitems), !head.level);
@@ -445,9 +445,9 @@ lower_bound (struct grub_btrfs_data *data,
/* FIXME: preread few nodes into buffer. */
err = grub_btrfs_read_logical (data, addr, , sizeof (head),
recursion_depth + 1);
+ if (err)
+ return err;
check_btrfs_header (data, , addr);
- if (err)
- return err;
addr += sizeof (head);
if (head.level)
{
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 038/117] zfs: Fix possible integer overflows
From: Darren Kenny
In all cases the problem is that the value being acted upon by
a left-shift is a 32-bit number which is then being used in the
context of a 64-bit number.
To avoid overflow we ensure that the number being shifted is 64-bit
before the shift is done.
Fixes: CID 73684, CID 73695, CID 73764
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/zfs/zfs.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index ca82eb878..9ef2fd61a 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -567,7 +567,7 @@ find_bestub (uberblock_phys_t * ub_array,
ubptr = (uberblock_phys_t *) ((grub_properly_aligned_t *) ub_array
+ ((i << ub_shift)
/ sizeof (grub_properly_aligned_t)));
- err = uberblock_verify (ubptr, offset, 1 << ub_shift);
+ err = uberblock_verify (ubptr, offset, (grub_size_t) 1 << ub_shift);
if (err)
{
grub_errno = GRUB_ERR_NONE;
@@ -1546,7 +1546,7 @@ read_device (grub_uint64_t offset, struct
grub_zfs_device_desc *desc,
high = grub_divmod64 ((offset >> desc->ashift) + c,
desc->n_children, );
- csize = bsize << desc->ashift;
+ csize = (grub_size_t) bsize << desc->ashift;
if (csize > len)
csize = len;
@@ -1638,8 +1638,8 @@ read_device (grub_uint64_t offset, struct
grub_zfs_device_desc *desc,
while (len > 0)
{
- grub_size_t csize;
- csize = ((s / (desc->n_children - desc->nparity))
+ grub_size_t csize = s;
+ csize = ((csize / (desc->n_children - desc->nparity))
<< desc->ashift);
if (csize > len)
csize = len;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 068/117] video/readers/jpeg: Catch OOB reads/writes in grub_jpeg_decode_du()
From: Daniel Axtens
The key line is:
du[jpeg_zigzag_order[pos]] = val * (int) data->quan_table[qt][pos];
jpeg_zigzag_order is grub_uint8_t[64].
I don't understand JPEG decoders quite well enough to explain what's
going on here. However, I observe sometimes pos=64, which leads to an
OOB read of the jpeg_zigzag_order global then an OOB write to du.
That leads to various unpleasant memory corruption conditions.
Catch where pos >= ARRAY_SIZE(jpeg_zigzag_order) and bail.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/video/readers/jpeg.c | 8
1 file changed, 8 insertions(+)
diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c
index 23f919aa0..e5148120f 100644
--- a/grub-core/video/readers/jpeg.c
+++ b/grub-core/video/readers/jpeg.c
@@ -526,6 +526,14 @@ grub_jpeg_decode_du (struct grub_jpeg_data *data, int id,
jpeg_data_unit_t du)
val = grub_jpeg_get_number (data, num & 0xF);
num >>= 4;
pos += num;
+
+ if (pos >= ARRAY_SIZE (jpeg_zigzag_order))
+ {
+ grub_error (GRUB_ERR_BAD_FILE_TYPE,
+ "jpeg: invalid position in zigzag order!?");
+ return;
+ }
+
du[jpeg_zigzag_order[pos]] = val * (int) data->quan_table[qt][pos];
pos++;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 095/117] kern/parser: Fix a memory leak
From: Chris Coulson
The getline() function supplied to grub_parser_split_cmdline() returns
a newly allocated buffer and can be called multiple times, but the
returned buffer is never freed.
Signed-off-by: Chris Coulson
Reviewed-by: Daniel Kiper
---
grub-core/kern/parser.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index d1cf061ad..39e4df65b 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -140,6 +140,7 @@ grub_parser_split_cmdline (const char *cmdline,
char buffer[1024];
char *bp = buffer;
char *rd = (char *) cmdline;
+ char *rp = rd;
char varname[200];
char *vp = varname;
char *args;
@@ -149,10 +150,18 @@ grub_parser_split_cmdline (const char *cmdline,
*argv = NULL;
do
{
- if (!rd || !*rd)
+ if (rp == NULL || *rp == '\0')
{
+ if (rd != cmdline)
+ {
+ grub_free (rd);
+ rd = rp = NULL;
+ }
if (getline)
- getline (, 1, getline_data);
+ {
+ getline (, 1, getline_data);
+ rp = rd;
+ }
else
break;
}
@@ -160,12 +169,12 @@ grub_parser_split_cmdline (const char *cmdline,
if (!rd)
break;
- for (; *rd; rd++)
+ for (; *rp != '\0'; rp++)
{
grub_parser_state_t newstate;
char use;
- newstate = grub_parser_cmdline_state (state, *rd, );
+ newstate = grub_parser_cmdline_state (state, *rp, );
/* If a variable was being processed and this character does
not describe the variable anymore, write the variable to
@@ -198,6 +207,9 @@ grub_parser_split_cmdline (const char *cmdline,
}
while (state != GRUB_PARSER_STATE_TEXT && !check_varstate (state));
+ if (rd != cmdline)
+grub_free (rd);
+
/* A special case for when the last character was part of a
variable. */
add_var (varname, , , state, GRUB_PARSER_STATE_TEXT);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 039/117] zfsinfo: Correct a check for error allocating memory
From: Darren Kenny
While arguably the check for grub_errno is correct, we should really be
checking the return value from the function since it is always possible
that grub_errno was set elsewhere, making this code behave incorrectly.
Fixes: CID 73668
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/zfs/zfsinfo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/fs/zfs/zfsinfo.c b/grub-core/fs/zfs/zfsinfo.c
index c8a28acf5..bf2918018 100644
--- a/grub-core/fs/zfs/zfsinfo.c
+++ b/grub-core/fs/zfs/zfsinfo.c
@@ -358,8 +358,8 @@ grub_cmd_zfs_bootfs (grub_command_t cmd __attribute__
((unused)), int argc,
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
devname = grub_file_get_device_name (args[0]);
- if (grub_errno)
-return grub_errno;
+ if (devname == NULL)
+return GRUB_ERR_OUT_OF_MEMORY;
dev = grub_device_open (devname);
grub_free (devname);
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 069/117] video/readers/jpeg: Don't decode data before start of stream
From: Daniel Axtens When a start of stream marker is encountered, we call grub_jpeg_decode_sos() which allocates space for a bitmap. When a restart marker is encountered, we call grub_jpeg_decode_data() which then fills in that bitmap. If we get a restart marker before the start of stream marker, we will attempt to write to a bitmap_ptr that hasn't been allocated. Catch this and bail out. This fixes an attempt to write to NULL. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/video/readers/jpeg.c | 4 1 file changed, 4 insertions(+) diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c index e5148120f..e31602f76 100644 --- a/grub-core/video/readers/jpeg.c +++ b/grub-core/video/readers/jpeg.c @@ -646,6 +646,10 @@ grub_jpeg_decode_data (struct grub_jpeg_data *data) nr1 = (data->image_height + vb - 1) >> (3 + data->log_vs); nc1 = (data->image_width + hb - 1) >> (3 + data->log_hs); + if (data->bitmap_ptr == NULL) +return grub_error(GRUB_ERR_BAD_FILE_TYPE, + "jpeg: attempted to decode data before start of stream"); + for (; data->r1 < nr1 && (!data->dri || rst); data->r1++, data->bitmap_ptr += (vb * data->image_width - hb * nc1) * 3) for (c1 = 0; c1 < nc1 && (!data->dri || rst); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 076/117] fs/jfs: Do not move to leaf level if name length is negative
From: Daniel Axtens
Fuzzing JFS revealed crashes where a negative number would be passed
to le_to_cpu16_copy(). There it would be cast to a large positive number
and the copy would read and write off the end of the respective buffers.
Catch this at the top as well as the bottom of the loop.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/jfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/fs/jfs.c b/grub-core/fs/jfs.c
index d5a6d6527..e5bbda61c 100644
--- a/grub-core/fs/jfs.c
+++ b/grub-core/fs/jfs.c
@@ -567,7 +567,7 @@ grub_jfs_getent (struct grub_jfs_diropen *diro)
/* Move down to the leaf level. */
nextent = leaf->next;
- if (leaf->next != 255)
+ if (leaf->next != 255 && len > 0)
do
{
next_leaf = >next_leaf[nextent];
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 033/117] disk/ldm: Fix memory leak on uninserted lv references
From: Darren Kenny
The problem here is that the memory allocated to the variable lv is not
yet inserted into the list that is being processed at the label fail2.
As we can already see at line 342, which correctly frees lv before going
to fail2, we should also be doing that at these earlier jumps to fail2.
Fixes: CID 73824
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/disk/ldm.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
index c25941ec9..4577a51dc 100644
--- a/grub-core/disk/ldm.c
+++ b/grub-core/disk/ldm.c
@@ -321,7 +321,10 @@ make_vg (grub_disk_t disk,
lv->visible = 1;
lv->segments = grub_zalloc (sizeof (*lv->segments));
if (!lv->segments)
- goto fail2;
+ {
+ grub_free (lv);
+ goto fail2;
+ }
lv->segments->start_extent = 0;
lv->segments->type = GRUB_DISKFILTER_MIRROR;
lv->segments->node_count = 0;
@@ -329,7 +332,10 @@ make_vg (grub_disk_t disk,
lv->segments->nodes = grub_calloc (lv->segments->node_alloc,
sizeof (*lv->segments->nodes));
if (!lv->segments->nodes)
- goto fail2;
+ {
+ grub_free (lv);
+ goto fail2;
+ }
ptr = vblk[i].dynamic;
if (ptr + *ptr + 1 >= vblk[i].dynamic
+ sizeof (vblk[i].dynamic))
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 064/117] script/execute: Don't crash on a "for" loop with no items
From: Daniel Axtens
The following crashes the parser:
for x in; do
0
done
This is because grub_script_arglist_to_argv() doesn't consider the
possibility that arglist is NULL. Catch that explicitly.
This avoids a NULL pointer dereference.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/script/execute.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/grub-core/script/execute.c b/grub-core/script/execute.c
index b88765f1d..25158407d 100644
--- a/grub-core/script/execute.c
+++ b/grub-core/script/execute.c
@@ -624,6 +624,9 @@ grub_script_arglist_to_argv (struct grub_script_arglist
*arglist,
struct grub_script_arg *arg = 0;
struct grub_script_argv result = { 0, 0, 0 };
+ if (arglist == NULL)
+return 1;
+
for (; arglist && arglist->arg; arglist = arglist->next)
{
if (grub_script_argv_next ())
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 087/117] disk/lvm: Don't blast past the end of the circular metadata buffer
From: Daniel Axtens
This catches at least some OOB reads, and it's possible I suppose that
if 2 * mda_size is less than GRUB_LVM_MDA_HEADER_SIZE it might catch some
OOB writes too (although that hasn't showed up as a crash in fuzzing yet).
It's a bit ugly and I'd appreciate better suggestions.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/disk/lvm.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 6988fe492..b70d7d79f 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -251,6 +251,16 @@ grub_lvm_detect (grub_disk_t disk,
if (grub_le_to_cpu64 (rlocn->offset) + grub_le_to_cpu64 (rlocn->size) >
grub_le_to_cpu64 (mdah->size))
{
+ if (2 * mda_size < GRUB_LVM_MDA_HEADER_SIZE ||
+ (grub_le_to_cpu64 (rlocn->offset) + grub_le_to_cpu64 (rlocn->size) -
+ grub_le_to_cpu64 (mdah->size) > mda_size - GRUB_LVM_MDA_HEADER_SIZE))
+ {
+#ifdef GRUB_UTIL
+ grub_util_info ("cannot copy metadata wrap in circular buffer");
+#endif
+ goto fail2;
+ }
+
/* Metadata is circular. Copy the wrap in place. */
grub_memcpy (metadatabuf + mda_size,
metadatabuf + GRUB_LVM_MDA_HEADER_SIZE,
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 070/117] term/gfxterm: Don't set up a font with glyphs that are too big
From: Daniel Axtens Catch the case where we have a font so big that it causes the number of rows or columns to be 0. Currently we continue and allocate a virtual_screen.text_buffer of size 0. We then try to use that for glpyhs and things go badly. On the emu platform, malloc() may give us a valid pointer, in which case we'll access heap memory which we shouldn't. Alternatively, it may give us NULL, in which case we'll crash. For other platforms, if I understand grub_memalign() correctly, we will receive a valid but small allocation that we will very likely later overrun. Prevent the creation of a virtual screen that isn't at least 40 cols by 12 rows. This is arbitrary, but it seems that if your width or height is half a standard 80x24 terminal, you're probably going to struggle to read anything anyway. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/term/gfxterm.c | 9 + 1 file changed, 9 insertions(+) diff --git a/grub-core/term/gfxterm.c b/grub-core/term/gfxterm.c index af7c090a3..b40fcce91 100644 --- a/grub-core/term/gfxterm.c +++ b/grub-core/term/gfxterm.c @@ -232,6 +232,15 @@ grub_virtual_screen_setup (unsigned int x, unsigned int y, virtual_screen.columns = virtual_screen.width / virtual_screen.normal_char_width; virtual_screen.rows = virtual_screen.height / virtual_screen.normal_char_height; + /* + * There must be a minimum number of rows and columns for the screen to + * make sense. Arbitrarily pick half of 80x24. If either dimensions is 0 + * we would allocate 0 bytes for the text_buffer. + */ + if (virtual_screen.columns < 40 || virtual_screen.rows < 12) +return grub_error (GRUB_ERR_BAD_FONT, + "font: glyphs too large to fit on screen"); + /* Allocate memory for text buffer. */ virtual_screen.text_buffer = (struct grub_colored_char *) grub_malloc (virtual_screen.columns -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 027/117] gnulib/regcomp: Fix uninitialized re_token
From: Darren Kenny
This issue has been fixed in the latest version of gnulib, so to
maintain consistency, I've backported that change rather than doing
something different.
Fixes: CID 73828
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
bootstrap.conf| 4 ++--
conf/Makefile.extra-dist | 1 +
.../lib/gnulib-patches/fix-regcomp-uninit-token.patch | 15 +++
3 files changed, 18 insertions(+), 2 deletions(-)
create mode 100644 grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index 22feff5c0..6b043fc35 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -79,8 +79,8 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-base64 fix-null-deref fix-null-state-deref
fix-regexec-null-deref \
- fix-uninit-structure fix-unused-value fix-width no-abort; do
+ for patchname in fix-base64 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"
done
diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index c0ac65ec3..8f1485d52 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -31,6 +31,7 @@ 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
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
diff --git a/grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
b/grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
new file mode 100644
index 0..02e06315d
--- /dev/null
+++ b/grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
@@ -0,0 +1,15 @@
+--- a/lib/regcomp.c2020-11-24 17:06:08.159223858 +
b/lib/regcomp.c2020-11-24 17:06:15.630253923 +
+@@ -3808,11 +3808,7 @@
+ create_tree (re_dfa_t *dfa, bin_tree_t *left, bin_tree_t *right,
+re_token_type_t type)
+ {
+- re_token_t t;
+-#if defined GCC_LINT || defined lint
+- memset (, 0, sizeof t);
+-#endif
+- t.type = type;
++ re_token_t t = { .type = type };
+ return create_token_tree (dfa, left, right, );
+ }
+
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 079/117] fs/nilfs2: Reject too-large keys
From: Daniel Axtens
NILFS2 has up to 7 keys, per the data structure. Do not permit array
indices in excess of that.
This catches some OOB reads. I don't know how controllable the invalidly
read data is or if that could be used later in the program.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/nilfs2.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/nilfs2.c b/grub-core/fs/nilfs2.c
index 082326f38..20053caf5 100644
--- a/grub-core/fs/nilfs2.c
+++ b/grub-core/fs/nilfs2.c
@@ -569,6 +569,11 @@ grub_nilfs2_btree_lookup (struct grub_nilfs2_data *data,
static inline grub_uint64_t
grub_nilfs2_direct_lookup (struct grub_nilfs2_inode *inode, grub_uint64_t key)
{
+ if (1 + key > 6)
+{
+ grub_error (GRUB_ERR_BAD_FS, "key is too large");
+ return 0x;
+}
return grub_le_to_cpu64 (inode->i_bmap[1 + key]);
}
@@ -584,7 +589,7 @@ grub_nilfs2_bmap_lookup (struct grub_nilfs2_data *data,
{
grub_uint64_t ptr;
ptr = grub_nilfs2_direct_lookup (inode, key);
- if (need_translate)
+ if (ptr != ((grub_uint64_t) 0x) && need_translate)
ptr = grub_nilfs2_dat_translate (data, ptr);
return ptr;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 023/117] gnulib/regexec: Resolve unused variable
From: Darren Kenny
This is a really minor issue where a variable is being assigned to but
not checked before it is overwritten again.
The reason for this issue is that we are not building with DEBUG set and
this in turn means that the assert() that reads the value of the
variable match_last is being processed out.
The solution, move the assignment to match_last in to an ifdef DEBUG too.
Fixes: CID 292459
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
bootstrap.conf | 2 +-
conf/Makefile.extra-dist| 1 +
grub-core/lib/gnulib-patches/fix-unused-value.patch | 14 ++
3 files changed, 16 insertions(+), 1 deletion(-)
create mode 100644 grub-core/lib/gnulib-patches/fix-unused-value.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index 6d383240e..4c8c37c16 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -79,7 +79,7 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-base64 fix-null-deref fix-width no-abort; do
+ for patchname in fix-base64 fix-null-deref fix-unused-value fix-width
no-abort; 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 37dc3aa10..0d3b74e8e 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -30,6 +30,7 @@ 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-unused-value.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/no-abort.patch
diff --git a/grub-core/lib/gnulib-patches/fix-unused-value.patch
b/grub-core/lib/gnulib-patches/fix-unused-value.patch
new file mode 100644
index 0..ba51f1bf2
--- /dev/null
+++ b/grub-core/lib/gnulib-patches/fix-unused-value.patch
@@ -0,0 +1,14 @@
+--- a/lib/regexec.c2020-10-21 14:25:35.310195912 +
b/lib/regexec.c2020-10-21 14:32:07.961765604 +
+@@ -828,7 +828,11 @@
+ break;
+ if (__glibc_unlikely (err != REG_NOMATCH))
+ goto free_return;
++#ifdef DEBUG
++/* Only used for assertion below when DEBUG is set, otherwise
++ it will be over-written when we loop around. */
+ match_last = -1;
++#endif
+ }
+ else
+ break; /* We found a match. */
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 063/117] lib/arg: Block repeated short options that require an argument
From: Daniel Axtens
Fuzzing found the following crash:
search -hf
We didn't allocate enough option space for 13 hints because the
allocation code counts the number of discrete arguments (i.e. argc).
However, the shortopt parsing code will happily keep processing
a combination of short options without checking if those short
options require an argument. This means you can easily end writing
past the allocated option space.
This fixes a OOB write which can cause heap corruption.
Fixes: CVE-2021-20225
Reported-by: Daniel Axtens
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/lib/arg.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/grub-core/lib/arg.c b/grub-core/lib/arg.c
index 8439a0062..ade82d5dc 100644
--- a/grub-core/lib/arg.c
+++ b/grub-core/lib/arg.c
@@ -299,6 +299,19 @@ grub_arg_parse (grub_extcmd_t cmd, int argc, char **argv,
it can have an argument value. */
if (*curshort)
{
+ /*
+ * Only permit further short opts if this one doesn't
+ * require a value.
+ */
+ if (opt->type != ARG_TYPE_NONE &&
+ !(opt->flags & GRUB_ARG_OPTION_OPTIONAL))
+ {
+ grub_error (GRUB_ERR_BAD_ARGUMENT,
+ N_("missing mandatory option for `%s'"),
+ opt->longarg);
+ goto fail;
+ }
+
if (parse_option (cmd, opt, 0, usr) || grub_errno)
goto fail;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 083/117] io/gzio: Add init_dynamic_block() clean up if unpacking codes fails
From: Daniel Axtens
init_dynamic_block() didn't clean up gzio->tl and td in some error
paths. This left td pointing to part of tl. Then in grub_gzio_close(),
when tl was freed the storage for td would also be freed. The code then
attempts to free td explicitly, performing a UAF and then a double free.
Explicitly clean up tl and td in the error paths.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/io/gzio.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 4a8eaeae2..4236f0fd4 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -953,7 +953,7 @@ init_dynamic_block (grub_gzio_t gzio)
if ((unsigned) i + j > n)
{
grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "too many codes found");
- return;
+ goto fail;
}
while (j--)
ll[i++] = l;
@@ -966,7 +966,7 @@ init_dynamic_block (grub_gzio_t gzio)
if ((unsigned) i + j > n)
{
grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "too many codes found");
- return;
+ goto fail;
}
while (j--)
ll[i++] = 0;
@@ -981,7 +981,7 @@ init_dynamic_block (grub_gzio_t gzio)
if ((unsigned) i + j > n)
{
grub_error (GRUB_ERR_BAD_COMPRESSED_DATA, "too many codes found");
- return;
+ goto fail;
}
while (j--)
ll[i++] = 0;
@@ -1019,6 +1019,12 @@ init_dynamic_block (grub_gzio_t gzio)
/* indicate we're now working on a block */
gzio->code_state = 0;
gzio->block_len++;
+ return;
+
+ fail:
+ huft_free (gzio->tl);
+ gzio->td = NULL;
+ gzio->tl = NULL;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 052/117] gfxmenu/gui_list: Remove code that coverity is flagging as dead
From: Darren Kenny
The test of value for NULL before calling grub_strdup() is not required,
since the if condition prior to this has already tested for value being
NULL and cannot reach this code if it is.
Fixes: CID 73659
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/gfxmenu/gui_list.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/gfxmenu/gui_list.c b/grub-core/gfxmenu/gui_list.c
index 01477cdf2..df334a6c5 100644
--- a/grub-core/gfxmenu/gui_list.c
+++ b/grub-core/gfxmenu/gui_list.c
@@ -771,7 +771,7 @@ list_set_property (void *vself, const char *name, const
char *value)
{
self->need_to_recreate_boxes = 1;
grub_free (self->selected_item_box_pattern);
- self->selected_item_box_pattern = value ? grub_strdup (value) : 0;
+ self->selected_item_box_pattern = grub_strdup (value);
self->selected_item_box_pattern_inherit = 0;
}
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 073/117] fs/hfsplus: Don't use uninitialized data on corrupt filesystems
From: Daniel Axtens
Valgrind identified the following use of uninitialized data:
==2782220== Conditional jump or move depends on uninitialised value(s)
==2782220==at 0x42B364: grub_hfsplus_btree_search (hfsplus.c:566)
==2782220==by 0x42B21D: grub_hfsplus_read_block (hfsplus.c:185)
==2782220==by 0x42A693: grub_fshelp_read_file (fshelp.c:386)
==2782220==by 0x42C598: grub_hfsplus_read_file (hfsplus.c:219)
==2782220==by 0x42C598: grub_hfsplus_mount (hfsplus.c:330)
==2782220==by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
==2782220==by 0x4C1AE6: grub_fs_probe (fs.c:73)
==2782220==by 0x407C94: grub_ls_list_files (ls.c:186)
==2782220==by 0x407C94: grub_cmd_ls (ls.c:284)
==2782220==by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
==2782220==by 0x4045A6: execute_command (grub-fstest.c:59)
==2782220==by 0x4045A6: fstest (grub-fstest.c:433)
==2782220==by 0x4045A6: main (grub-fstest.c:772)
==2782220== Uninitialised value was created by a heap allocation
==2782220==at 0x483C7F3: malloc (in
/usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2782220==by 0x4C0305: grub_malloc (mm.c:42)
==2782220==by 0x42C21D: grub_hfsplus_mount (hfsplus.c:239)
==2782220==by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
==2782220==by 0x4C1AE6: grub_fs_probe (fs.c:73)
==2782220==by 0x407C94: grub_ls_list_files (ls.c:186)
==2782220==by 0x407C94: grub_cmd_ls (ls.c:284)
==2782220==by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
==2782220==by 0x4045A6: execute_command (grub-fstest.c:59)
==2782220==by 0x4045A6: fstest (grub-fstest.c:433)
==2782220==by 0x4045A6: main (grub-fstest.c:772)
This happens when the process of reading the catalog file goes sufficiently
wrong that there's an attempt to read the extent overflow file, which has
not yet been loaded. Keep track of when the extent overflow file is
fully loaded and refuse to use it before then.
The load valgrind doesn't like is btree->nodesize, and that's then used
to allocate a data structure. It looks like there are subsequently a lot
of reads based on that pointer so OOB reads are likely, and indeed crashes
(albeit difficult-to-replicate ones) have been observed in fuzzing.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/fs/hfsplus.c | 14 ++
include/grub/hfsplus.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 1c7791b02..361e5be49 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -177,6 +177,17 @@ grub_hfsplus_read_block (grub_fshelp_node_t node,
grub_disk_addr_t fileblock)
break;
}
+ /*
+ * If the extent overflow tree isn't ready yet, we can't look
+ * in it. This can happen where the catalog file is corrupted.
+ */
+ if (!node->data->extoverflow_tree_ready)
+ {
+ grub_error (GRUB_ERR_BAD_FS,
+ "attempted to read extent overflow tree before loading");
+ break;
+ }
+
/* Set up the key to look for in the extent overflow file. */
extoverflow.extkey.fileid = node->fileid;
extoverflow.extkey.type = 0;
@@ -241,6 +252,7 @@ grub_hfsplus_mount (grub_disk_t disk)
return 0;
data->disk = disk;
+ data->extoverflow_tree_ready = 0;
/* Read the bootblock. */
grub_disk_read (disk, GRUB_HFSPLUS_SBLOCK, 0, sizeof (volheader),
@@ -357,6 +369,8 @@ grub_hfsplus_mount (grub_disk_t disk)
if (data->extoverflow_tree.nodesize < 2)
goto fail;
+ data->extoverflow_tree_ready = 1;
+
if (grub_hfsplus_read_file (>attr_tree.file, 0, 0,
sizeof (struct grub_hfsplus_btnode),
sizeof (header), (char *) ) <= 0)
diff --git a/include/grub/hfsplus.h b/include/grub/hfsplus.h
index 117740ae2..e14dd31ff 100644
--- a/include/grub/hfsplus.h
+++ b/include/grub/hfsplus.h
@@ -113,6 +113,8 @@ struct grub_hfsplus_data
struct grub_hfsplus_btree extoverflow_tree;
struct grub_hfsplus_btree attr_tree;
+ int extoverflow_tree_ready;
+
struct grub_hfsplus_file dirroot;
struct grub_hfsplus_file opened_file;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 051/117] video/readers/jpeg: Test for an invalid next marker reference from a jpeg file
From: Darren Kenny
While it may never happen, and potentially could be caught at the end of
the function, it is worth checking up front for a bad reference to the
next marker just in case of a maliciously crafted file being provided.
Fixes: CID 73694
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/video/readers/jpeg.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c
index 31359a4c9..0b6ce3cee 100644
--- a/grub-core/video/readers/jpeg.c
+++ b/grub-core/video/readers/jpeg.c
@@ -253,6 +253,12 @@ grub_jpeg_decode_quan_table (struct grub_jpeg_data *data)
next_marker = data->file->offset;
next_marker += grub_jpeg_get_word (data);
+ if (next_marker > data->file->size)
+{
+ /* Should never be set beyond the size of the file. */
+ return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid next
reference");
+}
+
while (data->file->offset + sizeof (data->quan_table[id]) + 1
<= next_marker)
{
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 072/117] fs/hfsplus: Don't fetch a key beyond the end of the node
From: Daniel Axtens Otherwise you get a wild pointer, leading to a bunch of invalid reads. Check it falls inside the given node. Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/fs/hfsplus.c | 4 1 file changed, 4 insertions(+) diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c index 8fe7c12ed..1c7791b02 100644 --- a/grub-core/fs/hfsplus.c +++ b/grub-core/fs/hfsplus.c @@ -635,6 +635,10 @@ grub_hfsplus_btree_search (struct grub_hfsplus_btree *btree, pointer = ((char *) currkey + grub_be_to_cpu16 (currkey->keylen) + 2); + + if ((char *) pointer > node + btree->nodesize - 2) + return grub_error (GRUB_ERR_BAD_FS, "HFS+ key beyond end of node"); + currnode = grub_be_to_cpu32 (grub_get_unaligned32 (pointer)); match = 1; } -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 017/117] mmap: Fix memory leak when iterating over mapped memory
From: Darren Kenny
When returning from grub_mmap_iterate() the memory allocated to present
is not being released causing it to leak.
Fixes: CID 96655
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/mmap/mmap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/grub-core/mmap/mmap.c b/grub-core/mmap/mmap.c
index 3cae68364..c8c8312c5 100644
--- a/grub-core/mmap/mmap.c
+++ b/grub-core/mmap/mmap.c
@@ -270,6 +270,7 @@ grub_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
hook_data))
{
grub_free (ctx.scanline_events);
+ grub_free (present);
return GRUB_ERR_NONE;
}
@@ -282,6 +283,7 @@ grub_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
}
grub_free (ctx.scanline_events);
+ grub_free (present);
return GRUB_ERR_NONE;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 041/117] libgcrypt/mpi: Fix possible unintended sign extension
From: Darren Kenny The array of unsigned char gets promoted to a signed 32-bit int before it is finally promoted to a size_t. There is the possibility that this may result in the signed-bit being set for the intermediate signed 32-bit int. We should ensure that the promotion is to the correct type before we bitwise-OR the values. Fixes: CID 96697 Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper --- grub-core/lib/libgcrypt/mpi/mpicoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grub-core/lib/libgcrypt/mpi/mpicoder.c b/grub-core/lib/libgcrypt/mpi/mpicoder.c index a3435ed14..7ecad27b2 100644 --- a/grub-core/lib/libgcrypt/mpi/mpicoder.c +++ b/grub-core/lib/libgcrypt/mpi/mpicoder.c @@ -458,7 +458,7 @@ gcry_mpi_scan (struct gcry_mpi **ret_mpi, enum gcry_mpi_format format, if (len && len < 4) return gcry_error (GPG_ERR_TOO_SHORT); - n = (s[0] << 24 | s[1] << 16 | s[2] << 8 | s[3]); + n = ((size_t)s[0] << 24 | (size_t)s[1] << 16 | (size_t)s[2] << 8 | (size_t)s[3]); s += 4; if (len) len -= 4; -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 066/117] kern/misc: Always set *end in grub_strtoull()
From: Daniel Axtens
Currently, if there is an error in grub_strtoull(), *end is not set.
This differs from the usual behavior of strtoull(), and also means that
some callers may use an uninitialized value for *end.
Set *end unconditionally.
Signed-off-by: Daniel Axtens
Reviewed-by: Daniel Kiper
---
grub-core/kern/misc.c | 8
1 file changed, 8 insertions(+)
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index a278e069b..430e72340 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -419,6 +419,10 @@ grub_strtoull (const char * restrict str, const char **
const restrict end,
{
grub_error (GRUB_ERR_OUT_OF_RANGE,
N_("overflow is detected"));
+
+ if (end)
+*end = (char *) str;
+
return ~0ULL;
}
@@ -430,6 +434,10 @@ grub_strtoull (const char * restrict str, const char **
const restrict end,
{
grub_error (GRUB_ERR_BAD_NUMBER,
N_("unrecognized number"));
+
+ if (end)
+*end = (char *) str;
+
return 0;
}
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 035/117] hfsplus: Check that the volume name length is valid
From: Darren Kenny
HFS+ documentation suggests that the maximum filename and volume name is
255 Unicode characters in length.
So, when converting from big-endian to little-endian, we should ensure
that the name of the volume has a length that is between 0 and 255,
inclusive.
Fixes: CID 73641
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/fs/hfsplus.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 9c4e4c88c..8fe7c12ed 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -1012,6 +1012,15 @@ grub_hfsplus_label (grub_device_t device, char **label)
grub_hfsplus_btree_recptr (>catalog_tree, node, ptr);
label_len = grub_be_to_cpu16 (catkey->namelen);
+
+ /* Ensure that the length is >= 0. */
+ if (label_len < 0)
+label_len = 0;
+
+ /* Ensure label length is at most 255 Unicode characters. */
+ if (label_len > 255)
+label_len = 255;
+
label_name = grub_calloc (label_len, sizeof (*label_name));
if (!label_name)
{
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 065/117] commands/menuentry: Fix quoting in setparams_prefix()
From: Daniel Axtens Commit 9acdcbf32542 (use single quotes in menuentry setparams command) says that expressing a quoted single quote will require 3 characters. It actually requires (and always did require!) 4 characters: str: a'b => a'\''b len: 3 => 6 (2 for the letters + 4 for the quote) This leads to not allocating enough memory and thus out of bounds writes that have been observed to cause heap corruption. Allocate 4 bytes for each single quote. Commit 22e7dbb2bb81 (Fix quoting in legacy parser.) does the same quoting, but it adds 3 as extra overhead on top of the single byte that the quote already needs. So it's correct. Fixes: 9acdcbf32542 (use single quotes in menuentry setparams command) Fixes: CVE-2021-20233 Reported-by: Daniel Axtens Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper --- grub-core/commands/menuentry.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grub-core/commands/menuentry.c b/grub-core/commands/menuentry.c index 9164df744..720e6d8ea 100644 --- a/grub-core/commands/menuentry.c +++ b/grub-core/commands/menuentry.c @@ -230,7 +230,7 @@ setparams_prefix (int argc, char **args) len += 3; /* 3 = 1 space + 2 quotes */ p = args[i]; while (*p) - len += (*p++ == '\'' ? 3 : 1); + len += (*p++ == '\'' ? 4 : 1); } result = grub_malloc (len + 2); -- 2.11.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 031/117] disk/ldm: Make sure comp data is freed before exiting from make_vg()
From: Marco A Benatto
Several error handling paths in make_vg() do not free comp data before
jumping to fail2 label and returning from the function. This will leak
memory. So, let's fix all issues of that kind.
Fixes: CID 73804
Signed-off-by: Marco A Benatto
Reviewed-by: Daniel Kiper
---
grub-core/disk/ldm.c | 51 ---
1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
index 912e88255..48942549a 100644
--- a/grub-core/disk/ldm.c
+++ b/grub-core/disk/ldm.c
@@ -554,7 +554,11 @@ make_vg (grub_disk_t disk,
comp->segments = grub_calloc (comp->segment_alloc,
sizeof (*comp->segments));
if (!comp->segments)
- goto fail2;
+ {
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
}
else
{
@@ -562,7 +566,11 @@ make_vg (grub_disk_t disk,
comp->segment_count = 1;
comp->segments = grub_malloc (sizeof (*comp->segments));
if (!comp->segments)
- goto fail2;
+ {
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
comp->segments->start_extent = 0;
comp->segments->extent_count = lv->size;
comp->segments->layout = 0;
@@ -574,15 +582,26 @@ make_vg (grub_disk_t disk,
comp->segments->layout = GRUB_RAID_LAYOUT_SYMMETRIC_MASK;
}
else
- goto fail2;
+ {
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
ptr += *ptr + 1;
ptr++;
if (!(vblk[i].flags & 0x10))
- goto fail2;
+ {
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic)
|| ptr + *ptr + 1 >= vblk[i].dynamic
+ sizeof (vblk[i].dynamic))
{
+ grub_free (comp->segments);
grub_free (comp->internal_id);
grub_free (comp);
goto fail2;
@@ -592,6 +611,7 @@ make_vg (grub_disk_t disk,
if (ptr + *ptr + 1 >= vblk[i].dynamic
+ sizeof (vblk[i].dynamic))
{
+ grub_free (comp->segments);
grub_free (comp->internal_id);
grub_free (comp);
goto fail2;
@@ -601,7 +621,12 @@ make_vg (grub_disk_t disk,
comp->segments->nodes = grub_calloc (comp->segments->node_alloc,
sizeof
(*comp->segments->nodes));
if (!lv->segments->nodes)
- goto fail2;
+ {
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
}
if (lv->segments->node_alloc == lv->segments->node_count)
@@ -611,11 +636,23 @@ make_vg (grub_disk_t disk,
if (grub_mul (lv->segments->node_alloc, 2,
>segments->node_alloc) ||
grub_mul (lv->segments->node_alloc, sizeof
(*lv->segments->nodes), ))
- goto fail2;
+ {
+ grub_free (comp->segments->nodes);
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
t = grub_realloc (lv->segments->nodes, sz);
if (!t)
- goto fail2;
+ {
+ grub_free (comp->segments->nodes);
+ grub_free (comp->segments);
+ grub_free (comp->internal_id);
+ grub_free (comp);
+ goto fail2;
+ }
lv->segments->nodes = t;
}
lv->segments->nodes[lv->segments->node_count].pv = 0;
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 030/117] kern/partition: Check for NULL before dereferencing input string
From: Darren Kenny
There is the possibility that the value of str comes from an external
source and continuing to use it before ever checking its validity is
wrong. So, needs fixing.
Additionally, drop unneeded part initialization.
Fixes: CID 292444
Signed-off-by: Darren Kenny
Reviewed-by: Daniel Kiper
---
grub-core/kern/partition.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/partition.c b/grub-core/kern/partition.c
index 2c401b866..3068c4dca 100644
--- a/grub-core/kern/partition.c
+++ b/grub-core/kern/partition.c
@@ -109,11 +109,14 @@ grub_partition_map_probe (const grub_partition_map_t
partmap,
grub_partition_t
grub_partition_probe (struct grub_disk *disk, const char *str)
{
- grub_partition_t part = 0;
+ grub_partition_t part;
grub_partition_t curpart = 0;
grub_partition_t tail;
const char *ptr;
+ if (str == NULL)
+return 0;
+
part = tail = disk->partition;
for (ptr = str; *ptr;)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[SECURITY PATCH 013/117] loader/xnu: Don't allow loading extension and packages when locked down
From: Javier Martinez Canillas
The shim_lock verifier validates the XNU kernels but no its extensions
and packages. Prevent these to be loaded when the GRUB is locked down.
Signed-off-by: Javier Martinez Canillas
Reviewed-by: Daniel Kiper
---
grub-core/loader/xnu.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/grub-core/loader/xnu.c b/grub-core/loader/xnu.c
index 9ae4ceb35..44fd5a979 100644
--- a/grub-core/loader/xnu.c
+++ b/grub-core/loader/xnu.c
@@ -1485,20 +1485,23 @@ GRUB_MOD_INIT(xnu)
N_("Load XNU image."));
cmd_kernel64 = grub_register_command ("xnu_kernel64", grub_cmd_xnu_kernel64,
0, N_("Load 64-bit XNU image."));
- cmd_mkext = grub_register_command ("xnu_mkext", grub_cmd_xnu_mkext, 0,
-N_("Load XNU extension package."));
- cmd_kext = grub_register_command ("xnu_kext", grub_cmd_xnu_kext, 0,
- N_("Load XNU extension."));
- cmd_kextdir = grub_register_command ("xnu_kextdir", grub_cmd_xnu_kextdir,
- /* TRANSLATORS: OSBundleRequired is a
- variable name in xnu extensions
- manifests. It behaves mostly like
- GNU/Linux runlevels.
- */
- N_("DIRECTORY [OSBundleRequired]"),
- /* TRANSLATORS: There are many extensions
- in extension directory. */
- N_("Load XNU extension directory."));
+ cmd_mkext = grub_register_command_lockdown ("xnu_mkext", grub_cmd_xnu_mkext,
0,
+ N_("Load XNU extension
package."));
+ cmd_kext = grub_register_command_lockdown ("xnu_kext", grub_cmd_xnu_kext, 0,
+N_("Load XNU extension."));
+ cmd_kextdir = grub_register_command_lockdown ("xnu_kextdir",
grub_cmd_xnu_kextdir,
+ /*
+* TRANSLATORS:
OSBundleRequired is
+* a variable name in xnu
extensions
+* manifests. It behaves mostly
like
+* GNU/Linux runlevels.
+*/
+ N_("DIRECTORY
[OSBundleRequired]"),
+ /*
+* TRANSLATORS: There are many
extensions
+* in extension directory.
+*/
+ N_("Load XNU extension
directory."));
cmd_ramdisk = grub_register_command ("xnu_ramdisk", grub_cmd_xnu_ramdisk, 0,
/* TRANSLATORS: ramdisk here isn't identifier. It can be translated. */
N_("Load XNU ramdisk. "
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
