[PATCH 2/2] Add net_set_vlan command

2022-03-04 Thread Chad Kimes via Grub-devel
Previously there was no way to set the 802.1Q VLAN identifier, despite
support for vlantag in the net module. The only location vlantag was
being populated was from PXE boot and only for Open Firmware hardware.
This commit allows users to manually configure VLAN information for any
interface.

Example usage:
  grub> net_ls_addr
  efinet1 00:11:22:33:44:55 192.168.0.100
  grub> net_set_vlan efinet1 100
  grub> net_ls_addr
  efinet1 00:11:22:33:44:55 192.168.0.100 vlan100
  grub> net_set_vlan efinet1 0
  efinet1 00:11:22:33:44:55 192.168.0.100

Signed-off-by: Chad Kimes 
---
 docs/grub.texi  |  9 +
 grub-core/net/net.c | 34 +-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index caba8befb..5758ec285 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5553,6 +5553,7 @@ This command is only available on AArch64 systems.
 * net_ls_cards::List network cards
 * net_ls_dns::  List DNS servers
 * net_ls_routes::   List routing entries
+* net_set_vlan::Set vlan id on an interface
 * net_nslookup::Perform a DNS lookup
 @end menu
 
@@ -5721,6 +5722,14 @@ List routing entries.
 @end deffn
 
 
+@node net_set_vlan
+@subsection net_set_vlan
+
+@deffn Command net_set_vlan @var{interface} @var{vlanid}
+Set the 802.1Q VLAN identifier on @var{interface} to @var{vlanid}.
+@end deffn
+
+
 @node net_nslookup
 @subsection net_nslookup
 
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 33e35d5b5..f2acd2ecf 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1176,6 +1176,35 @@ grub_cmd_addroute (struct grub_command *cmd 
__attribute__ ((unused)),
 }
 }
 
+static grub_err_t
+grub_cmd_setvlan (struct grub_command *cmd __attribute__ ((unused)),
+ int argc, char **args)
+{
+  if (argc < 2)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
+
+  const char *vlanptr = args[1];
+  unsigned long vlantag;
+  vlantag = grub_strtoul (vlanptr, , 10);
+
+  if (vlantag > 4094)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid vlan id"));
+
+  struct grub_net_network_level_interface *inter;
+
+  FOR_NET_NETWORK_LEVEL_INTERFACES (inter)
+{
+  if (grub_strcmp (inter->name, args[0]) != 0)
+   continue;
+
+  inter->vlantag = vlantag;
+  return GRUB_ERR_NONE;
+}
+
+  return grub_error (GRUB_ERR_BAD_ARGUMENT,
+ N_("network interface not found"));
+}
+
 static void
 print_net_address (const grub_net_network_level_netaddress_t *target)
 {
@@ -1892,7 +1921,7 @@ static struct grub_preboot *fini_hnd;
 
 static grub_command_t cmd_addaddr, cmd_deladdr, cmd_addroute, cmd_delroute;
 static grub_command_t cmd_lsroutes, cmd_lscards;
-static grub_command_t cmd_lsaddr, cmd_slaac;
+static grub_command_t cmd_lsaddr, cmd_slaac, cmd_setvlan;
 
 GRUB_MOD_INIT(net)
 {
@@ -1935,6 +1964,9 @@ GRUB_MOD_INIT(net)
   "", N_("list network cards"));
   cmd_lsaddr = grub_register_command ("net_ls_addr", grub_cmd_listaddrs,
   "", N_("list network addresses"));
+  cmd_setvlan = grub_register_command ("net_set_vlan", grub_cmd_setvlan,
+  N_("SHORTNAME VLANID"),
+  N_("Set an interace's vlan id."));
   grub_bootp_init ();
   grub_dns_init ();
 
-- 
2.25.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 1/2] Add vlan information to net_ls_addr output

2022-03-04 Thread Chad Kimes via Grub-devel
Example output:
  grub> net_ls_addr
  efinet1 00:11:22:33:44:55 192.168.0.100 vlan100

Signed-off-by: Chad Kimes 
---
 grub-core/net/net.c | 18 +-
 include/grub/net.h  |  8 
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 4d3eb5c1a..33e35d5b5 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -781,6 +781,20 @@ grub_net_hwaddr_to_str (const 
grub_net_link_level_address_t *addr, char *str)
   grub_printf (_("Unsupported hw address type %d\n"), addr->type);
 }
 
+void
+grub_net_vlan_to_str (grub_uint16_t vlantag, char *str)
+{
+  str[0] = 0;
+
+  /* 12 bits are used to identify the vlan in 802.1Q */
+  vlantag = vlantag & 0xFFF;
+
+  if (vlantag == 0)
+return;
+
+  grub_snprintf (str, GRUB_NET_MAX_STR_VLAN_LEN, "vlan%u", vlantag);
+}
+
 int
 grub_net_hwaddr_cmp (const grub_net_link_level_address_t *a,
 const grub_net_link_level_address_t *b)
@@ -1250,9 +1264,11 @@ grub_cmd_listaddrs (struct grub_command *cmd 
__attribute__ ((unused)),
   {
 char bufh[GRUB_NET_MAX_STR_HWADDR_LEN];
 char bufn[GRUB_NET_MAX_STR_ADDR_LEN];
+char bufv[GRUB_NET_MAX_STR_VLAN_LEN];
 grub_net_hwaddr_to_str (>hwaddress, bufh);
 grub_net_addr_to_str (>address, bufn);
-grub_printf ("%s %s %s\n", inf->name, bufh, bufn);
+grub_net_vlan_to_str (inf->vlantag, bufv);
+grub_printf ("%s %s %s %s\n", inf->name, bufh, bufn, bufv);
   }
   return GRUB_ERR_NONE;
 }
diff --git a/include/grub/net.h b/include/grub/net.h
index 7ae4b6bd8..7fccad8ec 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -512,12 +512,20 @@ grub_net_addr_cmp (const grub_net_network_level_address_t 
*a,
 
 #define GRUB_NET_MAX_STR_HWADDR_LEN (sizeof ("XX:XX:XX:XX:XX:XX"))
 
+/*
+  Max VLAN id = 4094
+ */
+#define GRUB_NET_MAX_STR_VLAN_LEN (sizeof ("vlan"))
+
 void
 grub_net_addr_to_str (const grub_net_network_level_address_t *target,
  char *buf);
 void
 grub_net_hwaddr_to_str (const grub_net_link_level_address_t *addr, char *str);
 
+void
+grub_net_vlan_to_str (grub_uint16_t vlantag, char *str);
+
 grub_err_t
 grub_env_set_net_property (const char *intername, const char *suffix,
const char *value, grub_size_t len);
-- 
2.25.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v8 0/6] Update gnulib version and drop most gnulib patches

2022-03-04 Thread Glenn Washburn
On Wed,  2 Mar 2022 14:08:23 -0500
Robbie Harwood  wrote:

> Changes this version:
> 
> - Reorder last two commits so that warning fixes come after the change that
>   introduces them.
> - Fix comment formatting to comply with grub2 style.

Either I missed it before or something changed. But I'm getting this
build error now for x86_64-efi, and I'm not getting it without this patch
series.

In file included from /root/grub-tests.update-gnulib/grub/include/grub/disk.h:31
,
 from /root/grub-tests.update-gnulib/grub/include/grub/file.h:26
,
 from /root/grub-tests.update-gnulib/grub/include/grub/loader.h:
23,
 from /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/
bsd.c:19:
/root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c: In function ‘gr
ub_freebsd_add_meta_module’:
/root/grub-tests.update-gnulib/grub/include/grub/misc.h:71:10: error: ‘ptr’ may 
be used uninitialized in this function [-Werror=maybe-uninitialized]
   71 |   return grub_memmove (dest, src, n);
  |  ^~~
/root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c:266:9: note: 
‘ptr’ was declared here
  266 |   void *ptr;
  | ^~~

Reviewing the code it doesn't look like ptr can actually be used
uninitialized, so it seems like GCC 10.1.0 isn't smart enough to figure
that out. Initializing to NULL fixes the build issue.

Glenn

> 
> No functional code changes.
> 
> Be well,
> --Robbie
> 
> Robbie Harwood (6):
>   Use visual indentation in config.h.in
>   Where present, ensure config-util.h precedes config.h
>   Drop gnulib fix-base64.patch
>   Drop gnulib no-abort.patch
>   Update gnulib version and drop most gnulib patches
>   Handle warnings introduced by updated gnulib
> 
>  INSTALL   |   4 +-
>  bootstrap | 319 ++
>  bootstrap.conf|  23 +-
>  conf/Makefile.extra-dist  |   8 -
>  config.h.in   | 142 ++--
>  configure.ac  |   2 +-
>  grub-core/Makefile.core.def   |   3 +
>  grub-core/disk/host.c |   2 +-
>  grub-core/disk/luks2.c|   4 +-
>  grub-core/gensymlist.sh   |   1 +
>  grub-core/kern/emu/argp_common.c  |   2 +-
>  grub-core/kern/emu/main.c |   2 +-
>  grub-core/lib/gnulib-patches/fix-base64.patch |  21 --
>  .../lib/gnulib-patches/fix-null-deref.patch   |  13 -
>  .../gnulib-patches/fix-null-state-deref.patch |  12 -
>  .../fix-regcomp-uninit-token.patch|  15 -
>  .../fix-regexec-null-deref.patch  |  12 -
>  .../gnulib-patches/fix-uninit-structure.patch |  11 -
>  .../lib/gnulib-patches/fix-unused-value.patch |  14 -
>  grub-core/lib/gnulib-patches/no-abort.patch   |  26 --
>  grub-core/lib/posix_wrap/limits.h |   6 +-
>  grub-core/lib/posix_wrap/sys/types.h  |   7 +-
>  grub-core/lib/xzembed/xz.h|   5 +-
>  grub-core/osdep/aros/config.c |   2 +-
>  grub-core/osdep/basic/emunet.c|   2 +-
>  grub-core/osdep/basic/init.c  |   2 +-
>  grub-core/osdep/haiku/getroot.c   |   2 +-
>  grub-core/osdep/linux/emunet.c|   2 +-
>  grub-core/osdep/unix/config.c |   2 +-
>  grub-core/osdep/unix/cputime.c|   2 +-
>  grub-core/osdep/unix/dl.c |   2 +-
>  grub-core/osdep/unix/emuconsole.c |   2 +-
>  grub-core/osdep/unix/getroot.c|   2 +-
>  grub-core/osdep/windows/config.c  |   2 +-
>  grub-core/osdep/windows/cputime.c |   2 +-
>  grub-core/osdep/windows/dl.c  |   2 +-
>  grub-core/osdep/windows/emuconsole.c  |   2 +-
>  grub-core/osdep/windows/init.c|   2 +-
>  include/grub/compiler.h   |   4 +-
>  include/grub/list.h   |   2 +-
>  40 files changed, 347 insertions(+), 343 deletions(-)
>  delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch
>  delete mode 100644 grub-core/lib/gnulib-patches/fix-null-deref.patch
>  delete mode 100644 grub-core/lib/gnulib-patches/fix-null-state-deref.patch
>  delete mode 100644 
> grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
>  delete mode 100644 grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
>  delete mode 100644 grub-core/lib/gnulib-patches/fix-uninit-structure.patch
>  delete mode 100644 grub-core/lib/gnulib-patches/fix-unused-value.patch
>  delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch
> 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: Minimum compiler version (was: Re: [PATCH v5 0/4] Update gnulib and drop some patches)

2022-03-04 Thread Daniel Kiper
On Mon, Feb 28, 2022 at 12:24:50PM -0500, Robbie Harwood wrote:
> Glenn Washburn  writes:
>
> > GCC 5.1.0 looks like it came out on April 22, 2015[1] and 5.2 was used
> > in Ubuntu Xenial from 2016 (which is no longer supported). At what
> > point do we bump up the minimum supported version? And doing so
> > wouldn't mean that GRUB can't be compiled with eariler versions of
> > GCC, it just means we don't test that. I also think it would be
> > acceptable to accept patches that fix issues with compiling on GCC
> > versions less than the stated minimum supported version (with in
> > reason and subject to discretion).
> >
> > One idea is to update the minimum supported version every release cycle
> > to the lowest GCC version that is about 5 years old (that's artitrary
> > but seems reasonable).

I think we should support a given minimum compiler version until it does
not require a lot of effort from our side. So, as long as possible...

> > I'm interested in this because it seems to imply that for the testing
> > system it should do two compilations for every target, one for the
> > munimum supported GCC and one for a somewhat recent version.

I fully agree with this.

> I support raising the minimum compiler version.
>
> Looking at https://www.gnu.org/software/gcc/releases.html , a five-year
> proposal would raise to gcc-7.

I am OK with this because as it was pointed out in other places older
compilers do not support RISC-V builds. Of course if I do not hear
a lot of complaints... :-)

So, I am looking forward for an INSTALL file patch...

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 1/2] EFI: console: Do not set colorstate until the first text output

2022-03-04 Thread Daniel Kiper
On Fri, Mar 04, 2022 at 11:30:13AM +0100, Hans de Goede wrote:
> Hi Daniel,
>
> On 1/28/22 12:43, Hans de Goede wrote:
> > GRUB_MOD_INIT(normal) does an unconditional:
> >
> > grub_env_set ("color_normal", "light-gray/black");
> >
> > which triggers a grub_term_setcolorstate() call. The original version
> > of the "efi/console: Do not set text-mode until we actually need it" patch:
> > https://lists.gnu.org/archive/html/grub-devel/2018-03/msg00125.html
> >
> > Protected against this by caching the requested state in
> > grub_console_setcolorstate () and then only applying it when the first
> > text output actually happens. During refactoring to move the
> > grub_console_setcolorstate () up higher in the grub-core/term/efi/console.c
> > file the code to cache the color-state + bail early was accidentally
> > dropped.
> >
> > Restore the cache the color-state + bail early behavior from the original.
> >
> > Cc: Javier Martinez Canillas 
> > Fixes: 2d7c3abd871f ("efi/console: Do not set text-mode until we actually 
> > need it")
> > Signed-off-by: Hans de Goede 
>
> What is the status of this series? It has been well over a month and both
> patches have received 2 Reviewed-by-s and no other comments:
>
> Reviewed-by: Javier Martinez Canillas 
> Reviewed-by: Robbie Harwood 

I will take a look at them in a week or two. Sorry for delay...

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v8 4/6] Drop gnulib no-abort.patch

2022-03-04 Thread Daniel Kiper
On Fri, Mar 04, 2022 at 11:40:10AM -0600, Glenn Washburn wrote:
> On Thu, 03 Mar 2022 13:47:29 -0500
> Robbie Harwood  wrote:
> > Glenn Washburn  writes:
> > > Robbie Harwood  wrote:

[...]

> > For completeness, here's what happens if one just defines to grub_abort
> > without further modification:
> >
> > $ uname -m
> > x86_64
> > $ ./bootstrap
> > ...
> > $ ./configure --enable-grub-mkfont
> > ...
> > $ make
> > ...
> > gcc -DHAVE_CONFIG_H -I. -I..  -Wall -W  -DGRUB_MACHINE_PCBIOS=1 
> > -DGRUB_MACHINE=I386_PC -m32 -nostdinc -isystem 
> > /usr/lib/gcc/x86_64-linux-gnu/11/include -I../include -I../include 
> > -DGRUB_FILE=\"lib/gnulib/regex.c\" -I. -I. -I.. -I.. -I../include 
> > -I../include -I../grub-core/lib/libgcrypt-grub/src/   
> > -I../grub-core/lib/posix_wrap -I../grub-core/lib/gnulib 
> > -I../grub-core/lib/gnulib  -D_FILE_OFFSET_BITS=64 -std=gnu99 -Os -m32 -Wall 
> > -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment 
> > -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero 
> > -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit 
> > -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces 
> > -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type 
> > -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs 
> > -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label 
> > -Wunused-parameter -Wunused-value  -Wunused-variable -Wwrite-strings 
> > -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls 
> > -Wmissing-prototypes -Wmissing-declarations  -Wextra -Wattributes 
> > -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch 
> > -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla 
> > -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros 
> > -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs 
> > -Wmissing-prototypes -Wmissing-declarations -Wformat=2 -march=i386 -mrtd 
> > -mregparm=3 -falign-functions=1 -falign-loops=1 -falign-jumps=1 
> > -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow 
> > -Wa,-mx86-used-note=no -msoft-float -fno-dwarf2-cfi-asm 
> > -mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables 
> > -fno-ident -fno-PIE -fno-pie -fno-stack-protector -Wtrampolines -Werror   
> > -ffreestanding -fno-builtin -Wno-undef -Wno-sign-compare -Wno-unused 
> > -Wno-unused-parameter -Wno-redundant-decls -Wno-unreachable-code 
> > -Wno-conversion   -MT lib/gnulib/regexp_module-regex.o -MD -MP -MF 
> > lib/gnulib/.deps-core/regexp_module-regex.Tpo -c -o 
> > lib/gnulib/regexp_module-regex.o `test -f 'lib/gnulib/regex.c' || echo 
> > './'`lib/gnulib/regex.c
> > In file included from ../grub-core/lib/gnulib/libc-config.h:36,
> >  from lib/gnulib/regex.c:23:
> > lib/gnulib/regcomp.c: In function ‘regerror’:
> > ../config.h:145:19: error: implicit declaration of function ‘grub_abort’; 
> > did you mean ‘grub_reboot’? [-Werror=implicit-function-declaration]
> >   145 | #define abort grub_abort
> >   |   ^~
> > lib/gnulib/regcomp.c:509:5: note: in expansion of macro ‘abort’
> >   509 | abort ();
> >   | ^
> > ../config.h:145:19: error: nested extern declaration of ‘grub_abort’ 
> > [-Werror=nested-externs]
> >   145 | #define abort grub_abort
> >   |   ^~
> > lib/gnulib/regcomp.c:509:5: note: in expansion of macro ‘abort’
> >   509 | abort ();
> >   | ^
> > cc1: all warnings being treated as errors
> > ...
> > $ git diff
> > diff --git a/config.h.in b/config.h.in
> > index 0fca0597d..8ad4aa0ac 100644
> > --- a/config.h.in
> > +++ b/config.h.in
> > @@ -142,7 +142,7 @@ typedef __UINT_FAST32_TYPE__ uint_fast32_t;
> >   * a prototype for abort(), so leave this as a macro that doesn't take
> >   * arguments.
> >   */
> > -#define abort __builtin_trap
> > +#define abort grub_abort
>
> Thank you for letting me know what you tried and what was not working.
> It turns out the issue was more complicated and different than I was let
> on to believe based on the comment above. And while my link solution
> could be part of the solution, its not necessary.
>
> I'll send my changes to this thread shortly, unless you want me to roll
> a v9. Thanks for getting this within a hare's breath of the finish line.

Glenn, I am OK with improving this but I want to take v8 first to not
delay gnulib bump any longer. So, please post your patch(es) when Robbie
patches are in (Monday or Tuesday).

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v8] Fix various new autotools warnings

2022-03-04 Thread Robbie Harwood
Signed-off-by: Robbie Harwood 
---
 configure.ac | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6a8ae668d..46b976562 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,6 +36,7 @@ dnl description of the relationships between them.
 
 AC_INIT([GRUB],[2.11],[bug-g...@gnu.org])
 
+AC_USE_SYSTEM_EXTENSIONS
 AC_CONFIG_AUX_DIR([build-aux])
 
 # We don't want -g -O2 by default in CFLAGS
@@ -51,7 +52,7 @@ program_prefix="${save_program_prefix}"
 AM_INIT_AUTOMAKE([1.11])
 AC_PREREQ(2.64)
 AC_CONFIG_SRCDIR([include/grub/dl.h])
-AC_CONFIG_HEADER([config-util.h])
+AC_CONFIG_HEADERS([config-util.h])
 
 # Explicitly check for pkg-config early on, since otherwise conditional
 # calls are problematic.
@@ -333,7 +334,7 @@ fi
 AC_PROG_RANLIB
 AC_PROG_INSTALL
 AC_PROG_AWK
-AC_PROG_LEX
+AC_PROG_LEX([noyywrap])
 AC_PROG_YACC
 AC_PROG_MAKE_SET
 AC_PROG_MKDIR_P
@@ -369,7 +370,6 @@ test "x$GCC" = xyes || AC_MSG_ERROR([GCC is required])
 
 AC_CHECK_PROG(HAVE_CXX, $CXX, yes, no)
 
-AC_GNU_SOURCE
 AM_GNU_GETTEXT([external])
 AM_GNU_GETTEXT_VERSION([0.18.3])
 AC_SYS_LARGEFILE
-- 
2.34.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v8 4/6] Drop gnulib no-abort.patch

2022-03-04 Thread Glenn Washburn
On Thu, 03 Mar 2022 13:47:29 -0500
Robbie Harwood  wrote:

> Glenn Washburn  writes:
> 
> > Robbie Harwood  wrote:
> >
> >> If you have a patch that makes this work, I don't have a problem with
> >> it.  However, I was unable to make that work in practice.
> >
> > Can you provide some specifics on what problem you were running in to?
> > Was it a link issue at build time? platform specific? Did it build
> > fine, but blew up in testing? Did you try using the linker options I
> > suggested above?
> 
> No, nor am I about to.  This is code that I have, that builds, that I'm
> submitting to grub.  If you want a different approach, that's fine -
> you're welcome to write a patch to do that instead.  I have built this
> bike shed and I *really* do not care what color it is, nor do I
> appreciate being asked to test other people's proposals for them: you're
> presumably just as capable of building the code yourself and seeing if
> something works or doesn't.  This is v8 of the series and I'm pretty
> much done caring about it at this point.

Sounds like you're frustrated with the process and needed to let off
some stream. I can speak from experience on that subject (I got up to
v9 on a cryptodisk patch series that was actually fixing bugs). Of
course, its fine that you dont appreciate being asked to "test other
people's proposals" and its fine for anyone to ask you to do that,
which I did not (I asked if you had, there's a difference). You seem
capable of setting boundaries, as evidenced here, so no problem there.

There are many people on this list, including me, that have on occasion
modified a patch series according to someone else's proposal. I don't
think its an unreasonable ask, especially when the patch in question is
not correct. You're a free human being capable of saying no, as is your
perogative. There's no need to take it as a personal affront.

The GRUB project seems to me to have a fairly high code quality bar.
I've been frustrated in the past about what I considered bike shedding
as well and have made my frustrations known on and off this list. And I
think this is less trivial than some of those things.

Also, I think a more accurate metaphor, in this case, rather than
quibbling over color is whether you'll get electrocuted when lightening
strikes. This seems like its introducing a misfeature that someone else
down the line will get bitten by. So, if we can avoid that now when we
know there's an issue, the cost is much lower.

I hear you loud and clear saying you're done pushing on this patch
series. Would you like me to take it over? I've found what I believe to
be an acceptable solution using grub_abort.

> 
> For completeness, here's what happens if one just defines to grub_abort
> without further modification:
> 
> $ uname -m
> x86_64
> $ ./bootstrap
> ...
> $ ./configure --enable-grub-mkfont
> ...
> $ make
> ...
> gcc -DHAVE_CONFIG_H -I. -I..  -Wall -W  -DGRUB_MACHINE_PCBIOS=1 
> -DGRUB_MACHINE=I386_PC -m32 -nostdinc -isystem 
> /usr/lib/gcc/x86_64-linux-gnu/11/include -I../include -I../include 
> -DGRUB_FILE=\"lib/gnulib/regex.c\" -I. -I. -I.. -I.. -I../include 
> -I../include -I../grub-core/lib/libgcrypt-grub/src/   
> -I../grub-core/lib/posix_wrap -I../grub-core/lib/gnulib 
> -I../grub-core/lib/gnulib  -D_FILE_OFFSET_BITS=64 -std=gnu99 -Os -m32 -Wall 
> -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment 
> -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal 
> -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit 
> -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces 
> -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type 
> -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs 
> -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label 
> -Wunused-parameter -Wunused-value  -Wunused-variable -Wwrite-strings 
> -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls 
> -Wmissing-prototypes -Wmissing-declarations  -Wextra -Wattributes 
> -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch 
> -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast 
> -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign 
> -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 
> -march=i386 -mrtd -mregparm=3 -falign-functions=1 -falign-loops=1 
> -falign-jumps=1 -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 
> -mno-3dnow -Wa,-mx86-used-note=no -msoft-float -fno-dwarf2-cfi-asm 
> -mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables 
> -fno-ident -fno-PIE -fno-pie -fno-stack-protector -Wtrampolines -Werror   
> -ffreestanding -fno-builtin -Wno-undef -Wno-sign-compare -Wno-unused 
> -Wno-unused-parameter -Wno-redundant-decls -Wno-unreachable-code 
> -Wno-conversion   -MT lib/gnulib/regexp_module-regex.o -MD -MP -MF 
> lib/gnulib/.deps-core/regexp_module-regex.Tpo -c -o 
> lib/gnulib/regexp_module-regex.o `test -f 

[PATCH] net: check against nb->tail in grub_netbuff_pull

2022-03-04 Thread Daniel Axtens
GRUB netbuff structure members track 2 different things: the extent of memory
allocated for the packet, and the extent of memory currently being worked on.

This works out in the structure as follows:

 nb->head: beginning of the allocation
 nb->data: beginning of the working data
 nb->tail: end of the working data
 nb->end:  end of the allocation

The head and end pointers are set in grub_netbuff_alloc and do not change.
The data and tail pointers are initialised to point at start of the
allocation (that is, head == data == tail initially), and are then
manipulated by grub_netbuff_* functions. Key functions are as follows:

 - grub_netbuff_put: 'Put' more data into the packet - advance nb->tail.
 - grub_netbuff_unput: trim the tail of the packet - retract nb->tail
 - grub_netbuff_pull: 'consume' some packet data - advance nb->data
 - grub_netbuff_reserve: reserve space for future headers - advance nb->data and
 nb->tail
 - grub_netbuff_push: 'un-consume' data to allow headers to be written -
  retract nb->data.

Each of those functions does some form of error checking. For example,
grub_netbuff_put does not allow nb->tail to exceed nb->end, and
grub_netbuff_push does not allow nb->data to be before nb->head.

However, grub_netbuff_pull's error checking is a bit weird. It advances nb->data
and checks that it does not exceed nb->end. That allows you to get into the
situation where nb->data > nb->tail, which should not be.

Make grub_netbuff_pull check against both nb->tail and nb->end. In theory just
checking against ->tail should be sufficient but the extra check should be
cheap and seems like good defensive practice.

Signed-off-by: Daniel Axtens 

---

I'm not aware of any particular bug this fixes. All it can do is prevent you
reading uninitialised but still allocated memory. It just seems like a good
idea.

The netboot test still passses on the pc platform, more testing would be good.
---
 grub-core/net/netbuff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/net/netbuff.c b/grub-core/net/netbuff.c
index dbeeefe4783c..72e5296356f0 100644
--- a/grub-core/net/netbuff.c
+++ b/grub-core/net/netbuff.c
@@ -54,7 +54,7 @@ grub_err_t
 grub_netbuff_pull (struct grub_net_buff *nb, grub_size_t len)
 {
   nb->data += len;
-  if (nb->data > nb->end)
+  if (nb->data > nb->end || nb->data > nb->tail)
 return grub_error (GRUB_ERR_BUG,
   "pull out of the packet range.");
   return GRUB_ERR_NONE;
-- 
2.32.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 1/2] EFI: console: Do not set colorstate until the first text output

2022-03-04 Thread Hans de Goede
Hi Daniel,

On 1/28/22 12:43, Hans de Goede wrote:
> GRUB_MOD_INIT(normal) does an unconditional:
> 
> grub_env_set ("color_normal", "light-gray/black");
> 
> which triggers a grub_term_setcolorstate() call. The original version
> of the "efi/console: Do not set text-mode until we actually need it" patch:
> https://lists.gnu.org/archive/html/grub-devel/2018-03/msg00125.html
> 
> Protected against this by caching the requested state in
> grub_console_setcolorstate () and then only applying it when the first
> text output actually happens. During refactoring to move the
> grub_console_setcolorstate () up higher in the grub-core/term/efi/console.c
> file the code to cache the color-state + bail early was accidentally
> dropped.
> 
> Restore the cache the color-state + bail early behavior from the original.
> 
> Cc: Javier Martinez Canillas 
> Fixes: 2d7c3abd871f ("efi/console: Do not set text-mode until we actually 
> need it")
> Signed-off-by: Hans de Goede 

What is the status of this series? It has been well over a month and both
patches have received 2 Reviewed-by-s and no other comments:

Reviewed-by: Javier Martinez Canillas 
Reviewed-by: Robbie Harwood 

Regards,

Hans



> ---
>  grub-core/term/efi/console.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
> index 2f1ae85ba..c44b2ac31 100644
> --- a/grub-core/term/efi/console.c
> +++ b/grub-core/term/efi/console.c
> @@ -82,6 +82,16 @@ grub_console_setcolorstate (struct grub_term_output *term
>  {
>grub_efi_simple_text_output_interface_t *o;
>  
> +  if (grub_efi_is_finished || text_mode != GRUB_TEXT_MODE_AVAILABLE)
> +{
> +  /*
> +   * Cache colorstate changes before the first text-output, this avoids
> +   * "color_normal" environment writes causing a switch to textmode.
> +   */
> +  text_colorstate = state;
> +  return;
> +}
> +
>if (grub_efi_is_finished)
>  return;
>  


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 0/2] EFI/menu: Do not print messages when autobooting with TIMEOUT_STYLE_HIDDEN

2022-03-04 Thread Hans de Goede
Hi Daniel,

On 1/28/22 11:30, Hans de Goede wrote:
> Hi All,
> 
> This series in essence is a renewed attempt to get grub to really
> be truly hidden when autobooting with TIMEOUT_STYLE_HIDDEN.
> 
> This is something which multiple big Linux distributions want,
> this is a minimized version of my previous attempt from March 2018:
> https://lists.gnu.org/archive/html/grub-devel/2018-03/msg00126.html
> With some discussion about this patch in April 2018:
> https://lists.gnu.org/archive/html/grub-devel/2018-04/msg9.html
> 
> The first patch silences the notify_booting() callback from
> grub-core/normal/menu.c based on the timeout_style (1), as
> suggested in the original discussion.
> 
> After that there is just one message left, from grub-core/kern/main.c
> which is printed before the config file loads. When timeout_style
> is not set to hidden this will only show very briefly to then be
> replaced by the menu / countdown; and with timeout_style=hidden
> we actually want it to behave the same as any other grub messages
> and have it hidden. The second patch makes this happen by just
> disabling the logging of the message for EFI builds.
> 
> This also fixes the message briefly flashing by (in an ugly manner
> because of the "flash" part) when timeout_style!=hidden.
> 
> Regards,
> 
> Hans
> 
> 
> 1) It is only silenced when autobooting with TIMEOUT_STYLE_HIDDEN

What is the status of this series? It has been well over a month and both
patches have received a Reviewed-by and no other comments:

Reviewed-by: Robbie Harwood 

Regards,

Hans






> Hans de Goede (2):
>   normal/menu: Don't show "Booting `%s'" msg when auto-booting with
> TIMEOUT_STYLE_HIDDEN
>   EFI: suppress the "Welcome to GRUB!" message in EFI builds
> 
>  grub-core/kern/main.c   |  3 +++
>  grub-core/normal/menu.c | 25 +
>  2 files changed, 20 insertions(+), 8 deletions(-)
> 


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel