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] http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/loader/i386/linux.c#n809 [2] https://www.kernel.org/doc/Documentation/x86/boot.txt [3] http://git.savannah.gnu.org/cgit/grub.git/commit/grub-core/loader/i386/linux.c?id=8c411768822a75c8c15108872191a05e84befa6e _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
