Re: Linux DRTM on UEFI platforms
On 7/5/22 20:03, Brendan Trotter wrote: Hi, Greetings! Not sure why I got dropped from distro, but no worries. On Wed, Jul 6, 2022 at 4:52 AM Daniel P. Smith wrote: On 6/10/22 12:40, Ard Biesheuvel wrote:> On Thu, 19 May 2022 at 22:59, To help provide clarity, consider the following flows for comparison, Normal/existing efi-stub: EFI -> efi-stub -> head_64.S Proposed secure launch: EFI -> efi-stub -> dl-handler -> [cpu] -> sl_stub ->head_64.S For more clarity; the entire point is to ensure that the kernel only has to trust itself and the CPU/TPM hardware (and does not have to trust a potentially malicious boot loader)..Any attempt to avoid a one-off solution for Linux is an attempt to weaken security. Please elaborate so I might understand how this entrypoint allows for the kernel to only trust itself and the CPU/TPM. The only correct approach is "efi-stub -> head_64.S -> kernel's own secure init"; where (on UEFI systems) neither GRUB nor Trenchboot has a valid reason to exist and should never be installed. Cheers, Brendan v/r, dps ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] configure.ac: warn if stack-protector not allowed
On Wed, Jul 06, 2022 at 03:25:58AM -0400, Nicholas Vinson wrote: > Introduce ERROR_PLATFORM_NOT_SUPPORT_SSP environment variable to treat > the '--enable-stack-protector is only supported on EFI platforms' > message as a warning instead of an error. If > ERROR_PLATFORM_NOT_SUPPORT_SSP is set to 'no' (case-insensitive), then > the message will be printed as a warning. Otherwise, it prints as an > error. The default behavior is to print the message as an error. > > For any wrapper build script that has some variation of: > > for p in SELECTED_GRUB_PLATFORMS; do\ > configure --enable-stack-protector \ > --with-platform${P} ... || die; \ > done > make > > GRUB will fail to build if SELECTED_GRUB_PLATFORMS contains a platform > that does not support SSP. > > Such wrapper scripts need to work-around this issue by modifying the > above for-loop, so it conditionally passes '--enable-stack-protector' to > configure for the proper GRUB platform(s). > > However, if the above example is modified to have to conditionally pass > in --enable-stack-protector, its behavior is effectively the same as the > proposed change. Additionally, The list of SSP supported platforms is > now in 2 places. One in the configure script and one in the build > wrapper script. If the second list is not properly maintained it could > mistakenly disable SSP for a platform that later gained support for it. > > Signed-off-by: Nicholas Vinson Reviewed-by: Daniel Kiper But one nit below... > --- > configure.ac | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/configure.ac b/configure.ac > index 57fb70945..a08f3285e 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -36,6 +36,10 @@ dnl description of the relationships between them. > > AC_INIT([GRUB],[2.11],[bug-g...@gnu.org]) > > +AS_CASE(["$ERROR_PLATFORM_NOT_SUPPORT_SSP"], > + [n | no | nO | N | No | NO], [ERROR_PLATFORM_NOT_SUPPORT_SSP=no], > + [ERROR_PLATFORM_NOT_SUPPORT_SSP=yes]) > + > # We don't want -g -O2 by default in CFLAGS > : ${CFLAGS=""} > > @@ -1342,6 +1346,7 @@ AC_ARG_ENABLE([stack-protector], >[enable the stack protector]), > [], > [enable_stack_protector=no]) > + I think this change can be dropped... > if test "x$enable_stack_protector" = xno; then >if test "x$ssp_possible" = xyes; then > # Need that, because some distributions ship compilers that include > @@ -1349,7 +1354,12 @@ if test "x$enable_stack_protector" = xno; then > 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]) > + if test "$ERROR_PLATFORM_NOT_SUPPORT_SSP" = "yes"; then > +AC_MSG_ERROR([--enable-stack-protector is only supported on EFI > platforms]) > + else > +AC_MSG_WARN([--enable-stack-protector is only supported on EFI > platforms]) > + fi > + enable_stack_protector=no > 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 Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Relax memory constraints on required_pages for EFI boot
Hi, On Sun, Jan 17, 2021 at 02:43:50PM -0800, Hanson Char via Grub-devel wrote: > (Finally figured out how to git send-email on this patch properly.) > > As reported earlier, when booted in UEFI mode, Grub would fail to load a > ramdisk of size larger than "(total_pages >> 2)" with > > "error: out of memory" > > (https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/efi/mm.c#n616) > > Further investigation into the EFI memory map indicates the current limit of > MAX_HEAP_MEMORY and the use of a quarter of the total_pages returned from EFI > available memory (type 7) as the default seems arbitrary and unnecessary. > > Therefore this proposed patch removes the aribrary limit, and lets Grub make > full use of the EFI available memory reported by the BIOS. > > The patch has been successfully tested to load large ramdisk with size that > would otherwise fail. I am not sure you still care about the issue but I want to give you heads up we have just merged alternative solution into the GRUB upstream which should solve your problem. It would be nice if you could test it and tell us how it works for you. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH RESEND v3 1/2] font: Add font scaling feature to grub_font_draw_glyph()
On Mon, Jun 27, 2022 at 05:35:24PM +0800, Zhang Boyang wrote: > This patch adds an argument 'scale' to grub_font_draw_glyph(). If > scale > 1, then the function will create a new scaled bitmap of the > drawing glyph, and draws the scaled glyph. The scaled bitmap is cached > in the glyph itself, so it can be reused if same glyph is used many > times. > > To avoid interger overflow during scaling, this patch intruduces > dimension limits to font files, described by GRUB_FONT_MAX_DIMENSION. > The scaled glyph should also obey this limit. In addition, scale factor > is also limited by GRUB_FONT_MAX_SCALE. If you use overflow aware math operators defined as macros, e.g. grub_mul(), defined in include/grub/safemath.h probably you can drop these artificial limits. > Signed-off-by: Zhang Boyang > --- > grub-core/commands/videotest.c | 4 +- > grub-core/font/font.c | 120 ++--- > grub-core/gfxmenu/font.c | 2 +- > grub-core/term/gfxterm.c | 2 +- > include/grub/font.h| 16 - > 5 files changed, 131 insertions(+), 13 deletions(-) > > diff --git a/grub-core/commands/videotest.c b/grub-core/commands/videotest.c > index ac145afc2..d95ee411d 100644 > --- a/grub-core/commands/videotest.c > +++ b/grub-core/commands/videotest.c > @@ -87,7 +87,7 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ > ((unused)), >return grub_error (GRUB_ERR_BAD_FONT, "no font loaded"); > > glyph = grub_font_get_glyph (fixed, '*'); > -grub_font_draw_glyph (glyph, color, 200 ,0); > +grub_font_draw_glyph (glyph, color, 200, 0, 1); > > color = grub_video_map_rgb (255, 255, 255); > > @@ -148,7 +148,7 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ > ((unused)), >{ > color = grub_video_map_color (i); > palette[i] = color; > - grub_font_draw_glyph (glyph, color, 16 + i * 16, 220); > + grub_font_draw_glyph (glyph, color, 16 + i * 16, 220, 1); >} >} > > diff --git a/grub-core/font/font.c b/grub-core/font/font.c > index 42189c325..46ffec84c 100644 > --- a/grub-core/font/font.c > +++ b/grub-core/font/font.c > @@ -137,6 +137,8 @@ ascii_glyph_lookup (grub_uint32_t code) > ascii_font_glyph[current]->offset_x = 0; > ascii_font_glyph[current]->offset_y = -2; > ascii_font_glyph[current]->device_width = 8; > + ascii_font_glyph[current]->scaled_bitmap = NULL; > + ascii_font_glyph[current]->scale = 0; > ascii_font_glyph[current]->font = NULL; > > grub_memcpy (ascii_font_glyph[current]->bitmap, > @@ -172,6 +174,8 @@ grub_font_loader_init (void) >unknown_glyph->offset_x = 0; >unknown_glyph->offset_y = -3; >unknown_glyph->device_width = 8; > + unknown_glyph->scaled_bitmap = NULL; > + unknown_glyph->scale = 0; >grub_memcpy (unknown_glyph->bitmap, > unknown_glyph_bitmap, sizeof (unknown_glyph_bitmap)); > > @@ -646,6 +650,20 @@ grub_font_load (const char *filename) >goto fail; > } > > + if (font->max_char_width <= 0 > + || font->max_char_width > GRUB_FONT_MAX_DIMENSION > + || font->max_char_height <= 0 > + || font->max_char_height > GRUB_FONT_MAX_DIMENSION > + || font->ascent <= 0 > + || font->ascent > GRUB_FONT_MAX_DIMENSION > + || font->descent <= 0 > + || font->descent > GRUB_FONT_MAX_DIMENSION) > +{ > + grub_error (GRUB_ERR_BAD_FONT, > + "invalid font file: bad font metrics"); grub_error (GRUB_ERR_BAD_FONT, N_("invalid font file: bad font metrics")); Every grub_error() message has to be processed by N_() macro. And yes, there is no space after N_ and before "(". Additionally, I am OK with lines a bit longer than 80 chars. So, you can put grub_error() call in one line here and below. > + goto fail; > +} > + >/* Add the font to the global font registry. */ >if (register_font (font) != 0) > goto fail; > @@ -738,7 +756,7 @@ grub_font_get_glyph_internal (grub_font_t font, > grub_uint32_t code) >grub_uint16_t height; >grub_int16_t xoff; >grub_int16_t yoff; > - grub_int16_t dwidth; > + grub_uint16_t dwidth; >int len; > >if (index_entry->glyph) > @@ -760,7 +778,18 @@ grub_font_get_glyph_internal (grub_font_t font, > grub_uint32_t code) > || read_be_uint16 (font->file, &height) != 0 > || read_be_int16 (font->file, &xoff) != 0 > || read_be_int16 (font->file, &yoff) != 0 > - || read_be_int16 (font->file, &dwidth) != 0) > + || read_be_uint16 (font->file, &dwidth) != 0) > + { > + remove_font (font); > + return 0; > + } > + > + /* Reject illegal glyphs. */ > + if (width > font->max_char_width > + || height > font->max_char_height > + || xoff < -GRUB_FONT_MAX_DIMENSION || xoff > GRUB_FONT_MAX_DIMENSION > + || yoff < -GRUB_FONT_MAX_DIMENSION || yoff > GRUB_FONT_MAX_DIMENSION > + || dwidth > GRUB_FONT_MAX_DIME
Re: [PATCH RESEND v3 2/2] term/gfxterm: Preliminary HiDPI support
On Mon, Jun 27, 2022 at 05:35:25PM +0800, Zhang Boyang wrote: > Currently GRUB's default font is too small to see on a HiDPI monitor. > This patch adds preliminary HiDPI support to gfxterm. It introduces a > new environment variable 'gfxterm_scale'. If set to 0, and a high > resolution monitor is detected, it will scale the font size > automatically. If set to other number, that number will be the scale > factor, overriding automatic scale factor calculation. > > Signed-off-by: Zhang Boyang > --- > docs/grub.texi | 11 ++ > grub-core/gfxmenu/view.c | 1 + > grub-core/term/gfxterm.c | 72 > include/grub/gfxterm.h | 3 +- > 4 files changed, 64 insertions(+), 23 deletions(-) > > diff --git a/docs/grub.texi b/docs/grub.texi > index 9b902273c..8f82061f6 100644 > --- a/docs/grub.texi > +++ b/docs/grub.texi > @@ -3274,6 +3274,7 @@ These variables have special meaning to GRUB. > * gfxmode:: > * gfxpayload:: > * gfxterm_font:: > +* gfxterm_scale:: > * grub_cpu:: > * grub_platform:: > * icondir:: > @@ -3548,6 +3549,16 @@ If this variable is set, it names a font to use for > text on the > available font. > > > +@node gfxterm_scale > +@subsection gfxterm_scale > + > +If this variable is not set, or set to @samp{0}, the @samp{gfxterm} > +graphical terminal will scale the font automatically when a high resolution > +monitor is detected. If set to other number, the font scale factor will be > +forced to that number. Set this to @samp{1} if user don't want > +@samp{gfxterm} to scale the font on screen. > + > + > @node grub_cpu > @subsection grub_cpu > > diff --git a/grub-core/gfxmenu/view.c b/grub-core/gfxmenu/view.c > index 6358004b2..94b9ef4db 100644 > --- a/grub-core/gfxmenu/view.c > +++ b/grub-core/gfxmenu/view.c > @@ -546,6 +546,7 @@ init_terminal (grub_gfxmenu_view_t view) > view->terminal_rect.height, > view->double_repaint, > terminal_font, > + 1, > view->terminal_border); >grub_gfxterm_decorator_hook = grub_gfxmenu_draw_terminal_box; > } > diff --git a/grub-core/term/gfxterm.c b/grub-core/term/gfxterm.c > index 4512dee6f..df2d3f86b 100644 > --- a/grub-core/term/gfxterm.c > +++ b/grub-core/term/gfxterm.c > @@ -82,6 +82,7 @@ struct grub_virtual_screen > >/* Font settings. */ >grub_font_t font; > + unsigned int scale; > >/* Terminal color settings. */ >grub_uint8_t standard_color_setting; > @@ -204,7 +205,7 @@ grub_virtual_screen_free (void) > static grub_err_t > grub_virtual_screen_setup (unsigned int x, unsigned int y, > unsigned int width, unsigned int height, > -grub_font_t font) > +grub_font_t font, unsigned int scale) > { >unsigned int i; > > @@ -213,6 +214,7 @@ grub_virtual_screen_setup (unsigned int x, unsigned int y, > >/* Initialize with default data. */ >virtual_screen.font = font; > + virtual_screen.scale = scale; >virtual_screen.width = width; >virtual_screen.height = height; >virtual_screen.offset_x = x; > @@ -220,9 +222,9 @@ grub_virtual_screen_setup (unsigned int x, unsigned int y, >virtual_screen.normal_char_width = > calculate_normal_character_width (virtual_screen.font); >virtual_screen.normal_char_height = > -grub_font_get_max_char_height (virtual_screen.font); > +grub_font_get_max_char_height (virtual_screen.font) * > virtual_screen.scale; Another good place for safe math macro... >if (virtual_screen.normal_char_height == 0) > -virtual_screen.normal_char_height = 16; > +virtual_screen.normal_char_height = 16 * virtual_screen.scale; ... and here and below... >virtual_screen.cursor_x = 0; >virtual_screen.cursor_y = 0; >virtual_screen.cursor_state = 1; > @@ -297,7 +299,8 @@ grub_err_t > grub_gfxterm_set_window (struct grub_video_render_target *target, >int x, int y, int width, int height, >int double_repaint, > - grub_font_t font, int border_width) > + grub_font_t font, int scale, > + int border_width) > { >/* Clean up any prior instance. */ >destroy_window (); > @@ -306,10 +309,10 @@ grub_gfxterm_set_window (struct > grub_video_render_target *target, >render_target = target; > >/* Create virtual screen. */ > - if (grub_virtual_screen_setup (border_width, border_width, > - width - 2 * border_width, > - height - 2 * border_width, > - font) > + if (grub_virtual_screen_setup (border_width * scale, border_width * scale, > + width - 2 * border_width * scale, > + height - 2 * border_width * scale, > + f
[PATCH v3 1/1] mkfont: Fix tainted loop boundary issues with substitutions
With gsub substitutions the offsets should be validated against the the number of glyphs in a font face and the memory allocated for the gsub substitution data. Both the number of glyphs and the last address in the allocated data are passed in to process_cursive(), where the number of glyphs validates the end of the range. Enabling memory allocation validation uses two macros, one to simply check the address against the allocated space, and the other to check that the number of items of a given size doesn't extend outside of the allocated space. Fixes: CID 73770 Fixes: CID 314040 Signed-off-by: Darren Kenny --- v2 -> v3: Updated to resolve style issues around the spacing when calling macros. util/grub-mkfont.c | 62 +++--- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/util/grub-mkfont.c b/util/grub-mkfont.c index d0e5ec564f03..5f4130951eb1 100644 --- a/util/grub-mkfont.c +++ b/util/grub-mkfont.c @@ -67,6 +67,11 @@ #define GRUB_FONT_RANGE_BLOCK 1024 +#define GSUB_PTR_VALID(x, end) assert((grub_uint8_t*)(x) <= (end)) + +#define GSUB_ARRAY_SIZE_VALID(a, sz, end) \ +assert((sz) >= 0 && ((sz) <= ((end) - (grub_uint8_t*)(a)) / sizeof(*(a + struct grub_glyph_info { struct grub_glyph_info *next; @@ -487,16 +492,22 @@ subst (const struct gsub_substitution *sub, grub_uint32_t glyph, static void process_cursive (struct gsub_feature *feature, struct gsub_lookup_list *lookups, -grub_uint32_t feattag) +grub_uint32_t feattag, +grub_uint32_t num_glyphs, +grub_uint8_t *gsub_end) { int j, k; int i; + int lookup_count = grub_be_to_cpu16 (feature->lookupcount); struct glyph_replace **target = NULL; struct gsub_substitution *sub; - for (j = 0; j < grub_be_to_cpu16 (feature->lookupcount); j++) + GSUB_ARRAY_SIZE_VALID (feature->lookupindices, lookup_count, gsub_end); + + for (j = 0; j < lookup_count; j++) { int lookup_index = grub_be_to_cpu16 (feature->lookupindices[j]); + int sub_count; struct gsub_lookup *lookup; if (lookup_index >= grub_be_to_cpu16 (lookups->count)) { @@ -538,7 +549,12 @@ process_cursive (struct gsub_feature *feature, target = &subst_medijoin; break; } - for (k = 0; k < grub_be_to_cpu16 (lookup->subtablecount); k++) + + sub_count = grub_be_to_cpu16 (lookup->subtablecount); + + GSUB_ARRAY_SIZE_VALID (lookup->subtables, sub_count, gsub_end); + + for (k = 0; k < sub_count; k++) { sub = (struct gsub_substitution *) ((grub_uint8_t *) lookup + grub_be_to_cpu16 (lookup->subtables[k])); @@ -559,18 +575,33 @@ process_cursive (struct gsub_feature *feature, if (covertype == GSUB_COVERAGE_LIST) { struct gsub_coverage_list *cover = coverage; + int count = grub_be_to_cpu16 (cover->count); int l; - for (l = 0; l < grub_be_to_cpu16 (cover->count); l++) + + GSUB_ARRAY_SIZE_VALID (cover->glyphs, count, gsub_end); + + for (l = 0; l < count; l++) subst (sub, grub_be_to_cpu16 (cover->glyphs[l]), target, &i); } else if (covertype == GSUB_COVERAGE_RANGE) { struct gsub_coverage_ranges *cover = coverage; + int count = grub_be_to_cpu16 (cover->count); int l, m; - for (l = 0; l < grub_be_to_cpu16 (cover->count); l++) - for (m = grub_be_to_cpu16 (cover->ranges[l].start); -m <= grub_be_to_cpu16 (cover->ranges[l].end); m++) - subst (sub, m, target, &i); + + GSUB_ARRAY_SIZE_VALID (cover->ranges, count, gsub_end); + + for (l = 0; l < count; l++) + { + grub_uint16_t start = grub_be_to_cpu16 (cover->ranges[l].start); + grub_uint16_t end = grub_be_to_cpu16 (cover->ranges[l].end); + + if (start > end || end > num_glyphs) + grub_util_error ("%s", _("invalid font range")); + + for (m = start; m <= end; m++) + subst (sub, m, target, &i); + } } else /* TRANSLATORS: most font transformations apply only to @@ -589,6 +620,7 @@ add_font (struct grub_font_info *font_info, FT_Face face, int nocut) { struct gsub_header *gsub = NULL; FT_ULong gsub_len = 0; + grub_uint8_t *gsub_end = NULL; if (!FT_Load_Sfnt_Table (face, TTAG_GSUB, 0, NULL, &gsub_len)) { @@ -600,6 +632,9 @@ add_font (struct grub_font_info *font_info, FT_Face face, int nocut) gsub_len = 0; } } + + gsub_end = (grub_uint8_t *) gsub + gsub_len; + if (gsub) { struct gsub_features *features @@ -610,6 +645,11 @@ add_font (struct grub_font_info *font_info, FT_Face face, int nocut)
Re: [PATCH v2] configure.ac: warn if stack-protector not allowed
On 7/7/22 09:17, Daniel Kiper wrote: On Wed, Jul 06, 2022 at 03:25:58AM -0400, Nicholas Vinson wrote: Introduce ERROR_PLATFORM_NOT_SUPPORT_SSP environment variable to treat the '--enable-stack-protector is only supported on EFI platforms' message as a warning instead of an error. If ERROR_PLATFORM_NOT_SUPPORT_SSP is set to 'no' (case-insensitive), then the message will be printed as a warning. Otherwise, it prints as an error. The default behavior is to print the message as an error. For any wrapper build script that has some variation of: for p in SELECTED_GRUB_PLATFORMS; do\ configure --enable-stack-protector \ --with-platform${P} ... || die; \ done make GRUB will fail to build if SELECTED_GRUB_PLATFORMS contains a platform that does not support SSP. Such wrapper scripts need to work-around this issue by modifying the above for-loop, so it conditionally passes '--enable-stack-protector' to configure for the proper GRUB platform(s). However, if the above example is modified to have to conditionally pass in --enable-stack-protector, its behavior is effectively the same as the proposed change. Additionally, The list of SSP supported platforms is now in 2 places. One in the configure script and one in the build wrapper script. If the second list is not properly maintained it could mistakenly disable SSP for a platform that later gained support for it. Signed-off-by: Nicholas Vinson Reviewed-by: Daniel Kiper But one nit below... --- configure.ac | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 57fb70945..a08f3285e 100644 --- a/configure.ac +++ b/configure.ac @@ -36,6 +36,10 @@ dnl description of the relationships between them. AC_INIT([GRUB],[2.11],[bug-g...@gnu.org]) +AS_CASE(["$ERROR_PLATFORM_NOT_SUPPORT_SSP"], + [n | no | nO | N | No | NO], [ERROR_PLATFORM_NOT_SUPPORT_SSP=no], + [ERROR_PLATFORM_NOT_SUPPORT_SSP=yes]) + # We don't want -g -O2 by default in CFLAGS : ${CFLAGS=""} @@ -1342,6 +1346,7 @@ AC_ARG_ENABLE([stack-protector], [enable the stack protector]), [], [enable_stack_protector=no]) + I think this change can be dropped... Agreed. It's an errant white space only change. Unless someone says otherwise, I'll regenerate the request with this change removed. Thanks, Nicholas Vinson if test "x$enable_stack_protector" = xno; then if test "x$ssp_possible" = xyes; then # Need that, because some distributions ship compilers that include @@ -1349,7 +1354,12 @@ if test "x$enable_stack_protector" = xno; then 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]) + if test "$ERROR_PLATFORM_NOT_SUPPORT_SSP" = "yes"; then +AC_MSG_ERROR([--enable-stack-protector is only supported on EFI platforms]) + else +AC_MSG_WARN([--enable-stack-protector is only supported on EFI platforms]) + fi + enable_stack_protector=no 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 Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3 1/1] mkfont: Fix tainted loop boundary issues with substitutions
On Thu, Jul 07, 2022 at 03:34:38PM +, Darren Kenny wrote: > With gsub substitutions the offsets should be validated against the > the number of glyphs in a font face and the memory allocated for the gsub > substitution data. > > Both the number of glyphs and the last address in the allocated data are > passed in to process_cursive(), where the number of glyphs validates the end > of the range. > > Enabling memory allocation validation uses two macros, one to simply check the > address against the allocated space, and the other to check that the number of > items of a given size doesn't extend outside of the allocated space. > > Fixes: CID 73770 > Fixes: CID 314040 > > Signed-off-by: Darren Kenny Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v3] configure.ac: warn if stack-protector not allowed
Introduce ERROR_PLATFORM_NOT_SUPPORT_SSP environment variable to treat the '--enable-stack-protector is only supported on EFI platforms' message as a warning instead of an error. If ERROR_PLATFORM_NOT_SUPPORT_SSP is set to 'no' (case-insensitive), then the message will be printed as a warning. Otherwise, it prints as an error. The default behavior is to print the message as an error. For any wrapper build script that has some variation of: for p in SELECTED_GRUB_PLATFORMS; do\ configure --enable-stack-protector \ --with-platform${P} ... || die; \ done make GRUB will fail to build if SELECTED_GRUB_PLATFORMS contains a platform that does not support SSP. Such wrapper scripts need to work-around this issue by modifying the above for-loop, so it conditionally passes '--enable-stack-protector' to configure for the proper GRUB platform(s). However, if the above example is modified to have to conditionally pass in --enable-stack-protector, its behavior is effectively the same as the proposed change. Additionally, The list of SSP supported platforms is now in 2 places. One in the configure script and one in the build wrapper script. If the second list is not properly maintained it could mistakenly disable SSP for a platform that later gained support for it. Signed-off-by: Nicholas Vinson --- configure.ac | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 57fb70945..90f686f79 100644 --- a/configure.ac +++ b/configure.ac @@ -36,6 +36,10 @@ dnl description of the relationships between them. AC_INIT([GRUB],[2.11],[bug-g...@gnu.org]) +AS_CASE(["$ERROR_PLATFORM_NOT_SUPPORT_SSP"], + [n | no | nO | N | No | NO], [ERROR_PLATFORM_NOT_SUPPORT_SSP=no], + [ERROR_PLATFORM_NOT_SUPPORT_SSP=yes]) + # We don't want -g -O2 by default in CFLAGS : ${CFLAGS=""} @@ -1349,7 +1353,12 @@ if test "x$enable_stack_protector" = xno; then 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]) + if test "$ERROR_PLATFORM_NOT_SUPPORT_SSP" = "yes"; then +AC_MSG_ERROR([--enable-stack-protector is only supported on EFI platforms]) + else +AC_MSG_WARN([--enable-stack-protector is only supported on EFI platforms]) + fi + enable_stack_protector=no 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 -- 2.35.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Linux DRTM on UEFI platforms
Hi, On Thu, Jul 7, 2022 at 7:18 PM Daniel P. Smith wrote: > On 7/5/22 20:03, Brendan Trotter wrote: > Greetings! > > Not sure why I got dropped from distro, but no worries. > > > On Wed, Jul 6, 2022 at 4:52 AM Daniel P. Smith > > wrote: > >> On 6/10/22 12:40, Ard Biesheuvel wrote:> On Thu, 19 May 2022 at 22:59, > >> To help provide clarity, consider the following flows for comparison, > >> > >> Normal/existing efi-stub: > >>EFI -> efi-stub -> head_64.S > >> > >> Proposed secure launch: > >>EFI -> efi-stub -> dl-handler -> [cpu] -> sl_stub ->head_64.S > > > > For more clarity; the entire point is to ensure that the kernel only > > has to trust itself and the CPU/TPM hardware (and does not have to > > trust a potentially malicious boot loader)..Any attempt to avoid a > > one-off solution for Linux is an attempt to weaken security. > > Please elaborate so I might understand how this entrypoint allows for > the kernel to only trust itself and the CPU/TPM. Is this a serious request? Kernel is started (via. firmware using the kernel's efi-stub, or via. "kexec()", or..); and regardless of how the kernel was started the kernel establishes its own dynamic root of trust.(e.g. AMD"s SKINIT or Intel's TXT, followed by measuring the remainder of itself and anything passed from firmware like APCI tables) without relying on a call-back provided by "untrusted by kernel" third-parties that don't exist in most cases. The dynamic root of trust that kernel creates depends on the kernel, CPU, TPM, etc (and excludes untrusted and unnecessary third parties).. The only potential benefit that the callback solution provides is that it, in theory, it could reduce duplication of work for other operating systems (FreeBSD, Solaris, Haiku, Fuchsia, .. could use the same callback instead of doing it themselves); but previous discussions (talk of formalising the contract between the boot stub and the Linux kernel) suggest that you aren't interested in any other OS. This leaves me wondering what your true motivation is. Are you trying to benefit GRUB/Trenchboot (at the expense of security, end-user convenience, distro installer hassle, etc); or trying to manufacture scope for future man-in-the middle attacks (by promoting a solution that requires something between firmware and kernel)? - Brendan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 00/15] GDB script fixes and improvements
On Tue, 5 Jul 2022 13:16:27 +0200 Daniel Kiper wrote: > Hi Glenn, > > On Fri, May 13, 2022 at 06:12:33PM -0500, Glenn Washburn wrote: > > There's been a lot of changes since v1. There are more fixes and more > > features. The majority of the shell code has been moved to an external > > file named gdb_helper.sh, instead of being inline in the GDB script. The > > one (direct) PERL dependency in GRUB has been removed and converted to > > shell script. Also a section on debugging is added to the developer docs. > > > > Glenn > > > > Glenn Washburn (15): > > gdb: Fix redirection issue in dump_module_sections > > gdb: Prevent wrapping when writing to .segments.tmp > > gdb: If no modules have been loaded, do not try to load module symbols > > gdb: Move runtime module loading into runtime_load_module > > gdb: Get correct mod variable value > > gdb: Do not run load_module if module has already been loaded > > gdb: Add functions to make loading from dynamically positioned targets > > easier > > gdb: Remove Perl dependency for GRUB GDB script > > gdb: If enabled, print line used to load EFI kernel symbols when using > > gdb_grub script > > gdb: Conditionally run GDB script logic for dynamically or statically > > positioned GRUB > > gdb: Only connect to remote target once when first sourced > > gdb: Allow user defined "onload_" command to be run when > > module is loaded > > gdb: Allow running user-defined commands at GRUB start > > gdb: Add ability to turn on shell tracing for gdb helper script > > docs: Add debugging chapter to development documentation > > > > config.h.in | 3 + > > docs/grub-dev.texi | 191 ++ > > grub-core/Makefile.core.def | 4 +- > > grub-core/gdb_grub.in | 198 > > grub-core/gdb_helper.sh.in | 108 > > grub-core/gmodule.pl.in | 30 -- > > grub-core/kern/efi/efi.c| 4 +- > > grub-core/kern/efi/init.c | 19 +++- > > include/grub/efi/efi.h | 2 +- > > 9 files changed, 501 insertions(+), 58 deletions(-) > > create mode 100644 grub-core/gdb_helper.sh.in > > delete mode 100644 grub-core/gmodule.pl.in > > This is great improvement. However, after looking at the list of changed > files it seems to me it should be rebased. May I ask you to do that? > Sorry for the inconvenience. A rangediff of this series against the rebased commits shows nothing has changed. Confirming rangediff, only docs/grub-dev.texi has been changed in master since this series was sent and those changes are all after the changes in this series. So there's no reason to send an updated series as it would just be the exact same patch files. Glenn > > Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Linux DRTM on UEFI platforms
On Fri, Jul 08, 2022 at 01:06:19PM +0930, Brendan Trotter wrote: > This leaves me wondering what your true motivation is. Are you trying > to benefit GRUB/Trenchboot (at the expense of security, end-user > convenience, distro installer hassle, etc); or trying to manufacture > scope for future man-in-the middle attacks (by promoting a solution > that requires something between firmware and kernel)? The described mechanism doesn't require trusting the code that's in the middle - if the state is perturbed by this code, the measurements will be different, and the system will be untrusted. I agree that this implementation is more complicated than just leaving it all up to the kernel, but I'm having a *lot* of trouble seeing how this has any impact on its security. Jumping immediately to impugning the motivation of the people involved is entirely inappropriate. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel