[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: What is this grub_disk_read doing in the i386 linux loader?

From: Andrew Jeddeloh
Subject: Re: What is this grub_disk_read doing in the i386 linux loader?
Date: Tue, 24 Apr 2018 16:08:57 -0700

Thanks for the reply.

I'm not sure I follow. Looking over the 32 bit boot spec, it looks
like the process is:

1) zero out linux_params
 - grub does this
2) copy the linux boot params (from 0x1f1) into linux params
 - grub does this by reading from 0x0 until the end of lh, then
copying lh+0x1f1 til the end of lh into linux_headers [4][5]
3) Do the read/write/modify operations that would happen for a 16 bit boot
4) calculate the end of the the setup header
 - grub does not do this and I think assumes its just the end of the
linux_params struct
5) fill out the rest of the setup header things from the zero page doc [6]
 - AFAICT this isn't meant to be read from the kernel image, but
instead filled out by bootloader or left as zero
   - It looks like the current code is trying to do this but is just
assuming the end is at the end of linux_params.
 - Looking over the items in the zero page doc, there doesn't seem to
be anything that the kernel could supply a useful "default" value.

As I understand it, the read is not harmful (as it just gets
overwritten by the bootloader later) but also not needed. Is this your

I'll definitely submit a patch upstream once I figure out what should
be going on.

 - Andrew


On Tue, Apr 24, 2018 at 5:56 AM, Daniel Kiper <address@hidden> wrote:
> On Thu, Apr 19, 2018 at 03:22:55PM -0700, Andrew Jeddeloh wrote:
>> While solving a bug in the coreos fork of grub I came across this disk
>> read in the i386 linux loader [1]. It looks like its reading whatever
>> is after the boot param header in the kernel file (defined by the
>> linux x86 boot protocol [2]) into the rest of the `linux_params`
>> struct. In practice this means overwriting part of the padding and the
>> e820 map. As far as I can tell, this is not necessary or a useful
>> thing to do. Am I missing something?
>> The bug we were hitting on our fork was miscalculating
>> (char*)&linux_params + sizeoh(lh) as &linux_params + sizeof(lh), which
>> (in addition to corrupting memory) means the contents wasn't being
>> written to (char*)&linux_params + sizeof(lh). However the machines
>> seem to boot just fine when the memory corruption didn't cause
>> problems. If I nop out the call to read that chunk into
>> (char*)linux_params + sizeof(lh) it also seems to boot fine.
>> Is this intended? If so what is it doing? It dates back to the
>> original i386 linux loader support [3], but I can't figure out why
>> this would be intended.
> This is intended. Look at [2], 32-bit BOOT PROTOCOL paragraph. However,
> at least since commit 2001169 math is wrong. I think that len should be
> calculated according to Linux boot protocol spec. The destination for
> read should be &linux_params.setup_sects.
> May I ask you to provide relevant patch for upstream?
> Daniel
> [1] 
> [2]
> [3] 

reply via email to

[Prev in Thread] Current Thread [Next in Thread]