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

[Andrew Jeddeloh]

> As Michael said, "0x0202 + byte value at offset 0x0201" is your friend.

I think Michael's message got lost; I think he replied to just you? I
didn't receive it and I don't see it in the archives.

> AIUI it is. Please look above.

I don't know what you're refering to here by "above".

> This is wrong due to grub_e820_mmap.

My understanding is wrong or grub's behavior is wrong? Can you elaborate please?

> This read is needed

I don't understand why. Again, looking at the zero page documentation,
there's nothing after the 16 bit boot params that the linux image
could know ahead of time. Is your concern that future fields could be
added that do need to be read?

Regardless, we can't calculate then length then blindly use it reading
into a struct. If the length grows larger than the size of the struct
we'll start corrupting memory when doing the read. Assuming the read
is needed, we'll need to malloc the amount that still needs to be read
then probably copy that into a struct to modify (and ignore the bits
we don't understand in the case of a new Linux release adding fields
grub doesn't have yet).

Sorry for all the questions, I want to ensure that I understand what's
supposed to be happening first.

Thanks,
 - Andrew

On Wed, Apr 25, 2018 at 1:59 AM, Daniel Kiper <address@hidden> wrote:
> On Tue, Apr 24, 2018 at 04:08:57PM -0700, Andrew Jeddeloh wrote:
>> 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
>
> As Michael said, "0x0202 + byte value at offset 0x0201" is your friend.
>
>> 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
>
> AIUI it is. Please look above.
>
>>    - It looks like the current code is trying to do this but is just
>> assuming the end is at the end of linux_params.
>
> This is wrong due to grub_e820_mmap.
>
>>  - 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.
>
> GRUB have to load the rest of Linux header into linux_params.
> Current math is wrong and have to be fixed. Please look above
> and think about newer versions of Linux headers which GRUB is
> not aware of.
>
>> 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
>> understanding?
>
> This read is needed, however, math has to be fixed. Additionally,
> I think that you can take a look at SYSLINUX and do something
> similar in GRUB.
>
>> I'll definitely submit a patch upstream once I figure out what should
>> be going on.
>
> Great! Thanks!
>
> Daniel