Re: port-i386/60992: efiboot: multiboot2 MODULE tags all describe the first module

["Andrius V via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR port-i386/60992; it has been noted by GNATS.

From: Andrius V <vezhlys%gmail.com@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: nao.nonaka%gmail.com@localhost
Subject: Re: port-i386/60992: efiboot: multiboot2 MODULE tags all describe the
 first module
Date: Thu, 12 Nov 2026 00:17:52 +0200

 On Wed, Nov 11, 2026 at 3:30=E2=80=AFPM nao.nonaka%gmail.com@localhost via gnats
 <gnats-admin%netbsd.org@localhost> wrote:
 >
 > >Number:         60992
 > >Category:       port-i386
 > >Synopsis:       efiboot: multiboot2 MODULE tags all describe the first m=
 odule
 > >Confidential:   no
 > >Severity:       serious
 > >Priority:       medium
 > >Responsible:    port-i386-maintainer
 > >State:          open
 > >Class:          sw-bug
 > >Submitter-Id:   net
 > >Arrival-Date:   Wed Nov 11 13:30:00 +0000 2026
 > >Originator:     Naomichi Nonaka
 > >Release:        NetBSD-current
 > >Organization:
 > >Environment:
 > Not a running NetBSD system: the boot loaders were cross-built from
 > src with the NetBSD tools (build.sh -m amd64 tools, then nbmake-amd64
 > in sys/arch/i386/stand/efiboot) on Linux 6.8.0 x86_64, and run under
 > qemu-system-x86_64 with OVMF (both the ia32 and the x64 firmware).
 > bootia32.efi and bootx64.efi are both affected, the latter being built
 > from the same source.
 >
 > >Description:
 > mbi_modules() in sys/arch/i386/stand/lib/exec_multiboot2.c emits one
 > MULTIBOOT_TAG_TYPE_MODULE tag per entry of btinfo_modulelist, but never
 > advances bim:
 >
 >         bim =3D (struct bi_modulelist_entry *)(btinfo_modulelist + 1);
 >         for (i =3D 0; i < btinfo_modulelist->num; i++) {
 >                 size_t pathlen =3D strlen(bim->path) + 1;
 >                 ...
 >                         mbt->mod_start =3D bim->base;
 >                         mbt->mod_end =3D bim->base + bim->len;
 >                         strncpy(mbt->cmdline, bim->path, pathlen);
 >
 >                         mbt =3D (struct multiboot_tag_module *)
 >                             ((char *)mbt + mbt_len_align);
 >                 }
 >         }
 >
 > Only mbt moves on.  Every tag therefore carries the first module's base,
 > length and path, repeated btinfo_modulelist->num times, and a multiboot2
 > kernel started by "multiboot" with more than one module is told that all
 > of its modules are the first one.
 >
 > module_init() in exec.c loads the modules correctly, to distinct
 > addresses, and records them correctly in btinfo_modulelist; it is only
 > the tags that report them which are wrong.  Nothing warns, and the total
 > size computed in the first pass is right (it adds up the same path length
 > num times rather than each path's), so the information structure is
 > well-formed -- just wrong.
 >
 > A single-module boot is unaffected, which is presumably why this has
 > stood: that is the common case for loading one kernel module.
 > >How-To-Repeat:
 > With an EFI multiboot2 kernel and two or more modules:
 >
 >         load /module-a
 >         load /module-b
 >         multiboot /kernel
 >
 > Build efiboot with -DMULTIBOOT2_DEBUG and multiboot2_info_dump() prints
 > the tag list it is about to pass: the MODULE tags all show the same
 > mod_start, mod_end and cmdline, those of /module-a.
 >
 > What I was doing when I found it: booting L4Ka::Pistachio, whose loader
 > (util/kickstart) takes kernel, sigma0 and root task as its first three
 > modules.  It ELF-loaded the kernel three times -- the second "module"
 > overlapping the image it had just written -- and stopped.  With the fix
 > below, that system boots to its test suite under both bootia32.efi and
 > bootx64.efi.
 >
 > >Fix:
 > Advance bim with i.  Against revision 1.10:
 >
 > --- sys/arch/i386/stand/lib/exec_multiboot2.c.orig
 > +++ sys/arch/i386/stand/lib/exec_multiboot2.c
 > @@ -514,7 +514,7 @@ mbi_modules(struct multiboot_package *mb
 >         len =3D 0;
 >
 >         bim =3D (struct bi_modulelist_entry *)(btinfo_modulelist + 1);
 > -       for (i =3D 0; i < btinfo_modulelist->num; i++) {
 > +       for (i =3D 0; i < btinfo_modulelist->num; i++, bim++) {
 >                 size_t pathlen =3D strlen(bim->path) + 1;
 >                 size_t mbt_len =3D sizeof(*mbt) + pathlen;
 >                 size_t mbt_len_align =3D roundup(mbt_len, MULTIBOOT_TAG_A=
 LIGN);
 >
 
 Committed the fix. Thank you for reporting this! Indeed looks like
 just an oversight.
 
 Regards,
 Andrius V