Re: Fixing the ELF priorities
Le 12/07/2014 00:42, Paul Goyette a écrit : > I run without _any_ EXEC_* or COMPAT_* module compiled into my kernel. ALL of > my EXEC_* and COMPAT_* modules are auto-loaded as needed from the file system. > > Please don't break this! :) Well, netbsd32 already breaks the module loader, because of a missing dependency. It can crash the system, and not a friendly way (fixed in -r1.4). So... > My currently auto-loaded modules as reported by modstat(8) include > > exec_script > exec_elf32 > exec_elf64 > compat > compat_linux Feel free to test my patch
Re: Fixing the ELF priorities
I run without _any_ EXEC_* or COMPAT_* module compiled into my kernel.
ALL of my EXEC_* and COMPAT_* modules are auto-loaded as needed from the
file system.
Please don't break this! :)
My currently auto-loaded modules as reported by modstat(8) include
exec_script
exec_elf32
exec_elf64
compat
compat_linux
On Fri, 11 Jul 2014, Maxime Villard wrote:
Le 05/07/2014 16:58, Maxime Villard a ?crit :
[...]
My approach was wrong. #if(ARCH_ELFSIZE != 64) doesn't remove the exec_elf32
module, since it is still listed in distrib/sets/lists/modules/md.amd64. It
just removes exec_elf32_modcmd() from the module.
If the module is built in (GENERIC kernel) then there's no problem, but if it
is loaded from the fs (with modload) it won't work, since the kernel doesn't
know what to do with a module that has no _modcmd().
If loaded from the filesystem, this module will make available all the 32bit
functions used by netbsd32 and linux32. Which means that we just can't delete
it. However, we can make it dormant (from an execsw point of view), so that it
won't interfere with the 32bit binaries.
I was nonetheless right on another point: there's an inconsistency in the
module dependency of netbsd32 and linux32; but it's fixed now.
Here is a working, clean patch tested by me and njoly@ on
linux/linux32/netbsd32.
exec_elf32 is still there, but not plugged in the global exec array. It is
just here to provide 32bit functions to linux32 and netbsd32.
I'll commit this, unless anyone disagrees.
Index: compat/netbsd32/netbsd32_mod.c
===
RCS file: /cvsroot/src/sys/compat/netbsd32/netbsd32_mod.c,v
retrieving revision 1.3
diff -u -r1.3 netbsd32_mod.c
--- compat/netbsd32/netbsd32_mod.c 7 Mar 2014 01:33:43 - 1.3
+++ compat/netbsd32/netbsd32_mod.c 11 Jul 2014 12:13:44 -
@@ -81,7 +81,7 @@
.elf_probe_func = netbsd32_elf32_probe,
},
.es_emul = &emul_netbsd32,
- .es_prio = EXECSW_PRIO_FIRST,
+ .es_prio = EXECSW_PRIO_ANY,
.es_arglen = ELF32_AUXSIZE,
.es_copyargs = netbsd32_elf32_copyargs,
.es_setregs = NULL,
Index: compat/linux32/common/linux32_mod.c
===
RCS file: /cvsroot/src/sys/compat/linux32/common/linux32_mod.c,v
retrieving revision 1.5
diff -u -r1.5 linux32_mod.c
--- compat/linux32/common/linux32_mod.c 7 Mar 2014 01:33:43 - 1.5
+++ compat/linux32/common/linux32_mod.c 11 Jul 2014 12:13:44 -
@@ -67,7 +67,7 @@
.elf_probe_func = linux32_elf32_probe,
},
.es_emul = &emul_linux32,
- .es_prio = EXECSW_PRIO_FIRST,
+ .es_prio = EXECSW_PRIO_ANY,
.es_arglen = LINUX32_ELF_AUX_ARGSIZ,
.es_copyargs = linux32_elf32_copyargs,
.es_setregs = NULL,
Index: kern/exec_elf32.c
===
RCS file: /cvsroot/src/sys/kern/exec_elf32.c,v
retrieving revision 1.140
diff -u -r1.140 exec_elf32.c
--- kern/exec_elf32.c 7 Apr 2014 17:02:15 - 1.140
+++ kern/exec_elf32.c 11 Jul 2014 12:13:44 -
@@ -59,7 +59,7 @@
.elf_probe_func = netbsd_elf32_probe,
},
.es_emul = &emul_netbsd,
- .es_prio = EXECSW_PRIO_ANY,
+ .es_prio = EXECSW_PRIO_FIRST,
.es_arglen = ELF32_AUXSIZE,
.es_copyargs = elf32_copyargs,
.es_setregs = NULL,
@@ -87,7 +87,28 @@
static int
exec_elf32_modcmd(modcmd_t cmd, void *arg)
{
+#if ARCH_ELFSIZE == 64
+ /*
+* If we are on a 64bit system, we don't want the 32bit execsw[] to be
+* added in the global array, because the exec_elf32 module only works
+* on 32bit systems.
+*
+* However, we need the exec_elf32 module, because it will make the
32bit
+* functions available for netbsd32 and linux32.
+*
+* Therefore, allow this module on 64bit systems, but make it dormant.
+*/
+ (void)exec_elf32_execsw; /* unused */
+
+ switch (cmd) {
+ case MODULE_CMD_INIT:
+ case MODULE_CMD_FINI:
+ return 0;
+ default:
+ return ENOTTY;
+ }
+#else
switch (cmd) {
case MODULE_CMD_INIT:
return exec_add(exec_elf32_execsw,
@@ -100,4 +121,5 @@
default:
return ENOTTY;
}
+#endif /* ARCH_ELFSIZE == 64 */
}
Index: kern/exec_elf64.c
===
RCS file: /cvsroot/src/sys/kern/exec_elf64.c,v
retrieving revision 1.5
diff -u -r1.5 exec_elf64.c
--- kern/exec_elf64.c 7 Mar 2014 01:34:29 - 1.5
+++ kern/exec_elf64.c 11 Jul 2014 12:13:44 -
@@ -60,7 +60,
Re: Fixing the ELF priorities
Le 05/07/2014 16:58, Maxime Villard a écrit :
>
> [...]
>
My approach was wrong. #if(ARCH_ELFSIZE != 64) doesn't remove the exec_elf32
module, since it is still listed in distrib/sets/lists/modules/md.amd64. It
just removes exec_elf32_modcmd() from the module.
If the module is built in (GENERIC kernel) then there's no problem, but if it
is loaded from the fs (with modload) it won't work, since the kernel doesn't
know what to do with a module that has no _modcmd().
If loaded from the filesystem, this module will make available all the 32bit
functions used by netbsd32 and linux32. Which means that we just can't delete
it. However, we can make it dormant (from an execsw point of view), so that it
won't interfere with the 32bit binaries.
I was nonetheless right on another point: there's an inconsistency in the
module dependency of netbsd32 and linux32; but it's fixed now.
Here is a working, clean patch tested by me and njoly@ on
linux/linux32/netbsd32.
exec_elf32 is still there, but not plugged in the global exec array. It is
just here to provide 32bit functions to linux32 and netbsd32.
I'll commit this, unless anyone disagrees.
Index: compat/netbsd32/netbsd32_mod.c
===
RCS file: /cvsroot/src/sys/compat/netbsd32/netbsd32_mod.c,v
retrieving revision 1.3
diff -u -r1.3 netbsd32_mod.c
--- compat/netbsd32/netbsd32_mod.c 7 Mar 2014 01:33:43 - 1.3
+++ compat/netbsd32/netbsd32_mod.c 11 Jul 2014 12:13:44 -
@@ -81,7 +81,7 @@
.elf_probe_func = netbsd32_elf32_probe,
},
.es_emul = &emul_netbsd32,
- .es_prio = EXECSW_PRIO_FIRST,
+ .es_prio = EXECSW_PRIO_ANY,
.es_arglen = ELF32_AUXSIZE,
.es_copyargs = netbsd32_elf32_copyargs,
.es_setregs = NULL,
Index: compat/linux32/common/linux32_mod.c
===
RCS file: /cvsroot/src/sys/compat/linux32/common/linux32_mod.c,v
retrieving revision 1.5
diff -u -r1.5 linux32_mod.c
--- compat/linux32/common/linux32_mod.c 7 Mar 2014 01:33:43 - 1.5
+++ compat/linux32/common/linux32_mod.c 11 Jul 2014 12:13:44 -
@@ -67,7 +67,7 @@
.elf_probe_func = linux32_elf32_probe,
},
.es_emul = &emul_linux32,
- .es_prio = EXECSW_PRIO_FIRST,
+ .es_prio = EXECSW_PRIO_ANY,
.es_arglen = LINUX32_ELF_AUX_ARGSIZ,
.es_copyargs = linux32_elf32_copyargs,
.es_setregs = NULL,
Index: kern/exec_elf32.c
===
RCS file: /cvsroot/src/sys/kern/exec_elf32.c,v
retrieving revision 1.140
diff -u -r1.140 exec_elf32.c
--- kern/exec_elf32.c 7 Apr 2014 17:02:15 - 1.140
+++ kern/exec_elf32.c 11 Jul 2014 12:13:44 -
@@ -59,7 +59,7 @@
.elf_probe_func = netbsd_elf32_probe,
},
.es_emul = &emul_netbsd,
- .es_prio = EXECSW_PRIO_ANY,
+ .es_prio = EXECSW_PRIO_FIRST,
.es_arglen = ELF32_AUXSIZE,
.es_copyargs = elf32_copyargs,
.es_setregs = NULL,
@@ -87,7 +87,28 @@
static int
exec_elf32_modcmd(modcmd_t cmd, void *arg)
{
+#if ARCH_ELFSIZE == 64
+ /*
+* If we are on a 64bit system, we don't want the 32bit execsw[] to be
+* added in the global array, because the exec_elf32 module only works
+* on 32bit systems.
+*
+* However, we need the exec_elf32 module, because it will make the
32bit
+* functions available for netbsd32 and linux32.
+*
+* Therefore, allow this module on 64bit systems, but make it dormant.
+*/
+ (void)exec_elf32_execsw; /* unused */
+
+ switch (cmd) {
+ case MODULE_CMD_INIT:
+ case MODULE_CMD_FINI:
+ return 0;
+ default:
+ return ENOTTY;
+ }
+#else
switch (cmd) {
case MODULE_CMD_INIT:
return exec_add(exec_elf32_execsw,
@@ -100,4 +121,5 @@
default:
return ENOTTY;
}
+#endif /* ARCH_ELFSIZE == 64 */
}
Index: kern/exec_elf64.c
===
RCS file: /cvsroot/src/sys/kern/exec_elf64.c,v
retrieving revision 1.5
diff -u -r1.5 exec_elf64.c
--- kern/exec_elf64.c 7 Mar 2014 01:34:29 - 1.5
+++ kern/exec_elf64.c 11 Jul 2014 12:13:44 -
@@ -60,7 +60,7 @@
.elf_probe_func = netbsd_elf64_probe,
},
.es_emul = &emul_netbsd,
- .es_prio = EXECSW_PRIO_ANY,
+ .es_prio = EXECSW_PRIO_FIRST,
.es_arglen = ELF64_AUXSIZE,
.es_copyargs = elf64_copyargs,
.es_setregs = NULL,
Re: Fixing the ELF priorities
Le 03/07/2014 21:40, David Holland a écrit :
>
> On Thu, Jul 03, 2014 at 03:26:09PM +0200, Maxime Villard wrote:
> > > Being able to automatically invoke a suitable emulator for user
> > > binaries of the wrong OS and/or architecture would be very helpful for
> > > crossbuilding difficult packages.
> >
> > But this isn't related to the priority problem, right?
>
> Right, I think Justin mentioned it just because it was worth
> mentioning and tangentially related.
>
> > Perhaps I haven't been clear enough; the question is:
> >
> > - exec_elf32 is a module, and is enabled with EXEC_ELF32 (#define).
> > - exec_elf32 (module) is dormant on 64bit systems.
> > - linux32 and netbsd32 *don't need* exec_elf32 (module), but need
> > EXEC_ELF32 (#define).
> > - my plan is to keep EXEC_ELF32 defined on 64bit systems, but to disable
> > exec_elf32 (module).
> > - Problem: linux32 has exec_elf32 (module) as dependency.
> > - Question: do I remove this dependency, given the fact that netbsd32
> > too does not have it?
>
> Well ok. It looks to me like the problem is that the EXEC_ELF32 and
> EXEC_ELF64 code each provides a "native" struct execsw entry, but
> obviously only one of them can be native at once so the other one
> won't ever work.
>
> It seems like the solution, then, is to take the "native" struct
> execsw entries out of the EXEC_ELF32 and EXEC_ELF64 code, and instead
> always provide a single native struct execsw entry in exec_elf.c based
> on ARCH_ELFSIZE. That will always work for native binaries, and a
> 64-bit machine with compat32 (of whatever flavor) will load one or
> more additional execsw entries via compat32 that use compat32.
>
> And then the compat_linux32, compat_netbsd32, etc. can all depend on
> the EXEC_ELF32 module like they need to.
Yes. Normally there should be one, native exec_elf module, and another
exec_elf32 for linux32 and netbsd32.
But exec_elf{32/64} also compile core_elf{32/64}.c, and they all rely
on ELFSIZE, EXEC_ELF32/EXEC_ELF64, ARCH_ELFSIZE, etc, that are defined
everywhere and nowhere. It is not clear to me how all these #defines are
supposed to work.
>
> AIUI, btw, it is wrong for this dependency to depend on whether
> EXEC_ELF32 is defined during compilation; theoretically if EXEC_ELF32
> is compiled into the kernel the module should still depend on it and
> will still "load" it properly. (However, it's possible that the reason
> such logic is in place is that this doesn't actually work in
> practice.)
>
> The question is whether taking the "native" execsw entries out of the
> EXEC_ELF32 and EXEC_ELF64 code will break the module system. I'm not
> clear on this.
>
I don't think it would break the module system. But this big change will
surely break something else. That's why I wanted to keep the current
architecture and just remove exec_elf32 on 64bit systems, for the moment.
Re: Fixing the ELF priorities
On Thu, Jul 03, 2014 at 03:26:09PM +0200, Maxime Villard wrote: > > Being able to automatically invoke a suitable emulator for user > > binaries of the wrong OS and/or architecture would be very helpful for > > crossbuilding difficult packages. > > But this isn't related to the priority problem, right? Right, I think Justin mentioned it just because it was worth mentioning and tangentially related. > Perhaps I haven't been clear enough; the question is: > > - exec_elf32 is a module, and is enabled with EXEC_ELF32 (#define). > - exec_elf32 (module) is dormant on 64bit systems. > - linux32 and netbsd32 *don't need* exec_elf32 (module), but need > EXEC_ELF32 (#define). > - my plan is to keep EXEC_ELF32 defined on 64bit systems, but to disable > exec_elf32 (module). > - Problem: linux32 has exec_elf32 (module) as dependency. > - Question: do I remove this dependency, given the fact that netbsd32 > too does not have it? Well ok. It looks to me like the problem is that the EXEC_ELF32 and EXEC_ELF64 code each provides a "native" struct execsw entry, but obviously only one of them can be native at once so the other one won't ever work. It seems like the solution, then, is to take the "native" struct execsw entries out of the EXEC_ELF32 and EXEC_ELF64 code, and instead always provide a single native struct execsw entry in exec_elf.c based on ARCH_ELFSIZE. That will always work for native binaries, and a 64-bit machine with compat32 (of whatever flavor) will load one or more additional execsw entries via compat32 that use compat32. And then the compat_linux32, compat_netbsd32, etc. can all depend on the EXEC_ELF32 module like they need to. AIUI, btw, it is wrong for this dependency to depend on whether EXEC_ELF32 is defined during compilation; theoretically if EXEC_ELF32 is compiled into the kernel the module should still depend on it and will still "load" it properly. (However, it's possible that the reason such logic is in place is that this doesn't actually work in practice.) The question is whether taking the "native" execsw entries out of the EXEC_ELF32 and EXEC_ELF64 code will break the module system. I'm not clear on this. -- David A. Holland dholl...@netbsd.org
Re: Fixing the ELF priorities
On Jul 3, 2014, at 6:26 AM, Maxime Villard wrote: > Le 02/07/2014 07:01, David Holland a écrit : >> >> On Tue, Jul 01, 2014 at 07:38:33PM +0100, Justin Cormack wrote: >>> FreeBSD recently ( >>> http://svnweb.freebsd.org/base?view=revision&revision=264269 ) added >>> elf header signature parsing to decide how to execute binaries (based >>> on the Linux binfmt_misc). The main use case is for qemu emulation, >>> but it could also apply to this type of issue. It is obviously a >>> bigger change, but could be worth considering. >> >> Being able to automatically invoke a suitable emulator for user >> binaries of the wrong OS and/or architecture would be very helpful for >> crossbuilding difficult packages. >> > > But this isn't related to the priority problem, right? > > Perhaps I haven't been clear enough; the question is: > > - exec_elf32 is a module, and is enabled with EXEC_ELF32 (#define). > - exec_elf32 (module) is dormant on 64bit systems. > - linux32 and netbsd32 *don't need* exec_elf32 (module), but need >EXEC_ELF32 (#define). > - my plan is to keep EXEC_ELF32 defined on 64bit systems, but to disable >exec_elf32 (module). > - Problem: linux32 has exec_elf32 (module) as dependency. > - Question: do I remove this dependency, given the fact that netbsd32 >too does not have it? Why not just make the exec elf32 be an compiled if _LP64 is defined?
Re: Fixing the ELF priorities
Le 02/07/2014 07:01, David Holland a écrit : > > On Tue, Jul 01, 2014 at 07:38:33PM +0100, Justin Cormack wrote: > > FreeBSD recently ( > > http://svnweb.freebsd.org/base?view=revision&revision=264269 ) added > > elf header signature parsing to decide how to execute binaries (based > > on the Linux binfmt_misc). The main use case is for qemu emulation, > > but it could also apply to this type of issue. It is obviously a > > bigger change, but could be worth considering. > > Being able to automatically invoke a suitable emulator for user > binaries of the wrong OS and/or architecture would be very helpful for > crossbuilding difficult packages. > But this isn't related to the priority problem, right? Perhaps I haven't been clear enough; the question is: - exec_elf32 is a module, and is enabled with EXEC_ELF32 (#define). - exec_elf32 (module) is dormant on 64bit systems. - linux32 and netbsd32 *don't need* exec_elf32 (module), but need EXEC_ELF32 (#define). - my plan is to keep EXEC_ELF32 defined on 64bit systems, but to disable exec_elf32 (module). - Problem: linux32 has exec_elf32 (module) as dependency. - Question: do I remove this dependency, given the fact that netbsd32 too does not have it?
Re: Fixing the ELF priorities
On Tue, Jul 01, 2014 at 07:38:33PM +0100, Justin Cormack wrote: > FreeBSD recently ( > http://svnweb.freebsd.org/base?view=revision&revision=264269 ) added > elf header signature parsing to decide how to execute binaries (based > on the Linux binfmt_misc). The main use case is for qemu emulation, > but it could also apply to this type of issue. It is obviously a > bigger change, but could be worth considering. Being able to automatically invoke a suitable emulator for user binaries of the wrong OS and/or architecture would be very helpful for crossbuilding difficult packages. -- David A. Holland dholl...@netbsd.org
Re: Fixing the ELF priorities
On Tue, Jul 1, 2014 at 9:03 AM, Maxime Villard wrote: > Hi, > I would like to improve the priorities of the binary loader. When the kernel > loads a binary, it basically loops and calls different loaders (for aout, ELF, > ...). There are several ELF loaders, for native and emulated binaries. This > loop has a particular order: the 32bit compat loaders are called *before* > the native ones. Which means that when you launch a simple 64bit binary on a > 64bit system, the kernel first tries to load it through the netbsd32 and > linux32 > compat loaders. The priority is obviously wrong, the native loaders should be > called first; it also has a non-negligible performance impact when executing > many binaries (when compiling something, for example). FreeBSD recently ( http://svnweb.freebsd.org/base?view=revision&revision=264269 ) added elf header signature parsing to decide how to execute binaries (based on the Linux binfmt_misc). The main use case is for qemu emulation, but it could also apply to this type of issue. It is obviously a bigger change, but could be worth considering. Justin
Fixing the ELF priorities
Hi, I would like to improve the priorities of the binary loader. When the kernel loads a binary, it basically loops and calls different loaders (for aout, ELF, ...). There are several ELF loaders, for native and emulated binaries. This loop has a particular order: the 32bit compat loaders are called *before* the native ones. Which means that when you launch a simple 64bit binary on a 64bit system, the kernel first tries to load it through the netbsd32 and linux32 compat loaders. The priority is obviously wrong, the native loaders should be called first; it also has a non-negligible performance impact when executing many binaries (when compiling something, for example). 32bit system: - exec_elf32 --> 32bit code and module, handles native binaries. 64bit system: - exec_elf64 --> 64bit code and module, handles native binaries. - exec_elf32 --> 32bit code and module. This module loads 32bit binaries properly, but they won't work in any case since it will then use the native 64bit environment. - netbsd32 --> this module loads 32bit binaries through the native 32bit code, but then uses its own 32bit syscalls. That's how a 32bit binary can work on a 64bit system. Let's consider this order: 1. exec_elf64 2. exec_elf32 3. netbsd32 That's how a 32bit binary will be processed: exec_elf64 will reject the binary, because it's not a 64bit one (ELF header). exec_elf32 will accept the binary, as it is a 32bit one, and will try to launch it, which of course will fail because it will use the native 64bit environment. So the binary will crash. netbsd32 is never reached. That's why netbsd32 is currently the first: it will use the 32bit code to load the binary (as well as exec_elf32 would), but it will then use its own 32bit environment. Now, a correct fix would be, on 64bit systems, to compile the 32bit code without setting up the exec_elf32 module. That way, on those systems, there's no more exec_elf32 module, which means that there's no module that will crash a binary before it actually reaches the intended compat layer, but there's still the 32bit code usable by netbsd32 and linux32. Then the order does not matter anymore, and we can give the native loader the highest priority. Attached are two patches (the former to disable the 32bit module, the latter to change the priorities). But I wonder if there wouldn't be a cleaner way to fix this. The problem with these patches is that exec_elf32 is a dependency of linux32: - linux32/common/linux32_mod.c - #if defined(EXEC_ELF32) # defineMD1 ",exec_elf32,compat_netbsd32" #else # defineMD1 "" #endif MODULE(MODULE_CLASS_EXEC, compat_linux32, "compat_linux" MD1); I think there's another issue with the dependency here: whether the exec_elf32 module is enabled or not does not matter, what matters is if the kernel is compiled with EXEC_ELF32. But as EXEC_ELF32 on 64bit systems will now disable exec_elf32, we have a problem. Beyond that, there's an inconsistency: if exec_elf32 is a dependency of linux32, then it should also be a dependency of netbsd32 (not the case currently). Do we just delete this dependency? This should really be fixed. I've made a small benchmark: on average, we save 30 seconds when doing "./build.sh kernel=GENERIC" with the attached fixes on amd64. -- 1 --- Index: exec_elf32.c === RCS file: /cvsroot/src/sys/kern/exec_elf32.c,v retrieving revision 1.140 diff -u -r1.140 exec_elf32.c --- exec_elf32.c7 Apr 2014 17:02:15 - 1.140 +++ exec_elf32.c8 Jun 2014 11:54:23 - @@ -34,10 +34,17 @@ #include __KERNEL_RCSID(0, "$NetBSD: exec_elf32.c,v 1.140 2014/04/07 17:02:15 rjs Exp $"); +/* Compile the 32bit code */ #defineELFSIZE 32 - #include "exec_elf.c" +#if ARCH_ELFSIZE != 64 +/* + * If we are on a 64bit system, we don't need the exec_elf32 module. However, + * we need the 32bit code to be compiled, because netbsd32 and linux32 will + * use it. + */ + #include #define ELF32_AUXSIZE (howmany(ELF_AUX_ENTRIES * sizeof(Aux32Info), \ @@ -101,3 +108,5 @@ return ENOTTY; } } + +#endif /* ARCH_ELFSIZE != 64 */ -- 2 --- Index: compat/linux32/common/linux32_mod.c === RCS file: /cvsroot/src/sys/compat/linux32/common/linux32_mod.c,v retrieving revision 1.5 diff -u -r1.5 linux32_mod.c --- compat/linux32/common/linux32_mod.c 7 Mar 2014 01:3
