Re: Grub XFS normal.mod not found error

2023-08-17 Thread Daniel Kiper
Vacation time, sorry for late reply...

Adding Lidong...

On Fri, Jul 28, 2023 at 09:27:33AM +, Steve wrote:
> Hello,
>
> Last reply from you guys was on the 13th, and we have yet to see issue
> addressed. I, as a Distro maintainer, had to create a new repo where I held
> grub back to r499 as a result of this, not really ideal now is it ?
>
> Kindly update us on the situation and ETA for fix..

Lidong is working on the fix. It will be a part of the upcoming GRUB release...

Daniel



Re: Grub XFS normal.mod not found error

2023-07-13 Thread Daniel Kiper
Hi,

Adding Lidong and Marta...

Steve, thank you for the report.

On Wed, Jul 12, 2023 at 03:05:34PM +0300, TechXero wrote:
> Hello
>
> Sorry for posting this again there was a formatting issue (Whitext/White bg)
>
> I found a strange issue with XFS filesystem when used for `/boot`. Out of

Could you tell us which version of the XFS tools, xfsprogs package, did
you use to create this filesystem?

> <100%> it fails to install grub with the Calamares installer. However when the
> commit ef7850c757fb3dd2462a512cfa0ff19c89fcc0b1 (fs/xfs: Fix issues found 
> while
> fuzzing the XFS filesystem) gets reverted installing on XFS `/boot` works. You
> can reproduce it 100% with this ISO and a positive result is possible with 
> that
> ISO. Also not that installing it locally sometimes fails also but not so often
>
> Here are the ISOs same exact ISO only difference is one is using modded Grub
> with commit reverted and the other unmodded grub from upstream...
>
> * Unmodded original :
> * https://xerolinux.xyz/iso/dev/xerolinux-oem.iso
>
> * Modded ISO :
> * https://xerolinux.xyz/iso/dev/xerolinux-modded.iso
>
> Distro Base : ArchLinux

Lidong, could you take a look at this issue?

Daniel



[bug #58516] GRUB Local Privileges Escalation

2020-06-08 Thread Daniel Kiper
Update of bug #58516 (project grub):

 Privacy:  Public => Private


___

Reply to this item at:

  

___
  Message sent via Savannah
  https://savannah.gnu.org/




Re: [RFC 1/1] mkimage: revert "Align efi sections on 4k boundary"

2019-04-23 Thread Daniel Kiper
On Fri, Apr 19, 2019 at 09:11:50PM +0200, Jesús Diéguez Fernández wrote:
> El 19/4/19 a las 17:32, Julien ROBIN escribió:
> > Thanks Heinrich, so I tested with your approach and u-boot.bin.
> >
>
> [...]
>
> >
> >
> > /PS : may be making a summary of all architectures/configurations tests
> > tutorials would be useful ? Even if it's just QEMU (it would even be a
> > lot of work), and used on new releases or wide changes, or just from
> > time to time to see if no accident happened in any recent changes. It
> > would also be an awesome tutorial for people starting to deal with grub/
> >
>
> As a complete newbie, I can confirm that this is a real need. I recently
> contributed a small modification and didn't know exactly which
> compilation options I should enable. I finally used a configuration I
> found elsewhere, but that's inconsistent if the official way ignores
> some warnings or the other way around, treats all warnings as errors.
>
> I know that recently a travis configuration file was added to test that
> GRUB builds fine on different platforms. I think that improving it has
> more value than just creating a plain text documentation.
>
> The travis integration requires that the code must be hosted on github
> (or setup something like this https://stackoverflow.com/a/49019950 ).
> Would it be possible to set a remote copy of GRUB's repository on
> github? I mean, not my personal copy, but an official mirror, something
> like the linux kernel has.
> That would allow anyone to make trivial forks on their github account
> that can be tested with travis. To deal with the PR submissions on
> github, the kernel has a bot that automatically replies to people (see
> this example
> https://github.com/torvalds/linux/pull/663#issuecomment-474615610 ).
>
> This way around, the only documentation that should be added to
> grub-dev.texi would be this workflow:
>
> Fork from github -> make changes -> validate travis build -> send patch.

Thank you for the input. Right now we are focusing on the release.
However, after that we are going to change various things in the GRUB
project. We will be looking at GRUB code test automation, etc. too.
So, I hope that at some point we will have something similar to the
thing used by Linux kernel folks.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: [RFC 1/1] mkimage: revert "Align efi sections on 4k boundary"

2019-04-19 Thread Daniel Kiper
On Thu, Apr 18, 2019 at 05:14:58PM +0200, Heinrich Schuchardt wrote:
> On 4/18/19 12:41 PM, Daniel Kiper wrote:
> > On Wed, Apr 17, 2019 at 07:50:09AM +0200, Heinrich Schuchardt wrote:
> > > Since this commit a51f953f4ee8 ("mkimage: Align efi sections on 4k
> > > boundary") grubarm.efi crashes. Let's revert it.
> >
> > Everywhere or on a specific machines?
>
> I observed the issue on a TinkerBoard (Rockchip RK3288 CPU) and on
> qemu-arm-system -machine virt with the current Debian Buster GRUB.

Leif, Alex, could you take closer look at this thread? I would like to
know what to do with reported issues in it and in particular we should
get or not the patch itself.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: [PATCH 1/1] efi: avoid NULL dereference if FilePath is NULL

2019-04-18 Thread Daniel Kiper
On Wed, Apr 17, 2019 at 07:12:56AM +0200, Heinrich Schuchardt wrote:
> The UEFI specification allows LoadImage() to be called with a memory
> location only and without a device path. In this case FilePath will not be
> set in the EFI_LOADED_IMAGE_PROTOCOL.
>
> So in function grub_efi_get_filename() the device path argument may be
> NULL. As we cannot determine the device path in this case just return NULL
> from the function.
>
> Signed-off-by: Heinrich Schuchardt 
> Reviewed-by: Leif Lindholm 

Reviewed-by: Daniel Kiper 

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: [RFC 1/1] mkimage: revert "Align efi sections on 4k boundary"

2019-04-18 Thread Daniel Kiper
On Wed, Apr 17, 2019 at 07:50:09AM +0200, Heinrich Schuchardt wrote:
> Since this commit a51f953f4ee8 ("mkimage: Align efi sections on 4k
> boundary") grubarm.efi crashes. Let's revert it.

Everywhere or on a specific machines?

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: grub make error

2019-03-20 Thread Daniel Kiper
On Wed, Mar 20, 2019 at 01:19:30AM +, Chen, Farrah wrote:
> Thank you for great help!

You are welcome! Patch has been pushed.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: grub make error

2019-03-19 Thread Daniel Kiper
On Tue, Mar 19, 2019 at 09:51:20AM +, Colin Watson wrote:
> On Tue, Mar 19, 2019 at 01:21:02AM +, Chen, Farrah wrote:
> >Thank you for your advice, but I think my environment has met the minimal
> >build requirements.
> >And I tried again, there’s no issue with commit:
> >f8f35acb5b05d40e3707a9d2db9ede60023e4cac, this error just happened from
> >35b909062e7b334eb4af372be3260d0f62d85375.
>
> I've reproduced this in a CentOS 7.6 container.  I'll see what I can do.

Great! Thanks!

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: grub make error

2019-03-19 Thread Daniel Kiper
On Tue, Mar 19, 2019 at 01:21:02AM +, Chen, Farrah wrote:
> Hi,
>
> Thank you for your advice, but I think my environment has met the minimal 
> build requirements.
> And I tried again, there's no issue with commit: 
> f8f35acb5b05d40e3707a9d2db9ede60023e4cac, this error just happened from 
> 35b909062e7b334eb4af372be3260d0f62d85375.
>
> Info in INSTALL:
> If you use a development snapshot or want to hack on GRUB you may
> need the following.
>
> * Python 2.6 or later
> * Autoconf 2.63 or later
> * Automake 1.11 or later
>
> * GCC 4.1.3 or later
>   Note: older versions may work but support is limited
>
> My environment:
> ./configure --version
> GRUB configure 2.03
> generated by GNU Autoconf 2.69
>
> yum info automake
> Name: automake
> Arch: noarch
> Version : 1.13.4
> Release : 3.el7
> Size: 1.7 M
> Repo: installed
>
> python --version
> Python 2.7.5
>
> gcc --version
> gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-36)

Hmmm... Colin, could you take closer look at this issue?
I will try to reproduce it too.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: grub make error

2019-03-18 Thread Daniel Kiper
On Mon, Mar 18, 2019 at 02:53:10PM +, Chen, Farrah wrote:
> Sorry, I attached wrong log just now, if you received the email, please 
> ignore.
>
> Yes, I am using git://git.savannah.gnu.org/grub.
> I attached all the log, can you help to check? It's strange, before this 
> commit, I never met such error.
> Do I need to update or install any other tools?
> And my OS is " Red Hat Enterprise Linux Server release 7.6 (Maipo)", 
> actually, I tried it on two different machines with Red Hat, all failed.
> Thanks a lot!

I am afraid that this happens due to old version of Autoconf and/or
Automake. Please take a look at INSTALL file to get info about the
minimal build requirements.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: grub make error

2019-03-18 Thread Daniel Kiper
On Mon, Mar 18, 2019 at 09:29:13AM +, Colin Watson wrote:
> On Mon, Mar 18, 2019 at 01:45:23AM +, Chen, Farrah wrote:
> >Then the following error occurred:
> >
> >..
> >
> >In file included from ../grub-core/lib/gnulib/libc-config.h:150:0,
> >
> >from lib/gnulib/regex.c:21:
> >
> >lib/gnulib/regexec.c: In function ‘re_search_2_stub’:
> >
> >../grub-core/lib/gnulib/intprops.h:398:37: error: ‘SCHAR_MIN’ undeclared
> >(first use in this function)
> >
> >signed char, SCHAR_MIN, SCHAR_MAX) \
> >
> >^
>
> I'm prepared to believe that something has gone wrong here, but I can't
> reproduce it myself.  Could you please give some details of your build
> environment, and attach config.log?

I am not able to reproduce it too. Could you try this on clean
repository taken directly from git://git.savannah.gnu.org/grub.git?

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: Make grub error "too few arguments" with xen

2018-12-04 Thread Daniel Kiper
On Tue, Dec 04, 2018 at 05:49:24AM +, Chen, Farrah wrote:
> Hi all,
>
> When I make grub, I met error " too few arguments to function 
> 'grub_create_loader_cmdline'" with xen.
> I used git bisect and found the error occurred from commit: 
> 4d4a8c96e3593d76fe7b025665ccdecc70a53c1f.
> Do you have any ideas? Thanks a lot!
>
> commit 4d4a8c96e3593d76fe7b025665ccdecc70a53c1f
> Author: Vladimir Serbinenko 
> Date:   Tue Feb 7 02:10:14 2017 +0100
>
> verifiers: Add possibility to verify kernel and modules command lines
>
> Signed-off-by: Vladimir Serbinenko 
> Signed-off-by: Daniel Kiper 
> Reviewed-by: Ross Philipson 
>
> Make steps and Error log:
>
> cd grub
> ./autogen.sh
> ./configure --target=amd64 --with-platform=xen --prefix=${PWD}/../pvgrub2
> make
> ..
> loader/i386/xen.c: In function 'grub_cmd_xen':
> loader/i386/xen.c:650:10: error: too few arguments to function 
> 'grub_create_loader_cmdline'
>   sizeof (xen_state.next_start.cmd_line) - 1);
>   ^

[...]

Ugh... This is the fallout after verifiers framework introduction.
I will fix this and post the patches probably tomorrow. Sorry for
the confusion.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: [PATCH] arm64: fix build regression in of_path_of_nvme

2018-03-01 Thread Daniel Kiper
On Tue, Feb 27, 2018 at 09:19:23AM -0700, Eric Snowberg wrote:
> > On Feb 27, 2018, at 5:06 AM, Daniel Kiper <daniel.ki...@oracle.com> wrote:
> > On Mon, Feb 26, 2018 at 10:59:24AM +0100, Joakim Bech wrote:
> >> On Mon, Feb 26, 2018 at 10:45:46AM +0100, Daniel Kiper wrote:
> >>> On Mon, Feb 26, 2018 at 09:57:34AM +0100, Joakim Bech wrote:
> >>>> The of_path_of_nvme function introduced a build regression:
> >>>>grub-core/osdep/linux/ofpath.c:365:21: error: comparison between 
> >>>> pointer
> >>>>and zero character constant [-Werror=pointer-compare]
> >>>>   if ((digit_string != '\0') && (*part_end == 'p'))
> >>>>
> >>>> Update digit_string to compare against the char instead of the pointer.
> >>>>
> >>>> Signed-off-by: Joakim Bech <joakim.b...@linaro.org>
> >>>
> >>> CC-ing Vladimir.
> >>>
> >>> Pushed with some commit message changes. Thanks! However, this raises
> >>> the question: why this code is build on ARM? Should we do not do that?
> >>>
> >> My bad here Daniel, I was building for the HiKey device which uses the
> >> AArch64 GCC for most of its software components, but you are right, when
> >> checking closer to the output from our builds I can see that we are
> >> using the regular x86 toolchain when compiling grub. So a more correct
> >
> > 8-) Well... Eric, should not we build this code on SPARC only? I do not
> > think it makes much sense to have that thing on platforms not supporting
> > Open Firmware…
>
> Some of the ofpath code is common across all Open Firmware platforms.  There
> are some unique differences in naming that have come about over time. I don’t
> know how PPC or x86 do their NVMe Open Firmware path naming.  I left it this
> way since once those architectures added NVMe support, hopefully most of the
> code could be reused. This is similar to how SAS originally only worked on PPC
> and didn’t work on SPARC. If you want to make it SPARC only at the moment,
> I’m ok with that.

Yes, please post relevant patch for this.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: [PATCH] arm64: fix build regression in of_path_of_nvme

2018-02-27 Thread Daniel Kiper
On Mon, Feb 26, 2018 at 10:59:24AM +0100, Joakim Bech wrote:
> On Mon, Feb 26, 2018 at 10:45:46AM +0100, Daniel Kiper wrote:
> > On Mon, Feb 26, 2018 at 09:57:34AM +0100, Joakim Bech wrote:
> > > The of_path_of_nvme function introduced a build regression:
> > > grub-core/osdep/linux/ofpath.c:365:21: error: comparison between 
> > > pointer
> > > and zero character constant [-Werror=pointer-compare]
> > >if ((digit_string != '\0') && (*part_end == 'p'))
> > >
> > > Update digit_string to compare against the char instead of the pointer.
> > >
> > > Signed-off-by: Joakim Bech <joakim.b...@linaro.org>
> >
> > CC-ing Vladimir.
> >
> > Pushed with some commit message changes. Thanks! However, this raises
> > the question: why this code is build on ARM? Should we do not do that?
> >
> My bad here Daniel, I was building for the HiKey device which uses the
> AArch64 GCC for most of its software components, but you are right, when
> checking closer to the output from our builds I can see that we are
> using the regular x86 toolchain when compiling grub. So a more correct

8-) Well... Eric, should not we build this code on SPARC only? I do not
think it makes much sense to have that thing on platforms not supporting
Open Firmware...

> subject of my patch would have been:
>   "[PATCH] fix build regression in of_path_of_nvm"
>
> I apologize for not paying more attention to that before sending out the
> patch.

No problem, I know how it works.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: [PATCH] arm64: fix build regression in of_path_of_nvme

2018-02-26 Thread Daniel Kiper
On Mon, Feb 26, 2018 at 09:57:34AM +0100, Joakim Bech wrote:
> The of_path_of_nvme function introduced a build regression:
> grub-core/osdep/linux/ofpath.c:365:21: error: comparison between pointer
> and zero character constant [-Werror=pointer-compare]
>if ((digit_string != '\0') && (*part_end == 'p'))
>
> Update digit_string to compare against the char instead of the pointer.
>
> Signed-off-by: Joakim Bech 

CC-ing Vladimir.

Pushed with some commit message changes. Thanks! However, this raises
the question: why this code is build on ARM? Should we do not do that?

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: "ELF-Symbols" tag for relocatable images

2017-03-22 Thread Daniel Kiper
On Wed, Mar 22, 2017 at 04:43:35PM +0100, Rodrigo Vali??a Guti??rrez wrote:
> >> They also may not match if virtual address != physical address, but as
> >> we do not establish any address translation when launching image, this
> >> probably is going to fail. Still would be good to have this assumption
> >> explicitly listed in multiboot2 manual.
> >
> > I think that we should state in multiboot2 spec that physical address ==
> > virtual address in ELF.
>
> That may be true (that is going to fail) for entry point address, but
> please note that many kernels have the entry point and bootstrap code
> in a section/segment with virtual == physical, but then setup address
> translation and jump to another sections/segments with virtual !=
> physical addresses.

OK, even if we assume that virtual and physical addresses can be different
in the ELF header I think that the bootloader should always put into sh_addr
(maybe others) physical addresses. Usually it (the bootloader) does not have
sufficient knowledge to properly calculate correct kernel virtual addresses.
However, kernel is sufficiently smart to translate physical addresses to
virtual ones.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: "ELF-Symbols" tag for relocatable images

2017-03-22 Thread Daniel Kiper
On Wed, Mar 22, 2017 at 03:50:44PM +0300, Andrei Borzenkov wrote:
> On Wed, Mar 22, 2017 at 3:07 PM, Daniel Kiper <dki...@net-space.pl> wrote:
> > Hi,
> >
> > Sorry for late reply but I am busy.
> >
> > On Sun, Mar 19, 2017 at 12:02:38PM +0300, Andrei Borzenkov wrote:
> >> 17.03.2017 22:53, Ahmed, Safayet (GE Global Research, US) ??:
> >> > Hello again,
> >> >
> >> > I had a question on the function, "grub_multiboot_load_elf(32/64)".
> >> > (grub/grub_core/loader/multiboot_elfxx.c: line 54)
> >> >
> >> > As a part of parsing an ELF image, the above-named function copies
> >> > the section header table into memory, and copies "unloaded" sections
> >> > into memory (lines 199 - 269). The section table may be passed to an
> >> > OS image as the "ELF-Symbols" tag of the multiboot2 information
> >> > structure.
> >> >
> >> > Section 2.6.7 of the specification states that "the physical address
> >> > fields of the ELF section header then refer to where the sections are
> >> > in memory".
> >> >
> >> > Sections that are loaded are handled differently in the code from
> >> > sections that are not loaded. This distinction is made at line  234.
> >> > The loaded sections are ignored.
> >> >
> >> > The "sh_addr" field of entries in the table for not-loaded sections
> >> > are explicitly updated to point to the address where those sections
> >> > are copied (line 265).
> >> >
> >> > For "loaded" sections loaded to a fixed address, the "sh_addr" field
> >> > of the section header table entries should be accurate without any
> >> > updates. However, if the image is relocated, the "sh_addr" field of
> >> > the entries for relocated sections are not necessarily accurate.
> >
> > I am not sure that I understand this correctly. Do you mean that sh_addr
> > for a given section in memory differs from its sh_addr in the image? I think
> > that it is OK.
>
> Specification claims that sh_addr is physical section address in
> memory. If you agree that sh_addr may not match physical address,
> specification must be updated to reflect reality.

You are right, implementation deviates from spec. We can fix it but it is
not one liner fix. And, IMO, this is not critical bug. So, we should not
change this before GRUB2 2.02 release.

> > We care more about what is in the mem then in the image here.
>
> You seem to misunderstand. Before introduction of relocatable images
> ELF image was loaded at (segment) physical address, which presumably
> matches (segment) virtual address; sh_addr is virtual section address

IIRC, I have not seen physical address == virtual address assumption in
multiboot2 spec. At least in regards to ELF itself. So, I think this
should be fixed in spec.

> which in this case matches physical address memory.
>
> Relocatable image can presumably be loaded everywhere, so its load
> address != segment physical address anymore, so sh_addr also do not
> match.

Right. This begs code fix.

> They also may not match if virtual address != physical address, but as
> we do not establish any address translation when launching image, this
> probably is going to fail. Still would be good to have this assumption
> explicitly listed in multiboot2 manual.

I think that we should state in multiboot2 spec that physical address ==
virtual address in ELF.

> >> > Is this a legitimate concern?
> >>
> >> Yes. @Daniel, note that tags 9, 10 are not even documented.
> >
> > Do you mean MULTIBOOT_TAG_TYPE_ELF_SECTIONS and MULTIBOOT_TAG_TYPE_APM?
>
> No, I mean
>
> #define MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64  9
> #define MULTIBOOT_HEADER_TAG_RELOCATABLE  10

They are documented in line 563 and 686.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: "ELF-Symbols" tag for relocatable images

2017-03-22 Thread Daniel Kiper
Hi,

Sorry for late reply but I am busy.

On Sun, Mar 19, 2017 at 12:02:38PM +0300, Andrei Borzenkov wrote:
> 17.03.2017 22:53, Ahmed, Safayet (GE Global Research, US) ??:
> > Hello again,
> >
> > I had a question on the function, "grub_multiboot_load_elf(32/64)".
> > (grub/grub_core/loader/multiboot_elfxx.c: line 54)
> >
> > As a part of parsing an ELF image, the above-named function copies
> > the section header table into memory, and copies "unloaded" sections
> > into memory (lines 199 - 269). The section table may be passed to an
> > OS image as the "ELF-Symbols" tag of the multiboot2 information
> > structure.
> >
> > Section 2.6.7 of the specification states that "the physical address
> > fields of the ELF section header then refer to where the sections are
> > in memory".
> >
> > Sections that are loaded are handled differently in the code from
> > sections that are not loaded. This distinction is made at line  234.
> > The loaded sections are ignored.
> >
> > The "sh_addr" field of entries in the table for not-loaded sections
> > are explicitly updated to point to the address where those sections
> > are copied (line 265).
> >
> > For "loaded" sections loaded to a fixed address, the "sh_addr" field
> > of the section header table entries should be accurate without any
> > updates. However, if the image is relocated, the "sh_addr" field of
> > the entries for relocated sections are not necessarily accurate.

I am not sure that I understand this correctly. Do you mean that sh_addr
for a given section in memory differs from its sh_addr in the image? I think
that it is OK. We care more about what is in the mem then in the image here.

> > Is this a legitimate concern?
>
> Yes. @Daniel, note that tags 9, 10 are not even documented.

Do you mean MULTIBOOT_TAG_TYPE_ELF_SECTIONS and MULTIBOOT_TAG_TYPE_APM?
They are. Please take a look at line 1036 and 1121 in doc/multiboot.texi.

> Unfortunately I suspect updating sh_addr may not be enough - this would
> require updating every reference to this section address everywhere
> else; not sure if this is really possible.
>
> > Alternatively, should the section
> > header table be absent from ELF images that contain the "relocatable
> > tag" in the multiboot2 header? Under normal circumstances, the
> > section header table isn't really necessary for loading.
>
> I guess enforcing it is the more straightforward choice.

You should be aware that multiboot protocols does not require ELF file
at all. It can be PE file or even anything which does not look like well
known executable. So, then there is no section headers at all. However,
if you do not use ELF container then you need provide all data, e.g. entry
point, in multiboot/multiboot2 header(s) which cannot be established other
way (I mean from ELF header).

Currently, there is no support for SHT_REL and SHT_RELA sections. OS
image has to take care about relocation itself. It has all data to
do that. You can take a look how it is done in Xen source
(xen/arch/x86/boot/head.S). I am attaching relevant patches.

If you have any questions drop me a line.

Daniel


xen_efi_multiboot2_20170310.tar.gz
Description: Binary data
___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub


Re: Tag-alignment in multiboot2 image headers

2017-03-09 Thread Daniel Kiper
Hi all,

On Thu, Mar 09, 2017 at 09:34:34PM +, Ahmed, Safayet (GE Global Research, 
US) wrote:
> Thank you all for the fast response.
>
> 8-byte alignment makes sense. I was looking at another implementation of 
> multiboot2:
> https://github.com/todorez/tummiboot
>
> I thought this implementation was using a 2-byte alignment. On reviewing the 
> code, I realized I misread it.
>
> Thank you for the clarification.
>
> Safayet
>
> From: Vladimir 'phcoder' Serbinenko [mailto:phco...@gmail.com]
> Sent: Thursday, March 09, 2017 1:37 AM
> To: Andrey Borzenkov <arvidj...@gmail.com>; Ahmed, Safayet (GE Global 
> Research, US) <safayet.ah...@ge.com>; Daniel Kiper <daniel.ki...@oracle.com>

Hmmm... It is interesting. I have not received original email...
So, sorry for late reply.

> Cc: bug-grub@gnu.org
> Subject: EXT: Re: Tag-alignment in multiboot2 image headers
>
>
> On Wed, Mar 8, 2017, 22:28 Andrei Borzenkov 
> <arvidj...@gmail.com<mailto:arvidj...@gmail.com>> wrote:
> On Thu, Mar 9, 2017 at 1:17 AM, Ahmed, Safayet (GE Global Research,
> US) <safayet.ah...@ge.com<mailto:safayet.ah...@ge.com>> wrote:
> > Hello,
> >
> > I'm seeing an inconsistency in the multiboot2 specification and the 
> > implementation of the multiboot2 loader code in GRUB. It may be my 
> > understanding that's incorrect. A clarification would be appreciated.
> >
> > This concerns the alignment requirements for tags in OS image headers. The 
> > specification states 4 bytes, but the code uses 8 bytes.
> >
> > The specification states (Section 3.1.3) that "Tags constitutes a buffer of 
> > structures following each other padded on 'u32' size."
>
> This is ambiguous and needs better wording as well (it is not clear
> whether "padded" here applies to individual tag or all tags block).

We have to fix the spec. I will do that tomorrow.

> > The "for" loop for parsing tags uses the following "increment" statement 
> > (grub/grub_core/loader/multiboot_mbi2.c: line 148):
> > tag = (struct multiboot_header_tag *) ((grub_uint32_t *) tag + ALIGN_UP 
> > (tag->size, MULTIBOOT_TAG_ALIGN) / 4))
> >
> > The macro MULTIBOOT_TAG_ALIGN is defined in (include/multiboot2.h) as 8. 
> > This alignment value is consistent with the specification for tags in the 
> > multiboot2 information structure, but not for tags in an OS image header.

As I can see GRUB2 code properly enforces alignment for both.
Though spec is really unclear. So, I will fix it.

> Yes, it sure looks wrong. Thanks for making us aware!
>
> @Vladimir, @Daniel - I think this is 2.02 material, we do not want
> release with such inconsistency. The question is what needs fixing
> though - about half of all tags are not multiple of 8 bytes, so I
> expect people to hit it in real life. What is current implementation
> in MB2 compliant kernels?
> The intention was 8, so that we can have u64 naturally aligned in the
> header even on non-x86. I believe all current kernels are compatible
> with grub, so I think we should update the docs. Daniel, your opinion?

I agree. I will do it tomorrow. Though, IMO, it should not delay RC2 cutting
because spec changes go to separate branch.

Daniel

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub