Re: kern/55958: pciback.hide parsing error

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

The following reply was made to PR kern/55958; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: Aleksey Arens <aza.sea.agenda%gmail.com@localhost>
Cc: jnemeth%cue.bc.ca@localhost, kern-bug-people%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
        netbsd-bugs%netbsd.org@localhost, gnats-bugs%netbsd.org@localhost
Subject: Re: kern/55958: pciback.hide parsing error
Date: Fri, 29 Jan 2021 11:19:52 +0100

 On Thu, Jan 28, 2021 at 10:33:46PM -0800, Aleksey Arens wrote:
 > 
 > Thank you for joining the discussion.  One of the essential points in
 > my previous message is that I suspect that there is at least a bug in
 > the multiboot implementation in /boot.  I'm also concerned about the
 > fact that the same data-structures are reused in implementing the
 > binary module layout for the cases when a native module is loaded and
 > when a multiboot module is loaded.  I hope that these points would
 > become clear to you if you invest your time and give your full
 > attention to the entire message.
 
 there's something that doesn't match here. A NetBSD dom0 can be loaded
 either from grub, pxelinux or NetBSD's boot loader.
 When started as dom0 (or as domU, the code path is the same), the kernel
 command line (which is the module command line in multiboot, when started
 as dom0) is in start_info.cmd_line as is a plain text string, not a
 btinfo_modulelist or bi_modulelist_entry.
 See xen_parse_cmdline() in xen_machdep.c
 
 
 
 > 
 > Finally, the command passed to the module -- in the example that you
 > had displayed -- is a bit too short.  To see the issue at hand, you'd
 > need to make it over 80 characters long, and then observe that it does
 > get truncated once it makes it to Xen.  This was demonstrated in the
 > first three messages to this PR.
 > 
 > > The "load" part is what does module loading (in this case, the
 > > NetBSD kernel which is built as a Xen module) and the "multiboot"
 > > part is what boots the Xen kernel.  Note that since it is my
 > > development box, I have many different variants of this line.
 > >
 > 
 > Could we please go into more details thereby?  The Xen expects the
 > modules to be loaded according to multiboot protocol.  What I'm seeing
 > is that "load" uses the same BM_TYPE_KMOD for either the case of
 > native or multiboot module loading scenario.  Furthermore:
 >   https://nxr.netbsd.org/xref/src/sys/arch/i386/stand/lib/exec.c#exec_multiboot
 >   https://nxr.netbsd.org/xref/src/sys/arch/i386/stand/lib/exec.c#exec_netbsd
 > are both calling to the same
 >   https://nxr.netbsd.org/xref/src/sys/arch/i386/stand/lib/exec.c#module_init
 > function, in which the short buffer is copied:
 >   https://nxr.netbsd.org/xref/src/sys/arch/i386/stand/lib/exec.c#763
 > 
 > So, clearly, both native and multiboot modules are initialized in an
 > identical fashion.  This must have led to this bug.  I request that
 > this bug be fixed, and I propose that it be done by separating the
 > code responsible for module loading in case of multiboot and Xen.
 
 This is not exactly how it works. Yes, the load command is parsed the
 same way for both native and multiboot case, using the same structure. But
 then the exec_multiboot1() function will convert the native structure
 to multiboot format. mmo_string points to bim->path, not the bim
 structure itself.
 
 So to fix the problem no change to bootinfo.h is needed; only the
 boot loader needs to be changed to pass the full string to exec_multiboot1().
 
 For example, instead of reusing bi_modulelist to represent the module,
 module_init() could use a private structure, which is then converted
 to the appropriate interface depending on the boot mode.
 
 -- 
 Manuel Bouyer <bouyer%antioche.eu.org@localhost>
      NetBSD: 26 ans d'experience feront toujours la difference
 --