| Message ID | 87pqfja28l.fsf@industria.weinholt.se |
|---|---|
| State | New |
| Headers | show |
On 20.12.2011, at 12:53, Göran Weinholt wrote: > Alexander Graf <agraf@suse.de> writes: > >> On 19.12.2011, at 23:01, Anthony Liguori wrote: >> >> On 12/19/2011 11:35 AM, Alexander Graf wrote: >> >> On 24.07.2011, at 17:55, Göran Weinholt wrote: >> >> Multiboot images can specify a bss segment. The boot loader must >> clear >> >> the memory of the bss and ensure that no modules or structures are >> >> allocated inside it. Several fields are provided in the Multiboot >> >> header that were previously not used properly. The header is now >> used >> >> to determine how much data should be read from the image and how >> much >> >> memory should be reserved to the bss segment. >> >> This patch breaks the OSX booter: >> >> http://people.exactcode.de/~rene/mac/boot >> >> How is this licensed? Is there source available? >> >> Yes, it's the XNU booter, but binaries are easier to debug when it comes to >> reading out the bootloader info: >> >> http://tgwbd.org/darwin/boot.html >> >> I don't know if the binary above is 1:1 built from those sources, but it's >> definitely close enough. > > I was unable to build that program from source, but I found a pre-built > binary that triggered the same bug. Please test the following patch. > > > Subject: [PATCH] multiboot: mh_load_end_addr and mh_bss_end_addr may be zero > > There are two special cases in the address fields of the multiboot > format. If mh_load_end_addr is zero then the whole image file should > be loaded and if mh_bss_end_addr is zero then there is no bss segment. > With this change it is again possible to boot kernels where these > fields are zero. > > Signed-off-by: Göran Weinholt <goran@weinholt.se> Yes, this patch makes things work again :). Thanks a lot! The only thing I could nitpick on would be the coding style - checkpatch.pl complains :). Could you please resend with braces? Justin, Please also queue this for 1.0-stable when it comes in its final form. Tested-by: Alexander Graf <agraf@suse.de> Alex > --- > hw/multiboot.c | 13 ++++++++++++- > 1 files changed, 12 insertions(+), 1 deletions(-) > > diff --git a/hw/multiboot.c b/hw/multiboot.c > index b4484a3..a43d56c 100644 > --- a/hw/multiboot.c > +++ b/hw/multiboot.c > @@ -202,10 +202,21 @@ int load_multiboot(void *fw_cfg, > uint32_t mh_bss_end_addr = ldl_p(header+i+24); > mh_load_addr = ldl_p(header+i+16); > uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr); > - uint32_t mb_load_size = mh_load_end_addr - mh_load_addr; > + uint32_t mb_load_size; > + > + /* A load end address of zero indicates that the whole file > + * should be loaded. */ > + if (!mh_load_end_addr) > + mh_load_end_addr = kernel_file_size + mh_load_addr; > + > + /* A bss end address of zero indicates that there is no bss > + * segment. */ > + if (!mh_bss_end_addr) > + mh_bss_end_addr = mh_load_end_addr; WARNING: braces {} are necessary for all arms of this statement #83: FILE: hw/multiboot.c:209: + if (!mh_load_end_addr) [...] WARNING: braces {} are necessary for all arms of this statement #88: FILE: hw/multiboot.c:214: + if (!mh_bss_end_addr) [...]
diff --git a/hw/multiboot.c b/hw/multiboot.c index b4484a3..a43d56c 100644 --- a/hw/multiboot.c +++ b/hw/multiboot.c @@ -202,10 +202,21 @@ int load_multiboot(void *fw_cfg, uint32_t mh_bss_end_addr = ldl_p(header+i+24); mh_load_addr = ldl_p(header+i+16); uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr); - uint32_t mb_load_size = mh_load_end_addr - mh_load_addr; + uint32_t mb_load_size; + + /* A load end address of zero indicates that the whole file + * should be loaded. */ + if (!mh_load_end_addr) + mh_load_end_addr = kernel_file_size + mh_load_addr; + + /* A bss end address of zero indicates that there is no bss + * segment. */ + if (!mh_bss_end_addr) + mh_bss_end_addr = mh_load_end_addr; mh_entry_addr = ldl_p(header+i+28); mb_kernel_size = mh_bss_end_addr - mh_load_addr; + mb_load_size = mh_load_end_addr - mh_load_addr; /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE. uint32_t mh_mode_type = ldl_p(header+i+32);