All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Chang <mchang@suse.com>
To: The development of GNU GRUB <grub-devel@gnu.org>
Cc: Colin Watson <cjwatson@debian.org>,
	Marco A Benatto <mbenatto@redhat.com>,
	Javier Martinez Canillas <javierm@redhat.com>,
	Daniel Kiper <daniel.kiper@oracle.com>
Subject: [PATCH v2] i386-pc: build verifiers API as module
Date: Thu, 18 Mar 2021 19:30:26 +0800	[thread overview]
Message-ID: <20210318113026.24963-1-mchang@suse.com> (raw)

Given no core functions on i386-pc would require verifiers to work and
the only consumer of the verifier API is the pgp module, it looks good
to me that we can move the verifiers out of the kernel image and let
moddep.lst to auto-load it when pgp is loaded on i386-pc platform.

This helps to reduce the size of core image and thus can relax the
tension of exploding on some i386-pc system with very short MBR gap
size. See also a very comprehensive summary from Colin [1] about the
details.

[1] https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00240.html

V2:
Drop COND_NOT_i386_pc and use !COND_i386_pc.
Add comment in kern/verifiers.c to help understanding what's going on
without digging into the commit history.

Reported-by: Colin Watson <cjwatson@debian.org>
Reviewed-by: Colin Watson <cjwatson@debian.org>
Signed-off-by: Michael Chang <mchang@suse.com>
---
 grub-core/Makefile.am       |  2 ++
 grub-core/Makefile.core.def |  8 +++++++-
 grub-core/kern/main.c       |  4 ++++
 grub-core/kern/verifiers.c  | 17 +++++++++++++++++
 include/grub/verify.h       |  9 +++++++++
 5 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
index ee88e44e9..b6872d20f 100644
--- a/grub-core/Makefile.am
+++ b/grub-core/Makefile.am
@@ -93,7 +93,9 @@ KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/partition.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/stack_protector.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/term.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/time.h
+if !COND_i386_pc
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/verify.h
+endif
 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 8022e1c0a..77fdccdb1 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -141,7 +141,7 @@ kernel = {
   common = kern/rescue_parser.c;
   common = kern/rescue_reader.c;
   common = kern/term.c;
-  common = kern/verifiers.c;
+  nopc = kern/verifiers.c;
 
   noemu = kern/compiler-rt.c;
   noemu = kern/mm.c;
@@ -946,6 +946,12 @@ module = {
   cppflags = '-I$(srcdir)/lib/posix_wrap';
 };
 
+module = {
+  name = verifiers;
+  common = kern/verifiers.c;
+  enable = i386_pc;
+};
+
 module = {
   name = hdparm;
   common = commands/hdparm.c;
diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
index 73967e2f5..c7c6d2d0b 100644
--- a/grub-core/kern/main.c
+++ b/grub-core/kern/main.c
@@ -29,7 +29,9 @@
 #include <grub/command.h>
 #include <grub/reader.h>
 #include <grub/parser.h>
+#ifndef GRUB_MACHINE_PCBIOS
 #include <grub/verify.h>
+#endif
 
 #ifdef GRUB_MACHINE_PCBIOS
 #include <grub/machine/memory.h>
@@ -275,8 +277,10 @@ grub_main (void)
   grub_printf ("Welcome to GRUB!\n\n");
   grub_setcolorstate (GRUB_TERM_COLOR_STANDARD);
 
+#ifndef GRUB_MACHINE_PCBIOS
   /* Init verifiers API. */
   grub_verifiers_init ();
+#endif
 
   grub_load_config ();
 
diff --git a/grub-core/kern/verifiers.c b/grub-core/kern/verifiers.c
index 75d7994cf..1245d0d9e 100644
--- a/grub-core/kern/verifiers.c
+++ b/grub-core/kern/verifiers.c
@@ -221,8 +221,25 @@ grub_verify_string (char *str, enum grub_verify_string_type type)
   return GRUB_ERR_NONE;
 }
 
+/*
+ * It is intended to build verifiers as module on i386-pc platform to minimize
+ * the impact of growing core image size could blow up the 63 sectors limit of
+ * some MBR gap one day. It is also adequate to do so, given no core function
+ * on i386-pc would require the verifiers API to work.
+ */
+#ifdef GRUB_MACHINE_PCBIOS
+GRUB_MOD_INIT(verifiers)
+#else
 void
 grub_verifiers_init (void)
+#endif
 {
   grub_file_filter_register (GRUB_FILE_FILTER_VERIFY, grub_verifiers_open);
 }
+
+#ifdef GRUB_MACHINE_PCBIOS
+GRUB_MOD_FINI(verifiers)
+{
+  grub_file_filter_unregister (GRUB_FILE_FILTER_VERIFY);
+}
+#endif
diff --git a/include/grub/verify.h b/include/grub/verify.h
index cd129c398..6fde244fc 100644
--- a/include/grub/verify.h
+++ b/include/grub/verify.h
@@ -64,10 +64,14 @@ struct grub_file_verifier
   grub_err_t (*verify_string) (char *str, enum grub_verify_string_type type);
 };
 
+#ifdef GRUB_MACHINE_PCBIOS
+extern struct grub_file_verifier *grub_file_verifiers;
+#else
 extern struct grub_file_verifier *EXPORT_VAR (grub_file_verifiers);
 
 extern void
 grub_verifiers_init (void);
+#endif
 
 static inline void
 grub_verifier_register (struct grub_file_verifier *ver)
@@ -81,7 +85,12 @@ grub_verifier_unregister (struct grub_file_verifier *ver)
   grub_list_remove (GRUB_AS_LIST (ver));
 }
 
+#ifdef GRUB_MACHINE_PCBIOS
+grub_err_t
+grub_verify_string (char *str, enum grub_verify_string_type type);
+#else
 extern grub_err_t
 EXPORT_FUNC (grub_verify_string) (char *str, enum grub_verify_string_type type);
+#endif
 
 #endif /* ! GRUB_VERIFY_HEADER */
-- 
2.26.2



             reply	other threads:[~2021-03-18 11:30 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-18 11:30 Michael Chang [this message]
2021-03-22 15:20 ` [PATCH v2] i386-pc: build verifiers API as module Daniel Kiper
2021-03-22 16:16   ` Colin Watson
2021-03-22 20:09     ` Colin Watson
2021-03-22 20:19     ` Glenn Washburn
2021-03-22 20:45       ` Colin Watson
2021-03-23 16:33         ` Daniel Kiper
2021-03-23 17:45           ` Lennart Sorensen
2021-03-24  4:44           ` Michael Chang
2021-03-26 17:01             ` Daniel Kiper
2021-04-12 13:15               ` Daniel Kiper
2021-04-13  4:13                 ` Michael Chang
2021-04-14 13:22                   ` Daniel Kiper
2021-04-16 21:23                     ` Colin Watson
2021-04-20  3:49                     ` Michael Chang
2021-04-28 13:45                       ` Daniel Kiper
2021-03-22 21:43       ` James Bottomley
2021-03-23  4:16   ` Michael Chang
2021-03-23 11:37     ` Javier Martinez Canillas
2021-03-23 13:27       ` Colin Watson
2021-03-23 14:26         ` Javier Martinez Canillas
2021-03-23 17:26         ` Daniel Kiper
2021-03-23 16:48     ` Daniel Kiper
2021-03-24  3:50       ` Michael Chang
2021-03-26 17:22         ` Daniel Kiper
2021-08-22 19:50 Michael Schierl
2021-08-22 20:23 ` Didier Spaier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210318113026.24963-1-mchang@suse.com \
    --to=mchang@suse.com \
    --cc=cjwatson@debian.org \
    --cc=daniel.kiper@oracle.com \
    --cc=grub-devel@gnu.org \
    --cc=javierm@redhat.com \
    --cc=mbenatto@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.