All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Jeddeloh <andrew.jeddeloh@redhat.com>
To: grub-devel@gnu.org
Subject: [PATCH] loader/i386/linux: calculate the size of the setup header
Date: Wed, 9 May 2018 10:46:47 -0700	[thread overview]
Message-ID: <CAK=WzweAuG_ygdHjJNB9uxfX-T3WvLDjxnkAaBdzsoKriGXa6g@mail.gmail.com> (raw)

This patch is prompted from a question I asked a while ago about why
the disk read is necessary. See the thread here [1].

This changes the disk read to use the length of the setup header as
calculated by the x86 32 bit linux boot protocol [1]. I'm not 100%
sure its patch that's wanted however. The idea was that grub should
only read the amount specified by the boot protocol and not more, but
it turns out the size of the linux_kernel_params struct is already
sized such that grub reads the exact amount anyway (at least with the
recent kernels I've tested with). This introduces two changes:
 - if a new version of linux makes the setup headers section larger,
this will fail instead of only readiing the old fields. I'm not sure
if this behavior is desired.
 - If older versions have a smaller setup header section, less will be read.

[1] http://lists.gnu.org/archive/html/grub-devel/2018-04/msg00073.html
[2] https://raw.githubusercontent.com/torvalds/linux/master/Documentation/x86/boot.txt


Previously the length was just assumed to be the size of the
linux_params struct. The linux x86 32 bit boot protocol says the size of
the setup header is 0x202 + the byte at 0x201 in the boot params.
Calculate the size of the header, rather than assume it is the size of
the linux_params struct.

Signed-off-by: Andrew Jeddeloh <andrew.jeddeloh@coreos.com>
---
 grub-core/loader/i386/linux.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
index 44301e126..9b4d33785 100644
--- a/grub-core/loader/i386/linux.c
+++ b/grub-core/loader/i386/linux.c
@@ -805,7 +805,16 @@ grub_cmd_linux (grub_command_t cmd __attribute__
((unused)),
   linux_params.kernel_alignment = (1 << align);
   linux_params.ps_mouse = linux_params.padding10 =  0;

-  len = sizeof (linux_params) - sizeof (lh);
+  // The linux 32 bit boot protocol defines the setup header size to
be 0x202 + the size of
+  // the jump at 0x200. We've already read sizeof(lh)
+  len = *((char *)&linux_params.jump + 1) + 0x202 - sizeof(lh);
+
+  // Verify the struct is big enough so we do not write past the end
+  if (len + sizeof(lh) > sizeof(linux_params)) {
+    grub_error (GRUB_ERR_BAD_OS, "boot params setup header too big
for linux_params struct");
+    goto fail;
+  }
+
   if (grub_file_read (file, (char *) &linux_params + sizeof (lh), len) != len)
     {
       if (!grub_errno)
-- 
2.14.1


             reply	other threads:[~2018-05-09 17:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-09 17:46 Andrew Jeddeloh [this message]
2018-05-10 17:11 ` [PATCH] loader/i386/linux: calculate the size of the setup header Daniel Kiper
     [not found]   ` <CAK=WzwcAa5kjyT+Tupr5jedX+XiW1i4hDjknLZHEst1EGLOmtg@mail.gmail.com>
2018-05-18 11:53     ` Daniel Kiper
2018-05-25 19:02       ` Andrew Jeddeloh
2018-05-30 14:40         ` Daniel Kiper

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='CAK=WzweAuG_ygdHjJNB9uxfX-T3WvLDjxnkAaBdzsoKriGXa6g@mail.gmail.com' \
    --to=andrew.jeddeloh@redhat.com \
    --cc=grub-devel@gnu.org \
    /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.