From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1fMHye-00038m-8p for mharc-grub-devel@gnu.org; Fri, 25 May 2018 15:02:12 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56340) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fMHyb-00038R-Qc for grub-devel@gnu.org; Fri, 25 May 2018 15:02:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fMHyX-0005su-Tu for grub-devel@gnu.org; Fri, 25 May 2018 15:02:09 -0400 Received: from mail-qk0-f193.google.com ([209.85.220.193]:43586) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fMHyX-0005s9-Ot for grub-devel@gnu.org; Fri, 25 May 2018 15:02:05 -0400 Received: by mail-qk0-f193.google.com with SMTP id h19-v6so4847930qkj.10 for ; Fri, 25 May 2018 12:02:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=lhL+J0JQb4jzr8ji0dwhp2U1USuCLU28UvTi8akLVIk=; b=Zrf6ObqEBOc9j7aSGhUAclU2nKbOAm/LvGZgRevqfCKyYDm+BZ/HLwOZDhgANoIOLQ n+30CYDQUuE5mpwBmtZwKNrj/rB8O+ZacRZFAkPcMe7rgNQG/6tNeUGRVXIu3UNXNhhB 0k/SsOYL9gRVHQ4SmQgzbU5fB1Sj1Oe9KiIOPPS8vrIh/zTxiybvl2JMuF37BbM5H5Jb Loas7OSVFYh0dQmNHhacIG3TCAIb7OYNUgppgE22Zv4HahZg50/PJwbfc6urN4GTzvxb Wc1e6nduVbIYER2Dz+11U8t58PPgOsw9crgMa8HmR+zAprVEmOSNv2uMT0cUuEjJyABS ZFEA== X-Gm-Message-State: ALKqPwd8o8fw04Itzur2j+8YxhAm21e5aCe9Rbda2uvMqD491Hp8p6wr mTnbHyDoa5hbuvH0P3FT85gzO9f6BLufwrDKaMzDHB2U X-Google-Smtp-Source: ADUXVKKBcFWbmuggXm3bi5+avYisKWW+CZ/PNVx/q0ih6buj9wamcOHv5TL4L+jH+1IaMR98z9B5mpY7ndRd/l7yG+E= X-Received: by 2002:a37:bac4:: with SMTP id k187-v6mr3155545qkf.66.1527274924355; Fri, 25 May 2018 12:02:04 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a0c:8b4b:0:0:0:0:0 with HTTP; Fri, 25 May 2018 12:02:03 -0700 (PDT) In-Reply-To: <20180518115301.GA13069@router-fw-old.local.net-space.pl> References: <20180510171133.GK25320@router-fw-old.local.net-space.pl> <20180518115301.GA13069@router-fw-old.local.net-space.pl> From: Andrew Jeddeloh Date: Fri, 25 May 2018 12:02:03 -0700 Message-ID: Subject: Re: [PATCH] loader/i386/linux: calculate the size of the setup header To: Daniel Kiper Cc: grub-devel@gnu.org Content-Type: text/plain; charset="UTF-8" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.220.193 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 May 2018 19:02:11 -0000 Oops, my bad, didn't mean to drop the ML. So you're saying the setup header could expand to include some of pad7 in the future, but we error if it goes beyond that (into the e820 map)? Looking at bootparam.h it looks like the _most_ correct thing would be to stop at edd_mbr_sig_buffer, but grub doesn't have a corresponding field so e820_map seems good enough. Thanks for working through this. Here's the revised patch >From d46c75b4a9151c10e7f7809637f9f42a2c393c25 Mon Sep 17 00:00:00 2001 From: Andrew Jeddeloh Date: Tue, 8 May 2018 14:39:01 -0700 Subject: [PATCH] loader/i386/linux: calculate setup header length 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 --- grub-core/loader/i386/linux.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c index 44301e126..b5b65ea36 100644 --- a/grub-core/loader/i386/linux.c +++ b/grub-core/loader/i386/linux.c @@ -805,7 +805,19 @@ 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 byte value + * at 0x201. */ + len = *((char *)&linux_params.jump + 1) + 0x202; + + /* Verify the struct is big enough so we do not write past the end */ + if (len > (char *)&linux_params.e820_map - (char *)&linux_params) { + grub_error (GRUB_ERR_BAD_OS, "boot params setup header too big"); + goto fail; + } + + /* We've already read lh so there is no need to read it second time. */ + len -= sizeof(lh); + if (grub_file_read (file, (char *) &linux_params + sizeof (lh), len) != len) { if (!grub_errno) -- 2.14.1 On Fri, May 18, 2018 at 4:53 AM, Daniel Kiper wrote: > Re-added grub-devel. Next time please do not drop GRUB ML from the addresses. > > On Thu, May 17, 2018 at 03:40:32PM -0700, Andrew Jeddeloh wrote: >> Sorry about the long delay, I agree with all the sugguestions, except shouldn't > > No problem. > >> if (len > &linux_params.e820_map - &linux_params) >> >> be >> >> if (len > sizeof(linux_params)) >> >> since at that point len is the total size of the header which >> linux_params represents? > > Please take a look at arch/x86/include/uapi/asm/bootparam.h in latest > Linux kernel source. My proposal is better but it seems to me right now > that it is still too much. I have a feeling that we should not go beyond > the end of boot_params._pad7. Am I right? > > Daniel