bFLT binaries are usually created using elf2flt.
The linker script used by elf2flt has defined the .data section like the following for the last 19 years:
.data : { _sdata = . ; __data_start = . ; data_start = . ; *(.got.plt) *(.got) FILL(0) ; . = ALIGN(0x20) ; LONG(-1) . = ALIGN(0x20) ; ... }
It places the .got.plt input section before the .got input section. The same is true for the default linker script (ld --verbose) on most architectures except x86/x86-64.
The binfmt_flat loader should relocate all GOT entries until it encounters a -1 (the LONG(-1) in the linker script).
The problem is that the .got.plt input section starts with a GOTPLT header (which has size 16 bytes on elf64-riscv and 8 bytes on elf32-riscv), where the first word is set to -1. See the binutils implementation for riscv [1].
This causes the binfmt_flat loader to stop relocating GOT entries prematurely and thus causes the application to crash when running.
Fix this by skipping the whole GOTPLT header, since the whole GOTPLT header is reserved for the dynamic linker.
The GOTPLT header will only be skipped for bFLT binaries with flag FLAT_FLAG_GOTPIC set. This flag is unconditionally set by elf2flt if the supplied ELF binary has the symbol _GLOBAL_OFFSET_TABLE_ defined. ELF binaries without a .got input section should thus remain unaffected.
Tested on RISC-V Canaan Kendryte K210 and RISC-V QEMU nommu_virt_defconfig.
[1] https://sourceware.org/git/?p=binutils-gdb.git%3Ba=blob%3Bf=bfd/elfnn-riscv....
Cc: stable@vger.kernel.org Signed-off-by: Niklas Cassel niklas.cassel@wdc.com --- Changes since v1: -Incorporated review comments from Eric Biederman.
RISC-V elf2flt patches are still not merged, they can be found here: https://github.com/floatious/elf2flt/tree/riscv
buildroot branch for k210 nommu (including this patch and elf2flt patches): https://github.com/floatious/buildroot/tree/k210-v14
fs/binfmt_flat.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 626898150011..e5e2a03b39c1 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -440,6 +440,30 @@ static void old_reloc(unsigned long rl)
/****************************************************************************/
+static inline u32 __user *skip_got_header(u32 __user *rp) +{ + if (IS_ENABLED(CONFIG_RISCV)) { + /* + * RISC-V has a 16 byte GOT PLT header for elf64-riscv + * and 8 byte GOT PLT header for elf32-riscv. + * Skip the whole GOT PLT header, since it is reserved + * for the dynamic linker (ld.so). + */ + u32 rp_val0, rp_val1; + + if (get_user(rp_val0, rp)) + return rp; + if (get_user(rp_val1, rp + 1)) + return rp; + + if (rp_val0 == 0xffffffff && rp_val1 == 0xffffffff) + rp += 4; + else if (rp_val0 == 0xffffffff) + rp += 2; + } + return rp; +} + static int load_flat_file(struct linux_binprm *bprm, struct lib_info *libinfo, int id, unsigned long *extra_stack) { @@ -789,7 +813,8 @@ static int load_flat_file(struct linux_binprm *bprm, * image. */ if (flags & FLAT_FLAG_GOTPIC) { - for (rp = (u32 __user *)datapos; ; rp++) { + rp = skip_got_header((u32 * __user) datapos); + for (; ; rp++) { u32 addr, rp_val; if (get_user(rp_val, rp)) return -EFAULT;
On Thu, Apr 14, 2022 at 11:10:18AM +0200, Niklas Cassel wrote:
bFLT binaries are usually created using elf2flt.
The linker script used by elf2flt has defined the .data section like the following for the last 19 years:
.data : { _sdata = . ; __data_start = . ; data_start = . ; *(.got.plt) *(.got) FILL(0) ; . = ALIGN(0x20) ; LONG(-1) . = ALIGN(0x20) ; ... }
It places the .got.plt input section before the .got input section. The same is true for the default linker script (ld --verbose) on most architectures except x86/x86-64.
The binfmt_flat loader should relocate all GOT entries until it encounters a -1 (the LONG(-1) in the linker script).
The problem is that the .got.plt input section starts with a GOTPLT header (which has size 16 bytes on elf64-riscv and 8 bytes on elf32-riscv), where the first word is set to -1. See the binutils implementation for riscv [1].
This causes the binfmt_flat loader to stop relocating GOT entries prematurely and thus causes the application to crash when running.
Fix this by skipping the whole GOTPLT header, since the whole GOTPLT header is reserved for the dynamic linker.
The GOTPLT header will only be skipped for bFLT binaries with flag FLAT_FLAG_GOTPIC set. This flag is unconditionally set by elf2flt if the supplied ELF binary has the symbol _GLOBAL_OFFSET_TABLE_ defined. ELF binaries without a .got input section should thus remain unaffected.
Tested on RISC-V Canaan Kendryte K210 and RISC-V QEMU nommu_virt_defconfig.
[1] https://sourceware.org/git/?p=binutils-gdb.git%3Ba=blob%3Bf=bfd/elfnn-riscv....
Cc: stable@vger.kernel.org Signed-off-by: Niklas Cassel niklas.cassel@wdc.com
Changes since v1: -Incorporated review comments from Eric Biederman.
Thanks! nit: please include a link to the archive so it's easier for people (and b4) to track earlier versions. i.e.: https://lore.kernel.org/linux-mm/20220412100338.437308-1-niklas.cassel@wdc.c...
RISC-V elf2flt patches are still not merged, they can be found here: https://github.com/floatious/elf2flt/tree/riscv
buildroot branch for k210 nommu (including this patch and elf2flt patches): https://github.com/floatious/buildroot/tree/k210-v14
fs/binfmt_flat.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 626898150011..e5e2a03b39c1 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -440,6 +440,30 @@ static void old_reloc(unsigned long rl) /****************************************************************************/ +static inline u32 __user *skip_got_header(u32 __user *rp) +{
- if (IS_ENABLED(CONFIG_RISCV)) {
/** RISC-V has a 16 byte GOT PLT header for elf64-riscv* and 8 byte GOT PLT header for elf32-riscv.* Skip the whole GOT PLT header, since it is reserved* for the dynamic linker (ld.so).*/u32 rp_val0, rp_val1;if (get_user(rp_val0, rp))return rp;if (get_user(rp_val1, rp + 1))return rp;if (rp_val0 == 0xffffffff && rp_val1 == 0xffffffff)rp += 4;else if (rp_val0 == 0xffffffff)rp += 2;
Just so I understand; due to the FILL(0) and the ALIGN, val1 will be 0 (or more specifically, not -1) in all other cases, yes?
I probably would have written this as:
if (rp_val0 == 0xffffffff) rp += 2; if (rp_val1 == 0xffffffff) rp += 2;
But no need to change it. I expect the compiler would optimize it in the same thing anyway. :)
- }
- return rp;
+}
static int load_flat_file(struct linux_binprm *bprm, struct lib_info *libinfo, int id, unsigned long *extra_stack) { @@ -789,7 +813,8 @@ static int load_flat_file(struct linux_binprm *bprm, * image. */ if (flags & FLAT_FLAG_GOTPIC) {
for (rp = (u32 __user *)datapos; ; rp++) {
rp = skip_got_header((u32 * __user) datapos);for (; ; rp++) { u32 addr, rp_val; if (get_user(rp_val, rp)) return -EFAULT;-- 2.35.1
Eric and Damien, does this look good to you? I'll take this into the execve tree unless you'd like to see further changes.
Thanks!
-Kees
On Thu, Apr 14, 2022 at 04:05:54PM -0700, Kees Cook wrote:
On Thu, Apr 14, 2022 at 11:10:18AM +0200, Niklas Cassel wrote:
(snip)
+static inline u32 __user *skip_got_header(u32 __user *rp) +{
- if (IS_ENABLED(CONFIG_RISCV)) {
/** RISC-V has a 16 byte GOT PLT header for elf64-riscv* and 8 byte GOT PLT header for elf32-riscv.* Skip the whole GOT PLT header, since it is reserved* for the dynamic linker (ld.so).*/u32 rp_val0, rp_val1;if (get_user(rp_val0, rp))return rp;if (get_user(rp_val1, rp + 1))return rp;if (rp_val0 == 0xffffffff && rp_val1 == 0xffffffff)rp += 4;else if (rp_val0 == 0xffffffff)rp += 2;Just so I understand; due to the FILL(0) and the ALIGN, val1 will be 0 (or more specifically, not -1) in all other cases, yes?
For elf64-riscv with a .got.plt header: rp+0: -1, rp+1: -1, rp+2: 0, rp+3: 0
For elf32-riscv with a .got.plt header: rp+0: -1, rp+1: 0
At least riscv binutils 2.32, 2.37 and 2.38 all create a .got.plt header even when there are no .got.plt entries following the header.
Even if the .got.plt section was empty, there will still be data in the .got section, so rp+0 will still not be -1.
If there is no data in the .got section, then the _GLOBAL_OFFSET_TABLE_ symbol will not be defined, so elf2flt will not set the FLAT_FLAG_GOTPIC flag. (This code is only executed if that flag is set.)
Kind regards, Niklas
On Thu, Apr 14, 2022 at 11:10:18AM +0200, Niklas Cassel wrote:
bFLT binaries are usually created using elf2flt. [...]
Hm, something in the chain broke DKIM, but I can't see what:
✗ [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv ✗ BADSIG: DKIM/wdc.com
Konstantin, do you have a process for debugging these? I don't see the "normal" stuff that breaks DKIM (i.e. a trailing mailing list footer, etc)
On 4/15/22 08:27, Kees Cook wrote:
On Thu, Apr 14, 2022 at 11:10:18AM +0200, Niklas Cassel wrote:
bFLT binaries are usually created using elf2flt. [...]
Hm, something in the chain broke DKIM, but I can't see what:
✗ [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv ✗ BADSIG: DKIM/wdc.com
Hu... WDC emails are not spams :) No clue what is going on here. I can check with our IT if they changed something though.
Konstantin, do you have a process for debugging these? I don't see the "normal" stuff that breaks DKIM (i.e. a trailing mailing list footer, etc)
On Thu, Apr 14, 2022 at 04:27:38PM -0700, Kees Cook wrote:
On Thu, Apr 14, 2022 at 11:10:18AM +0200, Niklas Cassel wrote:
bFLT binaries are usually created using elf2flt. [...]
Hm, something in the chain broke DKIM, but I can't see what:
✗ [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv ✗ BADSIG: DKIM/wdc.com
Konstantin, do you have a process for debugging these? I don't see the "normal" stuff that breaks DKIM (i.e. a trailing mailing list footer, etc)
It's our usual friend "c=simple/simple" -- vger just doesn't work with that. See here for reasons:
https://lore.kernel.org/lkml/20211214150032.nioelgvmase7yyus@meerkat.local/
You should try to convince wdc.com mail admins to set it to c=relaxed/simple, or you can cc all your patches to patches@lists.linux.dev (which does work with c=simple/simple), and then b4 will give those priority treatment.
-K
On Thu, Apr 14, 2022 at 09:26:10PM -0400, Konstantin Ryabitsev wrote:
On Thu, Apr 14, 2022 at 04:27:38PM -0700, Kees Cook wrote:
On Thu, Apr 14, 2022 at 11:10:18AM +0200, Niklas Cassel wrote:
bFLT binaries are usually created using elf2flt. [...]
Hm, something in the chain broke DKIM, but I can't see what:
✗ [PATCH v2] binfmt_flat: do not stop relocating GOT entries prematurely on riscv ✗ BADSIG: DKIM/wdc.com
Konstantin, do you have a process for debugging these? I don't see the "normal" stuff that breaks DKIM (i.e. a trailing mailing list footer, etc)
It's our usual friend "c=simple/simple" -- vger just doesn't work with that. See here for reasons:
https://lore.kernel.org/lkml/20211214150032.nioelgvmase7yyus@meerkat.local/
You should try to convince wdc.com mail admins to set it to c=relaxed/simple, or you can cc all your patches to patches@lists.linux.dev (which does work with c=simple/simple), and then b4 will give those priority treatment.
Ah-ha! Thank you! :)
On 4/14/22 18:10, Niklas Cassel wrote:
bFLT binaries are usually created using elf2flt.
The linker script used by elf2flt has defined the .data section like the following for the last 19 years:
.data : { _sdata = . ; __data_start = . ; data_start = . ; *(.got.plt) *(.got) FILL(0) ; . = ALIGN(0x20) ; LONG(-1) . = ALIGN(0x20) ; ... }
It places the .got.plt input section before the .got input section. The same is true for the default linker script (ld --verbose) on most architectures except x86/x86-64.
The binfmt_flat loader should relocate all GOT entries until it encounters a -1 (the LONG(-1) in the linker script).
The problem is that the .got.plt input section starts with a GOTPLT header (which has size 16 bytes on elf64-riscv and 8 bytes on elf32-riscv), where the first word is set to -1. See the binutils implementation for riscv [1].
This causes the binfmt_flat loader to stop relocating GOT entries prematurely and thus causes the application to crash when running.
Fix this by skipping the whole GOTPLT header, since the whole GOTPLT header is reserved for the dynamic linker.
The GOTPLT header will only be skipped for bFLT binaries with flag FLAT_FLAG_GOTPIC set. This flag is unconditionally set by elf2flt if the supplied ELF binary has the symbol _GLOBAL_OFFSET_TABLE_ defined. ELF binaries without a .got input section should thus remain unaffected.
Tested on RISC-V Canaan Kendryte K210 and RISC-V QEMU nommu_virt_defconfig.
[1] https://sourceware.org/git/?p=binutils-gdb.git%3Ba=blob%3Bf=bfd/elfnn-riscv....
Cc: stable@vger.kernel.org Signed-off-by: Niklas Cassel niklas.cassel@wdc.com
Changes since v1: -Incorporated review comments from Eric Biederman.
RISC-V elf2flt patches are still not merged, they can be found here: https://github.com/floatious/elf2flt/tree/riscv
buildroot branch for k210 nommu (including this patch and elf2flt patches): https://github.com/floatious/buildroot/tree/k210-v14
fs/binfmt_flat.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 626898150011..e5e2a03b39c1 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -440,6 +440,30 @@ static void old_reloc(unsigned long rl) /****************************************************************************/ +static inline u32 __user *skip_got_header(u32 __user *rp) +{
- if (IS_ENABLED(CONFIG_RISCV)) {
/** RISC-V has a 16 byte GOT PLT header for elf64-riscv* and 8 byte GOT PLT header for elf32-riscv.* Skip the whole GOT PLT header, since it is reserved* for the dynamic linker (ld.so).*/u32 rp_val0, rp_val1;if (get_user(rp_val0, rp))return rp;if (get_user(rp_val1, rp + 1))return rp;if (rp_val0 == 0xffffffff && rp_val1 == 0xffffffff)rp += 4;else if (rp_val0 == 0xffffffff)rp += 2;
This looks good to me. But thinking more about it, do we really need to check what the content of the header is ? Why not simply replace this entire hunk with:
return rp + sizeof(unsigned long) * 2;
to ignore the 16B (or 8B for 32-bits arch) header regardless of what the header word values are ? Are there any case where the header is *not* present ?
- }
- return rp;
+}
static int load_flat_file(struct linux_binprm *bprm, struct lib_info *libinfo, int id, unsigned long *extra_stack) { @@ -789,7 +813,8 @@ static int load_flat_file(struct linux_binprm *bprm, * image. */ if (flags & FLAT_FLAG_GOTPIC) {
for (rp = (u32 __user *)datapos; ; rp++) {
rp = skip_got_header((u32 * __user) datapos);for (; ; rp++) { u32 addr, rp_val; if (get_user(rp_val, rp)) return -EFAULT;
Regardless of the above nit, feel free to add:
Reviewed-by: Damien Le Moal damien.lemoal@opensource.wdc.com
On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote:
On 4/14/22 18:10, Niklas Cassel wrote:
(snip)
This looks good to me. But thinking more about it, do we really need to check what the content of the header is ? Why not simply replace this entire hunk with:
return rp + sizeof(unsigned long) * 2;to ignore the 16B (or 8B for 32-bits arch) header regardless of what the header word values are ? Are there any case where the header is *not* present ?
Considering that I haven't been able to find any real specification that describes the bFLT format. (No, the elf2flt source is no specification.) This whole format seems kind of fragile.
I realize that checking the first one or two entries after data start is not the most robust thing, but I still prefer it over skipping blindly.
Especially considering that only m68k seems to support shared libraries with bFLT. So even while this header is reserved for ld.so, it will most likely only be used on m68k bFLT binaries.. so perhaps elf2flt some day decides to strip away this header on all bFLT binaries except for m68k?
bFLT seems to currently be at version 4, perhaps such a change would require a version bump.. Or not? (Now, if there only was a spec.. :P)
Kind regards, Niklas
- }
- return rp;
+}
static int load_flat_file(struct linux_binprm *bprm, struct lib_info *libinfo, int id, unsigned long *extra_stack) { @@ -789,7 +813,8 @@ static int load_flat_file(struct linux_binprm *bprm, * image. */ if (flags & FLAT_FLAG_GOTPIC) {
for (rp = (u32 __user *)datapos; ; rp++) {
rp = skip_got_header((u32 * __user) datapos);for (; ; rp++) { u32 addr, rp_val; if (get_user(rp_val, rp)) return -EFAULT;Regardless of the above nit, feel free to add:
Reviewed-by: Damien Le Moal damien.lemoal@opensource.wdc.com
-- Damien Le Moal Western Digital Research
On 4/15/22 09:30, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote:
On 4/14/22 18:10, Niklas Cassel wrote:
(snip)
This looks good to me. But thinking more about it, do we really need to check what the content of the header is ? Why not simply replace this entire hunk with:
return rp + sizeof(unsigned long) * 2;to ignore the 16B (or 8B for 32-bits arch) header regardless of what the header word values are ? Are there any case where the header is *not* present ?
Considering that I haven't been able to find any real specification that describes the bFLT format. (No, the elf2flt source is no specification.) This whole format seems kind of fragile.
I realize that checking the first one or two entries after data start is not the most robust thing, but I still prefer it over skipping blindly.
Especially considering that only m68k seems to support shared libraries with bFLT. So even while this header is reserved for ld.so, it will most likely only be used on m68k bFLT binaries.. so perhaps elf2flt some day decides to strip away this header on all bFLT binaries except for m68k?
bFLT seems to currently be at version 4, perhaps such a change would require a version bump.. Or not? (Now, if there only was a spec.. :P)
The header skip is only for riscv since you have that "if (IS_ENABLED(CONFIG_RISCV)) {". So whatever you do under that if will not affect other architectures. The patch will be a nop for them.
So if we are sure that we can just skip the first 16B/8B for riscv, I would not bother checking the header content. But as mentioned, the current code is fine too.
Both approaches are fine with me but I prefer the simpler one :)
Kind regards, Niklas
- }
- return rp;
+}
static int load_flat_file(struct linux_binprm *bprm, struct lib_info *libinfo, int id, unsigned long *extra_stack) { @@ -789,7 +813,8 @@ static int load_flat_file(struct linux_binprm *bprm, * image. */ if (flags & FLAT_FLAG_GOTPIC) {
for (rp = (u32 __user *)datapos; ; rp++) {
rp = skip_got_header((u32 * __user) datapos);for (; ; rp++) { u32 addr, rp_val; if (get_user(rp_val, rp)) return -EFAULT;Regardless of the above nit, feel free to add:
Reviewed-by: Damien Le Moal damien.lemoal@opensource.wdc.com
-- Damien Le Moal Western Digital Research
On Fri, Apr 15, 2022 at 09:56:38AM +0900, Damien Le Moal wrote:
On 4/15/22 09:30, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote:
On 4/14/22 18:10, Niklas Cassel wrote:
(snip)
So if we are sure that we can just skip the first 16B/8B for riscv, I would not bother checking the header content. But as mentioned, the current code is fine too.
That was my point, I'm not sure that we can be sure that we can always skip it in the future. E.g. if the elf2flt linker script decides to swap the order of .got and .got.plt for some random reason in the future, we would skip data that really should have been relocated.
So I think that it is better to keep it, even if it is a bit verbose.
Kind regards, Niklas
On 4/15/22 10:08, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 09:56:38AM +0900, Damien Le Moal wrote:
On 4/15/22 09:30, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote:
On 4/14/22 18:10, Niklas Cassel wrote:
(snip)
So if we are sure that we can just skip the first 16B/8B for riscv, I would not bother checking the header content. But as mentioned, the current code is fine too.
That was my point, I'm not sure that we can be sure that we can always skip it in the future. E.g. if the elf2flt linker script decides to swap the order of .got and .got.plt for some random reason in the future, we would skip data that really should have been relocated.
Good point. Your current patch is indeed better then. BUT that would also mean that the skip header function needs to be called inside the loop then, no ? If the section orders are reversed, we would still need to skip that header in the middle of the relocation loop...
So I think that it is better to keep it, even if it is a bit verbose.
Kind regards, Niklas
On Fri, Apr 15, 2022 at 10:13:31AM +0900, Damien Le Moal wrote:
On 4/15/22 10:08, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 09:56:38AM +0900, Damien Le Moal wrote:
On 4/15/22 09:30, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote:
On 4/14/22 18:10, Niklas Cassel wrote:
(snip)
So if we are sure that we can just skip the first 16B/8B for riscv, I would not bother checking the header content. But as mentioned, the current code is fine too.
That was my point, I'm not sure that we can be sure that we can always skip it in the future. E.g. if the elf2flt linker script decides to swap the order of .got and .got.plt for some random reason in the future, we would skip data that really should have been relocated.
Good point. Your current patch is indeed better then. BUT that would also mean that the skip header function needs to be called inside the loop then, no ? If the section orders are reversed, we would still need to skip that header in the middle of the relocation loop...
So this is theoretical, but if the sections were swapped in the linker script, and we have the patch in $subject applied, we will not skip data that needs to be relocated. But after relocating all the entries in the .got section we will still break too early, if we actually had any .got.plt entries after the .got.plt header. The .got.plt entries would not get relocated.
However, the elf2flt maintainer explicitly asked ut to fix the kernel or binutils, so that they can continue using the exact same linker script that it has been using forever. (And we shouldn't need to change binutils just for the bFLT format.)
So the chance that the linker script changes in practice is really small. (This .got.plt vs .got hasn't changed in 19 years.)
But if it does, we will just have one problem instead of two :) However, I think that applying this patch is sufficient for now, since it makes the code work with the existing elf2flt linker script.
Adapting the code to also handle this theoretical layout of the linker script would just complicate things even more. I'm not even sure if we would be able to handle this case, since the information about the .got and .got.plt section sizes is lost once the ELF has been converted to bFLT.
Kind regards, Niklas
On 4/15/22 11:11, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 10:13:31AM +0900, Damien Le Moal wrote:
On 4/15/22 10:08, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 09:56:38AM +0900, Damien Le Moal wrote:
On 4/15/22 09:30, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote:
On 4/14/22 18:10, Niklas Cassel wrote:
(snip)
So if we are sure that we can just skip the first 16B/8B for riscv, I would not bother checking the header content. But as mentioned, the current code is fine too.
That was my point, I'm not sure that we can be sure that we can always skip it in the future. E.g. if the elf2flt linker script decides to swap the order of .got and .got.plt for some random reason in the future, we would skip data that really should have been relocated.
Good point. Your current patch is indeed better then. BUT that would also mean that the skip header function needs to be called inside the loop then, no ? If the section orders are reversed, we would still need to skip that header in the middle of the relocation loop...
So this is theoretical, but if the sections were swapped in the linker script, and we have the patch in $subject applied, we will not skip data that needs to be relocated. But after relocating all the entries in the .got section we will still break too early, if we actually had any .got.plt entries after the .got.plt header. The .got.plt entries would not get relocated.
However, the elf2flt maintainer explicitly asked ut to fix the kernel or binutils, so that they can continue using the exact same linker script that it has been using forever. (And we shouldn't need to change binutils just for the bFLT format.)
So the chance that the linker script changes in practice is really small. (This .got.plt vs .got hasn't changed in 19 years.)
But if it does, we will just have one problem instead of two :) However, I think that applying this patch is sufficient for now, since it makes the code work with the existing elf2flt linker script.
Adapting the code to also handle this theoretical layout of the linker script would just complicate things even more. I'm not even sure if we would be able to handle this case, since the information about the .got and .got.plt section sizes is lost once the ELF has been converted to bFLT.
OK. All good then. I maintain my reviewed-by tag :)
On 15/4/22 10:30, Niklas Cassel wrote:
On Fri, Apr 15, 2022 at 08:51:27AM +0900, Damien Le Moal wrote:
On 4/14/22 18:10, Niklas Cassel wrote:
(snip)
This looks good to me. But thinking more about it, do we really need to check what the content of the header is ? Why not simply replace this entire hunk with:
return rp + sizeof(unsigned long) * 2;to ignore the 16B (or 8B for 32-bits arch) header regardless of what the header word values are ? Are there any case where the header is *not* present ?
Considering that I haven't been able to find any real specification that describes the bFLT format. (No, the elf2flt source is no specification.) This whole format seems kind of fragile.
I realize that checking the first one or two entries after data start is not the most robust thing, but I still prefer it over skipping blindly.
Especially considering that only m68k seems to support shared libraries with bFLT. So even while this header is reserved for ld.so, it will most likely only be used on m68k bFLT binaries.. so perhaps elf2flt some day decides to strip away this header on all bFLT binaries except for m68k?
FWIW there has been talk for a couple of years now to remove the shared library support for m68k. It doesn't get used - probably not for a very long time now. And most likely doesn't even work anymore.
Regards Greg
bFLT seems to currently be at version 4, perhaps such a change would require a version bump.. Or not? (Now, if there only was a spec.. :P)
Kind regards, Niklas
- }
- return rp;
+}
- static int load_flat_file(struct linux_binprm *bprm, struct lib_info *libinfo, int id, unsigned long *extra_stack) {
@@ -789,7 +813,8 @@ static int load_flat_file(struct linux_binprm *bprm, * image. */ if (flags & FLAT_FLAG_GOTPIC) {
for (rp = (u32 __user *)datapos; ; rp++) {
rp = skip_got_header((u32 * __user) datapos);for (; ; rp++) { u32 addr, rp_val; if (get_user(rp_val, rp)) return -EFAULT;Regardless of the above nit, feel free to add:
Reviewed-by: Damien Le Moal damien.lemoal@opensource.wdc.com
-- Damien Le Moal Western Digital Research
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com ---
Can the binfmt_flat folks please verify that the shared library support really isn't used?
Was binfmt_flat being enabled on arm and sh the mistake it looks like?
arch/arm/configs/lpc18xx_defconfig | 1 - arch/arm/configs/mps2_defconfig | 1 - arch/arm/configs/stm32_defconfig | 1 - arch/arm/configs/vf610m4_defconfig | 1 - arch/sh/configs/rsk7201_defconfig | 1 - arch/sh/configs/rsk7203_defconfig | 1 - arch/sh/configs/se7206_defconfig | 1 - fs/Kconfig.binfmt | 6 - fs/binfmt_flat.c | 190 ++++++----------------------- 9 files changed, 40 insertions(+), 163 deletions(-)
diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig index be882ea0eee4..688c9849eec8 100644 --- a/arch/arm/configs/lpc18xx_defconfig +++ b/arch/arm/configs/lpc18xx_defconfig @@ -30,7 +30,6 @@ CONFIG_ARM_APPENDED_DTB=y # CONFIG_BLK_DEV_BSG is not set CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_NET=y CONFIG_PACKET=y diff --git a/arch/arm/configs/mps2_defconfig b/arch/arm/configs/mps2_defconfig index 89f4a6ff30bd..c1e98e33a348 100644 --- a/arch/arm/configs/mps2_defconfig +++ b/arch/arm/configs/mps2_defconfig @@ -23,7 +23,6 @@ CONFIG_PREEMPT_VOLUNTARY=y CONFIG_ZBOOT_ROM_TEXT=0x0 CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set # CONFIG_SUSPEND is not set CONFIG_NET=y diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig index 551db328009d..71d6bfcf4551 100644 --- a/arch/arm/configs/stm32_defconfig +++ b/arch/arm/configs/stm32_defconfig @@ -28,7 +28,6 @@ CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x08008000 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_DEVTMPFS=y CONFIG_DEVTMPFS_MOUNT=y diff --git a/arch/arm/configs/vf610m4_defconfig b/arch/arm/configs/vf610m4_defconfig index a89f035c3b01..70fdbfd83484 100644 --- a/arch/arm/configs/vf610m4_defconfig +++ b/arch/arm/configs/vf610m4_defconfig @@ -18,7 +18,6 @@ CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x0f000080 CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_SUSPEND is not set # CONFIG_UEVENT_HELPER is not set # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7201_defconfig b/arch/sh/configs/rsk7201_defconfig index e41526120be1..619c18699459 100644 --- a/arch/sh/configs/rsk7201_defconfig +++ b/arch/sh/configs/rsk7201_defconfig @@ -25,7 +25,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7203_defconfig b/arch/sh/configs/rsk7203_defconfig index 6af08fa1ddf8..5a54e2b883f0 100644 --- a/arch/sh/configs/rsk7203_defconfig +++ b/arch/sh/configs/rsk7203_defconfig @@ -30,7 +30,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y CONFIG_NET=y diff --git a/arch/sh/configs/se7206_defconfig b/arch/sh/configs/se7206_defconfig index 601d062250d1..122216123e63 100644 --- a/arch/sh/configs/se7206_defconfig +++ b/arch/sh/configs/se7206_defconfig @@ -40,7 +40,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC3,115200 ignore_loglevel earlyprintk=serial" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_BINFMT_MISC=y CONFIG_NET=y CONFIG_PACKET=y diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt index 21c6332fa785..32dff7ba3dda 100644 --- a/fs/Kconfig.binfmt +++ b/fs/Kconfig.binfmt @@ -142,12 +142,6 @@ config BINFMT_ZFLAT help Support FLAT format compressed binaries
-config BINFMT_SHARED_FLAT - bool "Enable shared FLAT support" - depends on BINFMT_FLAT - help - Support FLAT shared libraries - config HAVE_AOUT def_bool n
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 0ad2c7bbaddd..82e4412a9665 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -68,11 +68,7 @@ #define RELOC_FAILED 0xff00ff01 /* Relocation incorrect somewhere */ #define UNLOADED_LIB 0x7ff000ff /* Placeholder for unused library */
-#ifdef CONFIG_BINFMT_SHARED_FLAT -#define MAX_SHARED_LIBS (4) -#else -#define MAX_SHARED_LIBS (1) -#endif +#define MAX_SHARED_LIBS (1)
#ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET #define DATA_START_OFFSET_WORDS (0) @@ -92,10 +88,6 @@ struct lib_info { } lib_list[MAX_SHARED_LIBS]; };
-#ifdef CONFIG_BINFMT_SHARED_FLAT -static int load_flat_shared_library(int id, struct lib_info *p); -#endif - static int load_flat_binary(struct linux_binprm *);
static struct linux_binfmt flat_format = { @@ -307,51 +299,18 @@ static int decompress_exec(struct linux_binprm *bprm, loff_t fpos, char *dst, /****************************************************************************/
static unsigned long -calc_reloc(unsigned long r, struct lib_info *p, int curid, int internalp) +calc_reloc(unsigned long r, struct lib_info *p) { unsigned long addr; - int id; unsigned long start_brk; unsigned long start_data; unsigned long text_len; unsigned long start_code;
-#ifdef CONFIG_BINFMT_SHARED_FLAT - if (r == 0) - id = curid; /* Relocs of 0 are always self referring */ - else { - id = (r >> 24) & 0xff; /* Find ID for this reloc */ - r &= 0x00ffffff; /* Trim ID off here */ - } - if (id >= MAX_SHARED_LIBS) { - pr_err("reference 0x%lx to shared library %d", r, id); - goto failed; - } - if (curid != id) { - if (internalp) { - pr_err("reloc address 0x%lx not in same module " - "(%d != %d)", r, curid, id); - goto failed; - } else if (!p->lib_list[id].loaded && - load_flat_shared_library(id, p) < 0) { - pr_err("failed to load library %d", id); - goto failed; - } - /* Check versioning information (i.e. time stamps) */ - if (p->lib_list[id].build_date && p->lib_list[curid].build_date && - p->lib_list[curid].build_date < p->lib_list[id].build_date) { - pr_err("library %d is younger than %d", id, curid); - goto failed; - } - } -#else - id = 0; -#endif - - start_brk = p->lib_list[id].start_brk; - start_data = p->lib_list[id].start_data; - start_code = p->lib_list[id].start_code; - text_len = p->lib_list[id].text_len; + start_brk = p->lib_list[0].start_brk; + start_data = p->lib_list[0].start_data; + start_code = p->lib_list[0].start_code; + text_len = p->lib_list[0].text_len;
if (r > start_brk - start_data + text_len) { pr_err("reloc outside program 0x%lx (0 - 0x%lx/0x%lx)", @@ -419,7 +378,7 @@ static void old_reloc(unsigned long rl) /****************************************************************************/
static int load_flat_file(struct linux_binprm *bprm, - struct lib_info *libinfo, int id, unsigned long *extra_stack) + struct lib_info *libinfo, unsigned long *extra_stack) { struct flat_hdr *hdr; unsigned long textpos, datapos, realdatastart; @@ -471,14 +430,6 @@ static int load_flat_file(struct linux_binprm *bprm, goto err; }
- /* Don't allow old format executables to use shared libraries */ - if (rev == OLD_FLAT_VERSION && id != 0) { - pr_err("shared libraries are not available before rev 0x%lx\n", - FLAT_VERSION); - ret = -ENOEXEC; - goto err; - } - /* * fix up the flags for the older format, there were all kinds * of endian hacks, this only works for the simple cases @@ -529,15 +480,13 @@ static int load_flat_file(struct linux_binprm *bprm, }
/* Flush all traces of the currently running executable */ - if (id == 0) { - ret = begin_new_exec(bprm); - if (ret) - goto err; + ret = begin_new_exec(bprm); + if (ret) + goto err;
- /* OK, This is the point of no return */ - set_personality(PER_LINUX_32BIT); - setup_new_exec(bprm); - } + /* OK, This is the point of no return */ + set_personality(PER_LINUX_32BIT); + setup_new_exec(bprm);
/* * calculate the extra space we need to map in @@ -717,42 +666,40 @@ static int load_flat_file(struct linux_binprm *bprm, text_len -= sizeof(struct flat_hdr); /* the real code len */
/* The main program needs a little extra setup in the task structure */ - if (id == 0) { - current->mm->start_code = start_code; - current->mm->end_code = end_code; - current->mm->start_data = datapos; - current->mm->end_data = datapos + data_len; - /* - * set up the brk stuff, uses any slack left in data/bss/stack - * allocation. We put the brk after the bss (between the bss - * and stack) like other platforms. - * Userspace code relies on the stack pointer starting out at - * an address right at the end of a page. - */ - current->mm->start_brk = datapos + data_len + bss_len; - current->mm->brk = (current->mm->start_brk + 3) & ~3; + current->mm->start_code = start_code; + current->mm->end_code = end_code; + current->mm->start_data = datapos; + current->mm->end_data = datapos + data_len; + /* + * set up the brk stuff, uses any slack left in data/bss/stack + * allocation. We put the brk after the bss (between the bss + * and stack) like other platforms. + * Userspace code relies on the stack pointer starting out at + * an address right at the end of a page. + */ + current->mm->start_brk = datapos + data_len + bss_len; + current->mm->brk = (current->mm->start_brk + 3) & ~3; #ifndef CONFIG_MMU - current->mm->context.end_brk = memp + memp_size - stack_len; + current->mm->context.end_brk = memp + memp_size - stack_len; #endif - }
if (flags & FLAT_FLAG_KTRACE) { pr_info("Mapping is %lx, Entry point is %x, data_start is %x\n", textpos, 0x00ffffff&ntohl(hdr->entry), ntohl(hdr->data_start)); pr_info("%s %s: TEXT=%lx-%lx DATA=%lx-%lx BSS=%lx-%lx\n", - id ? "Lib" : "Load", bprm->filename, + "Load", bprm->filename, start_code, end_code, datapos, datapos + data_len, datapos + data_len, (datapos + data_len + bss_len + 3) & ~3); }
/* Store the current module values into the global library structure */ - libinfo->lib_list[id].start_code = start_code; - libinfo->lib_list[id].start_data = datapos; - libinfo->lib_list[id].start_brk = datapos + data_len + bss_len; - libinfo->lib_list[id].text_len = text_len; - libinfo->lib_list[id].loaded = 1; - libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; - libinfo->lib_list[id].build_date = ntohl(hdr->build_date); + libinfo->lib_list[0].start_code = start_code; + libinfo->lib_list[0].start_data = datapos; + libinfo->lib_list[0].start_brk = datapos + data_len + bss_len; + libinfo->lib_list[0].text_len = text_len; + libinfo->lib_list[0].loaded = 1; + libinfo->lib_list[0].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; + libinfo->lib_list[0].build_date = ntohl(hdr->build_date);
/* * We just load the allocations into some temporary memory to @@ -774,7 +721,7 @@ static int load_flat_file(struct linux_binprm *bprm, if (rp_val == 0xffffffff) break; if (rp_val) { - addr = calc_reloc(rp_val, libinfo, id, 0); + addr = calc_reloc(rp_val, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err; @@ -810,7 +757,7 @@ static int load_flat_file(struct linux_binprm *bprm, return -EFAULT; relval = ntohl(tmp); addr = flat_get_relocate_addr(relval); - rp = (u32 __user *)calc_reloc(addr, libinfo, id, 1); + rp = (u32 __user *)calc_reloc(addr, libinfo); if (rp == (u32 __user *)RELOC_FAILED) { ret = -ENOEXEC; goto err; @@ -833,7 +780,7 @@ static int load_flat_file(struct linux_binprm *bprm, */ addr = ntohl((__force __be32)addr); } - addr = calc_reloc(addr, libinfo, id, 0); + addr = calc_reloc(addr, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err; @@ -861,7 +808,7 @@ static int load_flat_file(struct linux_binprm *bprm, /* zero the BSS, BRK and stack areas */ if (clear_user((void __user *)(datapos + data_len), bss_len + (memp + memp_size - stack_len - /* end brk */ - libinfo->lib_list[id].start_brk) + /* start brk */ + libinfo->lib_list[0].start_brk) + /* start brk */ stack_len)) return -EFAULT;
@@ -871,49 +818,6 @@ static int load_flat_file(struct linux_binprm *bprm, }
-/****************************************************************************/ -#ifdef CONFIG_BINFMT_SHARED_FLAT - -/* - * Load a shared library into memory. The library gets its own data - * segment (including bss) but not argv/argc/environ. - */ - -static int load_flat_shared_library(int id, struct lib_info *libs) -{ - /* - * This is a fake bprm struct; only the members "buf", "file" and - * "filename" are actually used. - */ - struct linux_binprm bprm; - int res; - char buf[16]; - loff_t pos = 0; - - memset(&bprm, 0, sizeof(bprm)); - - /* Create the file name */ - sprintf(buf, "/lib/lib%d.so", id); - - /* Open the file up */ - bprm.filename = buf; - bprm.file = open_exec(bprm.filename); - res = PTR_ERR(bprm.file); - if (IS_ERR(bprm.file)) - return res; - - res = kernel_read(bprm.file, bprm.buf, BINPRM_BUF_SIZE, &pos); - - if (res >= 0) - res = load_flat_file(&bprm, libs, id, NULL); - - allow_write_access(bprm.file); - fput(bprm.file); - - return res; -} - -#endif /* CONFIG_BINFMT_SHARED_FLAT */ /****************************************************************************/
/* @@ -946,7 +850,7 @@ static int load_flat_binary(struct linux_binprm *bprm) stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ stack_len = ALIGN(stack_len, FLAT_STACK_ALIGN);
- res = load_flat_file(bprm, &libinfo, 0, &stack_len); + res = load_flat_file(bprm, &libinfo, &stack_len); if (res < 0) return res;
@@ -991,20 +895,6 @@ static int load_flat_binary(struct linux_binprm *bprm) */ start_addr = libinfo.lib_list[0].entry;
-#ifdef CONFIG_BINFMT_SHARED_FLAT - for (i = MAX_SHARED_LIBS-1; i > 0; i--) { - if (libinfo.lib_list[i].loaded) { - /* Push previos first to call address */ - unsigned long __user *sp; - current->mm->start_stack -= sizeof(unsigned long); - sp = (unsigned long __user *)current->mm->start_stack; - if (put_user(start_addr, sp)) - return -EFAULT; - start_addr = libinfo.lib_list[i].entry; - } - } -#endif - #ifdef FLAT_PLAT_INIT FLAT_PLAT_INIT(regs); #endif
On Wed, 20 Apr 2022 07:58:03 PDT (-0700), ebiederm@xmission.com wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
I don't actually know follow the RISC-V flat support, last I heard it was still sort of just in limbo (some toolchain/userspace bugs th at needed to be sorted out). Damien would know better, though, he's already on the thread. I'll leave it up to him to ack this one, if you were even looking for anything from the RISC-V folks at all (we don't have this in any defconfigs).
Was binfmt_flat being enabled on arm and sh the mistake it looks like?
arch/arm/configs/lpc18xx_defconfig | 1 - arch/arm/configs/mps2_defconfig | 1 - arch/arm/configs/stm32_defconfig | 1 - arch/arm/configs/vf610m4_defconfig | 1 - arch/sh/configs/rsk7201_defconfig | 1 - arch/sh/configs/rsk7203_defconfig | 1 - arch/sh/configs/se7206_defconfig | 1 - fs/Kconfig.binfmt | 6 - fs/binfmt_flat.c | 190 ++++++----------------------- 9 files changed, 40 insertions(+), 163 deletions(-)
diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig index be882ea0eee4..688c9849eec8 100644 --- a/arch/arm/configs/lpc18xx_defconfig +++ b/arch/arm/configs/lpc18xx_defconfig @@ -30,7 +30,6 @@ CONFIG_ARM_APPENDED_DTB=y # CONFIG_BLK_DEV_BSG is not set CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_NET=y CONFIG_PACKET=y diff --git a/arch/arm/configs/mps2_defconfig b/arch/arm/configs/mps2_defconfig index 89f4a6ff30bd..c1e98e33a348 100644 --- a/arch/arm/configs/mps2_defconfig +++ b/arch/arm/configs/mps2_defconfig @@ -23,7 +23,6 @@ CONFIG_PREEMPT_VOLUNTARY=y CONFIG_ZBOOT_ROM_TEXT=0x0 CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set # CONFIG_SUSPEND is not set CONFIG_NET=y diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig index 551db328009d..71d6bfcf4551 100644 --- a/arch/arm/configs/stm32_defconfig +++ b/arch/arm/configs/stm32_defconfig @@ -28,7 +28,6 @@ CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x08008000 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_DEVTMPFS=y CONFIG_DEVTMPFS_MOUNT=y diff --git a/arch/arm/configs/vf610m4_defconfig b/arch/arm/configs/vf610m4_defconfig index a89f035c3b01..70fdbfd83484 100644 --- a/arch/arm/configs/vf610m4_defconfig +++ b/arch/arm/configs/vf610m4_defconfig @@ -18,7 +18,6 @@ CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x0f000080 CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_SUSPEND is not set # CONFIG_UEVENT_HELPER is not set # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7201_defconfig b/arch/sh/configs/rsk7201_defconfig index e41526120be1..619c18699459 100644 --- a/arch/sh/configs/rsk7201_defconfig +++ b/arch/sh/configs/rsk7201_defconfig @@ -25,7 +25,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7203_defconfig b/arch/sh/configs/rsk7203_defconfig index 6af08fa1ddf8..5a54e2b883f0 100644 --- a/arch/sh/configs/rsk7203_defconfig +++ b/arch/sh/configs/rsk7203_defconfig @@ -30,7 +30,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y CONFIG_NET=y diff --git a/arch/sh/configs/se7206_defconfig b/arch/sh/configs/se7206_defconfig index 601d062250d1..122216123e63 100644 --- a/arch/sh/configs/se7206_defconfig +++ b/arch/sh/configs/se7206_defconfig @@ -40,7 +40,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC3,115200 ignore_loglevel earlyprintk=serial" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_BINFMT_MISC=y CONFIG_NET=y CONFIG_PACKET=y diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt index 21c6332fa785..32dff7ba3dda 100644 --- a/fs/Kconfig.binfmt +++ b/fs/Kconfig.binfmt @@ -142,12 +142,6 @@ config BINFMT_ZFLAT help Support FLAT format compressed binaries
-config BINFMT_SHARED_FLAT
- bool "Enable shared FLAT support"
- depends on BINFMT_FLAT
- help
Support FLAT shared librariesconfig HAVE_AOUT def_bool n
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 0ad2c7bbaddd..82e4412a9665 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -68,11 +68,7 @@ #define RELOC_FAILED 0xff00ff01 /* Relocation incorrect somewhere */ #define UNLOADED_LIB 0x7ff000ff /* Placeholder for unused library */
-#ifdef CONFIG_BINFMT_SHARED_FLAT -#define MAX_SHARED_LIBS (4) -#else -#define MAX_SHARED_LIBS (1) -#endif +#define MAX_SHARED_LIBS (1)
#ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET #define DATA_START_OFFSET_WORDS (0) @@ -92,10 +88,6 @@ struct lib_info { } lib_list[MAX_SHARED_LIBS]; };
-#ifdef CONFIG_BINFMT_SHARED_FLAT -static int load_flat_shared_library(int id, struct lib_info *p); -#endif
static int load_flat_binary(struct linux_binprm *);
static struct linux_binfmt flat_format = { @@ -307,51 +299,18 @@ static int decompress_exec(struct linux_binprm *bprm, loff_t fpos, char *dst, /****************************************************************************/
static unsigned long -calc_reloc(unsigned long r, struct lib_info *p, int curid, int internalp) +calc_reloc(unsigned long r, struct lib_info *p) { unsigned long addr;
- int id; unsigned long start_brk; unsigned long start_data; unsigned long text_len; unsigned long start_code;
-#ifdef CONFIG_BINFMT_SHARED_FLAT
- if (r == 0)
id = curid; /* Relocs of 0 are always self referring */- else {
id = (r >> 24) & 0xff; /* Find ID for this reloc */r &= 0x00ffffff; /* Trim ID off here */- }
- if (id >= MAX_SHARED_LIBS) {
pr_err("reference 0x%lx to shared library %d", r, id);goto failed;- }
- if (curid != id) {
if (internalp) {pr_err("reloc address 0x%lx not in same module ""(%d != %d)", r, curid, id);goto failed;} else if (!p->lib_list[id].loaded &&load_flat_shared_library(id, p) < 0) {pr_err("failed to load library %d", id);goto failed;}/* Check versioning information (i.e. time stamps) */if (p->lib_list[id].build_date && p->lib_list[curid].build_date &&p->lib_list[curid].build_date < p->lib_list[id].build_date) {pr_err("library %d is younger than %d", id, curid);goto failed;}- }
-#else
- id = 0;
-#endif
- start_brk = p->lib_list[id].start_brk;
- start_data = p->lib_list[id].start_data;
- start_code = p->lib_list[id].start_code;
- text_len = p->lib_list[id].text_len;
start_brk = p->lib_list[0].start_brk;
start_data = p->lib_list[0].start_data;
start_code = p->lib_list[0].start_code;
text_len = p->lib_list[0].text_len;
if (r > start_brk - start_data + text_len) { pr_err("reloc outside program 0x%lx (0 - 0x%lx/0x%lx)",
@@ -419,7 +378,7 @@ static void old_reloc(unsigned long rl) /****************************************************************************/
static int load_flat_file(struct linux_binprm *bprm,
struct lib_info *libinfo, int id, unsigned long *extra_stack)
struct lib_info *libinfo, unsigned long *extra_stack){ struct flat_hdr *hdr; unsigned long textpos, datapos, realdatastart; @@ -471,14 +430,6 @@ static int load_flat_file(struct linux_binprm *bprm, goto err; }
- /* Don't allow old format executables to use shared libraries */
- if (rev == OLD_FLAT_VERSION && id != 0) {
pr_err("shared libraries are not available before rev 0x%lx\n",FLAT_VERSION);ret = -ENOEXEC;goto err;- }
- /*
- fix up the flags for the older format, there were all kinds
- of endian hacks, this only works for the simple cases
@@ -529,15 +480,13 @@ static int load_flat_file(struct linux_binprm *bprm, }
/* Flush all traces of the currently running executable */
- if (id == 0) {
ret = begin_new_exec(bprm);if (ret)goto err;
- ret = begin_new_exec(bprm);
- if (ret)
goto err;
/* OK, This is the point of no return */set_personality(PER_LINUX_32BIT);setup_new_exec(bprm);- }
/* OK, This is the point of no return */
set_personality(PER_LINUX_32BIT);
setup_new_exec(bprm);
/*
- calculate the extra space we need to map in
@@ -717,42 +666,40 @@ static int load_flat_file(struct linux_binprm *bprm, text_len -= sizeof(struct flat_hdr); /* the real code len */
/* The main program needs a little extra setup in the task structure */
- if (id == 0) {
current->mm->start_code = start_code;current->mm->end_code = end_code;current->mm->start_data = datapos;current->mm->end_data = datapos + data_len;/** set up the brk stuff, uses any slack left in data/bss/stack* allocation. We put the brk after the bss (between the bss* and stack) like other platforms.* Userspace code relies on the stack pointer starting out at* an address right at the end of a page.*/current->mm->start_brk = datapos + data_len + bss_len;current->mm->brk = (current->mm->start_brk + 3) & ~3;
- current->mm->start_code = start_code;
- current->mm->end_code = end_code;
- current->mm->start_data = datapos;
- current->mm->end_data = datapos + data_len;
- /*
* set up the brk stuff, uses any slack left in data/bss/stack* allocation. We put the brk after the bss (between the bss* and stack) like other platforms.* Userspace code relies on the stack pointer starting out at* an address right at the end of a page.*/- current->mm->start_brk = datapos + data_len + bss_len;
- current->mm->brk = (current->mm->start_brk + 3) & ~3;
#ifndef CONFIG_MMU
current->mm->context.end_brk = memp + memp_size - stack_len;
- current->mm->context.end_brk = memp + memp_size - stack_len;
#endif
}
if (flags & FLAT_FLAG_KTRACE) { pr_info("Mapping is %lx, Entry point is %x, data_start is %x\n", textpos, 0x00ffffff&ntohl(hdr->entry), ntohl(hdr->data_start)); pr_info("%s %s: TEXT=%lx-%lx DATA=%lx-%lx BSS=%lx-%lx\n",
id ? "Lib" : "Load", bprm->filename,
"Load", bprm->filename, start_code, end_code, datapos, datapos + data_len, datapos + data_len, (datapos + data_len + bss_len + 3) & ~3);}
/* Store the current module values into the global library structure */
- libinfo->lib_list[id].start_code = start_code;
- libinfo->lib_list[id].start_data = datapos;
- libinfo->lib_list[id].start_brk = datapos + data_len + bss_len;
- libinfo->lib_list[id].text_len = text_len;
- libinfo->lib_list[id].loaded = 1;
- libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
- libinfo->lib_list[id].build_date = ntohl(hdr->build_date);
libinfo->lib_list[0].start_code = start_code;
libinfo->lib_list[0].start_data = datapos;
libinfo->lib_list[0].start_brk = datapos + data_len + bss_len;
libinfo->lib_list[0].text_len = text_len;
libinfo->lib_list[0].loaded = 1;
libinfo->lib_list[0].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
libinfo->lib_list[0].build_date = ntohl(hdr->build_date);
/*
- We just load the allocations into some temporary memory to
@@ -774,7 +721,7 @@ static int load_flat_file(struct linux_binprm *bprm, if (rp_val == 0xffffffff) break; if (rp_val) {
addr = calc_reloc(rp_val, libinfo, id, 0);
addr = calc_reloc(rp_val, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err;@@ -810,7 +757,7 @@ static int load_flat_file(struct linux_binprm *bprm, return -EFAULT; relval = ntohl(tmp); addr = flat_get_relocate_addr(relval);
rp = (u32 __user *)calc_reloc(addr, libinfo, id, 1);
rp = (u32 __user *)calc_reloc(addr, libinfo); if (rp == (u32 __user *)RELOC_FAILED) { ret = -ENOEXEC; goto err;@@ -833,7 +780,7 @@ static int load_flat_file(struct linux_binprm *bprm, */ addr = ntohl((__force __be32)addr); }
addr = calc_reloc(addr, libinfo, id, 0);
addr = calc_reloc(addr, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err;@@ -861,7 +808,7 @@ static int load_flat_file(struct linux_binprm *bprm, /* zero the BSS, BRK and stack areas */ if (clear_user((void __user *)(datapos + data_len), bss_len + (memp + memp_size - stack_len - /* end brk */
libinfo->lib_list[id].start_brk) + /* start brk */
libinfo->lib_list[0].start_brk) + /* start brk */ stack_len))@@ -871,49 +818,6 @@ static int load_flat_file(struct linux_binprm *bprm, }
-/****************************************************************************/ -#ifdef CONFIG_BINFMT_SHARED_FLAT
-/*
- Load a shared library into memory. The library gets its own data
- segment (including bss) but not argv/argc/environ.
- */
-static int load_flat_shared_library(int id, struct lib_info *libs) -{
- /*
* This is a fake bprm struct; only the members "buf", "file" and* "filename" are actually used.*/- struct linux_binprm bprm;
- int res;
- char buf[16];
- loff_t pos = 0;
- memset(&bprm, 0, sizeof(bprm));
- /* Create the file name */
- sprintf(buf, "/lib/lib%d.so", id);
- /* Open the file up */
- bprm.filename = buf;
- bprm.file = open_exec(bprm.filename);
- res = PTR_ERR(bprm.file);
- if (IS_ERR(bprm.file))
return res;- res = kernel_read(bprm.file, bprm.buf, BINPRM_BUF_SIZE, &pos);
- if (res >= 0)
res = load_flat_file(&bprm, libs, id, NULL);- allow_write_access(bprm.file);
- fput(bprm.file);
- return res;
-}
-#endif /* CONFIG_BINFMT_SHARED_FLAT */ /****************************************************************************/
/* @@ -946,7 +850,7 @@ static int load_flat_binary(struct linux_binprm *bprm) stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ stack_len = ALIGN(stack_len, FLAT_STACK_ALIGN);
- res = load_flat_file(bprm, &libinfo, 0, &stack_len);
- res = load_flat_file(bprm, &libinfo, &stack_len); if (res < 0) return res;
@@ -991,20 +895,6 @@ static int load_flat_binary(struct linux_binprm *bprm) */ start_addr = libinfo.lib_list[0].entry;
-#ifdef CONFIG_BINFMT_SHARED_FLAT
- for (i = MAX_SHARED_LIBS-1; i > 0; i--) {
if (libinfo.lib_list[i].loaded) {/* Push previos first to call address */unsigned long __user *sp;current->mm->start_stack -= sizeof(unsigned long);sp = (unsigned long __user *)current->mm->start_stack;if (put_user(start_addr, sp))return -EFAULT;start_addr = libinfo.lib_list[i].entry;}- }
-#endif
#ifdef FLAT_PLAT_INIT FLAT_PLAT_INIT(regs); #endif
On Wed, Apr 20, 2022 at 09:17:22AM -0700, Palmer Dabbelt wrote:
On Wed, 20 Apr 2022 07:58:03 PDT (-0700), ebiederm@xmission.com wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
I don't actually know follow the RISC-V flat support, last I heard it was still sort of just in limbo (some toolchain/userspace bugs th at needed to be sorted out). Damien would know better, though, he's already on the thread. I'll leave it up to him to ack this one, if you were even looking for anything from the RISC-V folks at all (we don't have this in any defconfigs).
For what it's worth, bimfmt_flat (with or without shared library support) should be simple to implement as a binfmt_misc handler if anyone needs the old shared library support (or if kernel wanted to drop it entirely, which I would be in favor of). That's how I handled old aout binaries I wanted to run after aout was removed: trivial binfmt_misc loader.
Rich
On Wed, Apr 20, 2022 at 12:59:37PM -0400, Rich Felker wrote:
On Wed, Apr 20, 2022 at 09:17:22AM -0700, Palmer Dabbelt wrote:
On Wed, 20 Apr 2022 07:58:03 PDT (-0700), ebiederm@xmission.com wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
I don't actually know follow the RISC-V flat support, last I heard it was still sort of just in limbo (some toolchain/userspace bugs th at needed to be sorted out). Damien would know better, though, he's already on the thread. I'll leave it up to him to ack this one, if you were even looking for anything from the RISC-V folks at all (we don't have this in any defconfigs).
For what it's worth, bimfmt_flat (with or without shared library support) should be simple to implement as a binfmt_misc handler if anyone needs the old shared library support (or if kernel wanted to drop it entirely, which I would be in favor of). That's how I handled old aout binaries I wanted to run after aout was removed: trivial binfmt_misc loader.
Yeah, I was trying to understand why systems were using binfmt_flat and not binfmt_elf, given the mention of elf2flat -- is there really such a large kernel memory footprint savings to be had from removing binfmt_elf?
But regardless, yes, it seems like if you're doing anything remotely needing shared libraries with binfmt_flat, such a system could just use ELF instead.
On Wed, Apr 20, 2022 at 7:47 PM Kees Cook keescook@chromium.org wrote:
Yeah, I was trying to understand why systems were using binfmt_flat and not binfmt_elf, given the mention of elf2flat -- is there really such a large kernel memory footprint savings to be had from removing binfmt_elf?
I think the main reason for using flat binaries is nommu support on m68k, xtensa and risc-v. The regular binfmt_elf support requires an MMU, and the elf-fdpic variant is only available for arm and sh at this point (the other nommu architectures got removed over time).
Arnd
On Wed, Apr 20, 2022 at 10:04:32PM +0200, Arnd Bergmann wrote:
On Wed, Apr 20, 2022 at 7:47 PM Kees Cook keescook@chromium.org wrote:
Yeah, I was trying to understand why systems were using binfmt_flat and not binfmt_elf, given the mention of elf2flat -- is there really such a large kernel memory footprint savings to be had from removing binfmt_elf?
I think the main reason for using flat binaries is nommu support on m68k, xtensa and risc-v. The regular binfmt_elf support requires an MMU, and the elf-fdpic variant is only available for arm and sh at this point (the other nommu architectures got removed over time).
I believe I made the elf-fdpic loader so that it's capable of loading normal non-fdpic elf files on nommu (1bde925d23), unless somebody broke that. I also seem to recall that capability being added to the main elf loader later.
Rich
On 4/21/22 05:23, Rich Felker wrote:
On Wed, Apr 20, 2022 at 10:04:32PM +0200, Arnd Bergmann wrote:
On Wed, Apr 20, 2022 at 7:47 PM Kees Cook keescook@chromium.org wrote:
Yeah, I was trying to understand why systems were using binfmt_flat and not binfmt_elf, given the mention of elf2flat -- is there really such a large kernel memory footprint savings to be had from removing binfmt_elf?
I think the main reason for using flat binaries is nommu support on m68k, xtensa and risc-v. The regular binfmt_elf support requires an MMU, and the elf-fdpic variant is only available for arm and sh at this point (the other nommu architectures got removed over time).
I believe I made the elf-fdpic loader so that it's capable of loading normal non-fdpic elf files on nommu (1bde925d23), unless somebody broke that. I also seem to recall that capability being added to the main elf loader later.
Last time I checked, building shared libraries usable with nommu riscv required gcc/ld options that were not supported for riscv (PIE related stuff). So removing the kernel support for shared flat libs is fine with me.
On 4/20/22 12:47, Kees Cook wrote:
For what it's worth, bimfmt_flat (with or without shared library support) should be simple to implement as a binfmt_misc handler if anyone needs the old shared library support (or if kernel wanted to drop it entirely, which I would be in favor of). That's how I handled old aout binaries I wanted to run after aout was removed: trivial binfmt_misc loader.
Yeah, I was trying to understand why systems were using binfmt_flat and not binfmt_elf, given the mention of elf2flat -- is there really such a large kernel memory footprint savings to be had from removing binfmt_elf?
elf2flat is a terrible name: it doesn't take an executable as input, it takes a .o file as input. (I mean it's an elf format .o file, but... misleading.)
But regardless, yes, it seems like if you're doing anything remotely needing shared libraries with binfmt_flat, such a system could just use ELF instead.
A) The binfmt_elf.c loader won't run on nommu systems. The fdpic loader will, and in theory can handle normal ELF binaries (it's ELF with _more_ capabilities), but sadly it's not supported on most architectures for reasons that are unclear to me.
B) You can't run conventional ELF on nommu, because everything is offset from 0 so PID 1 eats that address range and you can't run exec program.
You can run PIE binaries on nommu (the symbols offset from a base pointer which can point anywhere), but they're inefficient (can't share text or rodata sections between instances because every symbol is offset from a single shared base pointer), and highly vulnerable to fragmentation (because it needs a contiguous blob of memory for text, rodata, bss, and data: see single base pointer everything has an integer offset from).
All fdpic really does is give you 4 base pointers, one for each section. That way you can share text and rodata, and put bss and data into smaller independent fragments of memory. Various security guys use this as super-aslr even on mmu systems, but tend not to advertise that they're doing so. :)
Rob
On 25/4/22 13:38, Rob Landley wrote:
On 4/20/22 12:47, Kees Cook wrote:
For what it's worth, bimfmt_flat (with or without shared library support) should be simple to implement as a binfmt_misc handler if anyone needs the old shared library support (or if kernel wanted to drop it entirely, which I would be in favor of). That's how I handled old aout binaries I wanted to run after aout was removed: trivial binfmt_misc loader.
Yeah, I was trying to understand why systems were using binfmt_flat and not binfmt_elf, given the mention of elf2flat -- is there really such a large kernel memory footprint savings to be had from removing binfmt_elf?
elf2flat is a terrible name: it doesn't take an executable as input, it takes a .o file as input. (I mean it's an elf format .o file, but... misleading.)
No, not at all. "elf2flt" is exactly what it does. Couldn't get a more accurate name.
But regardless, yes, it seems like if you're doing anything remotely needing shared libraries with binfmt_flat, such a system could just use ELF instead.
A) The binfmt_elf.c loader won't run on nommu systems. The fdpic loader will, and in theory can handle normal ELF binaries (it's ELF with _more_ capabilities), but sadly it's not supported on most architectures for reasons that are unclear to me.
Inertia. Flat format has been around a very long time. And for most people it just works. Flat format works on MMU systems as well, though you would have to be crazy to choose to do that.
B) You can't run conventional ELF on nommu, because everything is offset from 0 so PID 1 eats that address range and you can't run exec program.
You can run PIE binaries on nommu (the symbols offset from a base pointer which can point anywhere), but they're inefficient (can't share text or rodata sections between instances because every symbol is offset from a single shared base pointer), and highly vulnerable to fragmentation (because it needs a contiguous blob of memory for text, rodata, bss, and data: see single base pointer everything has an integer offset from).
All fdpic really does is give you 4 base pointers, one for each section. That way you can share text and rodata, and put bss and data into smaller independent fragments of memory. Various security guys use this as super-aslr even on mmu systems, but tend not to advertise that they're doing so. :)
Well flat got half way there. You can have separate text/rodata and data/bss, used a lot back in the day for execute-in-place of the code.
Regards Greg
On 4/20/22 23:58, Eric W. Biederman wrote:
Nit: extra blank line here.
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made
s/becasue/because
life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary
s/libarary/library
support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
As mentioned in a different email, it is not supported by the toolchains for riscv.
Was binfmt_flat being enabled on arm and sh the mistake it looks like?
arch/arm/configs/lpc18xx_defconfig | 1 - arch/arm/configs/mps2_defconfig | 1 - arch/arm/configs/stm32_defconfig | 1 - arch/arm/configs/vf610m4_defconfig | 1 - arch/sh/configs/rsk7201_defconfig | 1 - arch/sh/configs/rsk7203_defconfig | 1 - arch/sh/configs/se7206_defconfig | 1 - fs/Kconfig.binfmt | 6 - fs/binfmt_flat.c | 190 ++++++----------------------- 9 files changed, 40 insertions(+), 163 deletions(-)
diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig index be882ea0eee4..688c9849eec8 100644 --- a/arch/arm/configs/lpc18xx_defconfig +++ b/arch/arm/configs/lpc18xx_defconfig @@ -30,7 +30,6 @@ CONFIG_ARM_APPENDED_DTB=y # CONFIG_BLK_DEV_BSG is not set CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_NET=y CONFIG_PACKET=y diff --git a/arch/arm/configs/mps2_defconfig b/arch/arm/configs/mps2_defconfig index 89f4a6ff30bd..c1e98e33a348 100644 --- a/arch/arm/configs/mps2_defconfig +++ b/arch/arm/configs/mps2_defconfig @@ -23,7 +23,6 @@ CONFIG_PREEMPT_VOLUNTARY=y CONFIG_ZBOOT_ROM_TEXT=0x0 CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set # CONFIG_SUSPEND is not set CONFIG_NET=y diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig index 551db328009d..71d6bfcf4551 100644 --- a/arch/arm/configs/stm32_defconfig +++ b/arch/arm/configs/stm32_defconfig @@ -28,7 +28,6 @@ CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x08008000 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_DEVTMPFS=y CONFIG_DEVTMPFS_MOUNT=y diff --git a/arch/arm/configs/vf610m4_defconfig b/arch/arm/configs/vf610m4_defconfig index a89f035c3b01..70fdbfd83484 100644 --- a/arch/arm/configs/vf610m4_defconfig +++ b/arch/arm/configs/vf610m4_defconfig @@ -18,7 +18,6 @@ CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x0f000080 CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_SUSPEND is not set # CONFIG_UEVENT_HELPER is not set # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7201_defconfig b/arch/sh/configs/rsk7201_defconfig index e41526120be1..619c18699459 100644 --- a/arch/sh/configs/rsk7201_defconfig +++ b/arch/sh/configs/rsk7201_defconfig @@ -25,7 +25,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7203_defconfig b/arch/sh/configs/rsk7203_defconfig index 6af08fa1ddf8..5a54e2b883f0 100644 --- a/arch/sh/configs/rsk7203_defconfig +++ b/arch/sh/configs/rsk7203_defconfig @@ -30,7 +30,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y CONFIG_NET=y diff --git a/arch/sh/configs/se7206_defconfig b/arch/sh/configs/se7206_defconfig index 601d062250d1..122216123e63 100644 --- a/arch/sh/configs/se7206_defconfig +++ b/arch/sh/configs/se7206_defconfig @@ -40,7 +40,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC3,115200 ignore_loglevel earlyprintk=serial" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_BINFMT_MISC=y CONFIG_NET=y CONFIG_PACKET=y diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt index 21c6332fa785..32dff7ba3dda 100644 --- a/fs/Kconfig.binfmt +++ b/fs/Kconfig.binfmt @@ -142,12 +142,6 @@ config BINFMT_ZFLAT help Support FLAT format compressed binaries -config BINFMT_SHARED_FLAT
- bool "Enable shared FLAT support"
- depends on BINFMT_FLAT
- help
Support FLAT shared librariesconfig HAVE_AOUT def_bool n diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 0ad2c7bbaddd..82e4412a9665 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -68,11 +68,7 @@ #define RELOC_FAILED 0xff00ff01 /* Relocation incorrect somewhere */ #define UNLOADED_LIB 0x7ff000ff /* Placeholder for unused library */ -#ifdef CONFIG_BINFMT_SHARED_FLAT -#define MAX_SHARED_LIBS (4) -#else -#define MAX_SHARED_LIBS (1) -#endif +#define MAX_SHARED_LIBS (1) #ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET #define DATA_START_OFFSET_WORDS (0) @@ -92,10 +88,6 @@ struct lib_info { } lib_list[MAX_SHARED_LIBS]; }; -#ifdef CONFIG_BINFMT_SHARED_FLAT -static int load_flat_shared_library(int id, struct lib_info *p); -#endif
static int load_flat_binary(struct linux_binprm *); static struct linux_binfmt flat_format = { @@ -307,51 +299,18 @@ static int decompress_exec(struct linux_binprm *bprm, loff_t fpos, char *dst, /****************************************************************************/ static unsigned long -calc_reloc(unsigned long r, struct lib_info *p, int curid, int internalp) +calc_reloc(unsigned long r, struct lib_info *p) { unsigned long addr;
- int id; unsigned long start_brk; unsigned long start_data; unsigned long text_len; unsigned long start_code;
-#ifdef CONFIG_BINFMT_SHARED_FLAT
- if (r == 0)
id = curid; /* Relocs of 0 are always self referring */- else {
id = (r >> 24) & 0xff; /* Find ID for this reloc */r &= 0x00ffffff; /* Trim ID off here */- }
- if (id >= MAX_SHARED_LIBS) {
pr_err("reference 0x%lx to shared library %d", r, id);goto failed;- }
- if (curid != id) {
if (internalp) {pr_err("reloc address 0x%lx not in same module ""(%d != %d)", r, curid, id);goto failed;} else if (!p->lib_list[id].loaded &&load_flat_shared_library(id, p) < 0) {pr_err("failed to load library %d", id);goto failed;}/* Check versioning information (i.e. time stamps) */if (p->lib_list[id].build_date && p->lib_list[curid].build_date &&p->lib_list[curid].build_date < p->lib_list[id].build_date) {pr_err("library %d is younger than %d", id, curid);goto failed;}- }
-#else
- id = 0;
-#endif
- start_brk = p->lib_list[id].start_brk;
- start_data = p->lib_list[id].start_data;
- start_code = p->lib_list[id].start_code;
- text_len = p->lib_list[id].text_len;
- start_brk = p->lib_list[0].start_brk;
- start_data = p->lib_list[0].start_data;
- start_code = p->lib_list[0].start_code;
- text_len = p->lib_list[0].text_len;
if (r > start_brk - start_data + text_len) { pr_err("reloc outside program 0x%lx (0 - 0x%lx/0x%lx)", @@ -419,7 +378,7 @@ static void old_reloc(unsigned long rl) /****************************************************************************/ static int load_flat_file(struct linux_binprm *bprm,
struct lib_info *libinfo, int id, unsigned long *extra_stack)
struct lib_info *libinfo, unsigned long *extra_stack){ struct flat_hdr *hdr; unsigned long textpos, datapos, realdatastart; @@ -471,14 +430,6 @@ static int load_flat_file(struct linux_binprm *bprm, goto err; }
- /* Don't allow old format executables to use shared libraries */
- if (rev == OLD_FLAT_VERSION && id != 0) {
pr_err("shared libraries are not available before rev 0x%lx\n",FLAT_VERSION);ret = -ENOEXEC;goto err;- }
- /*
- fix up the flags for the older format, there were all kinds
- of endian hacks, this only works for the simple cases
@@ -529,15 +480,13 @@ static int load_flat_file(struct linux_binprm *bprm, } /* Flush all traces of the currently running executable */
- if (id == 0) {
ret = begin_new_exec(bprm);if (ret)goto err;
- ret = begin_new_exec(bprm);
- if (ret)
goto err;
/* OK, This is the point of no return */set_personality(PER_LINUX_32BIT);setup_new_exec(bprm);- }
- /* OK, This is the point of no return */
- set_personality(PER_LINUX_32BIT);
- setup_new_exec(bprm);
/* * calculate the extra space we need to map in @@ -717,42 +666,40 @@ static int load_flat_file(struct linux_binprm *bprm, text_len -= sizeof(struct flat_hdr); /* the real code len */ /* The main program needs a little extra setup in the task structure */
- if (id == 0) {
current->mm->start_code = start_code;current->mm->end_code = end_code;current->mm->start_data = datapos;current->mm->end_data = datapos + data_len;/** set up the brk stuff, uses any slack left in data/bss/stack* allocation. We put the brk after the bss (between the bss* and stack) like other platforms.* Userspace code relies on the stack pointer starting out at* an address right at the end of a page.*/current->mm->start_brk = datapos + data_len + bss_len;current->mm->brk = (current->mm->start_brk + 3) & ~3;
- current->mm->start_code = start_code;
- current->mm->end_code = end_code;
- current->mm->start_data = datapos;
- current->mm->end_data = datapos + data_len;
- /*
* set up the brk stuff, uses any slack left in data/bss/stack* allocation. We put the brk after the bss (between the bss* and stack) like other platforms.* Userspace code relies on the stack pointer starting out at* an address right at the end of a page.*/- current->mm->start_brk = datapos + data_len + bss_len;
- current->mm->brk = (current->mm->start_brk + 3) & ~3;
#ifndef CONFIG_MMU
current->mm->context.end_brk = memp + memp_size - stack_len;
- current->mm->context.end_brk = memp + memp_size - stack_len;
#endif
- }
if (flags & FLAT_FLAG_KTRACE) { pr_info("Mapping is %lx, Entry point is %x, data_start is %x\n", textpos, 0x00ffffff&ntohl(hdr->entry), ntohl(hdr->data_start)); pr_info("%s %s: TEXT=%lx-%lx DATA=%lx-%lx BSS=%lx-%lx\n",
id ? "Lib" : "Load", bprm->filename,
"Load", bprm->filename, start_code, end_code, datapos, datapos + data_len, datapos + data_len, (datapos + data_len + bss_len + 3) & ~3);/* Store the current module values into the global library structure */
- libinfo->lib_list[id].start_code = start_code;
- libinfo->lib_list[id].start_data = datapos;
- libinfo->lib_list[id].start_brk = datapos + data_len + bss_len;
- libinfo->lib_list[id].text_len = text_len;
- libinfo->lib_list[id].loaded = 1;
- libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
- libinfo->lib_list[id].build_date = ntohl(hdr->build_date);
- libinfo->lib_list[0].start_code = start_code;
- libinfo->lib_list[0].start_data = datapos;
- libinfo->lib_list[0].start_brk = datapos + data_len + bss_len;
- libinfo->lib_list[0].text_len = text_len;
- libinfo->lib_list[0].loaded = 1;
- libinfo->lib_list[0].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
- libinfo->lib_list[0].build_date = ntohl(hdr->build_date);
/* * We just load the allocations into some temporary memory to @@ -774,7 +721,7 @@ static int load_flat_file(struct linux_binprm *bprm, if (rp_val == 0xffffffff) break; if (rp_val) {
addr = calc_reloc(rp_val, libinfo, id, 0);
addr = calc_reloc(rp_val, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err;@@ -810,7 +757,7 @@ static int load_flat_file(struct linux_binprm *bprm, return -EFAULT; relval = ntohl(tmp); addr = flat_get_relocate_addr(relval);
rp = (u32 __user *)calc_reloc(addr, libinfo, id, 1);
rp = (u32 __user *)calc_reloc(addr, libinfo); if (rp == (u32 __user *)RELOC_FAILED) { ret = -ENOEXEC; goto err;@@ -833,7 +780,7 @@ static int load_flat_file(struct linux_binprm *bprm, */ addr = ntohl((__force __be32)addr); }
addr = calc_reloc(addr, libinfo, id, 0);
addr = calc_reloc(addr, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err;@@ -861,7 +808,7 @@ static int load_flat_file(struct linux_binprm *bprm, /* zero the BSS, BRK and stack areas */ if (clear_user((void __user *)(datapos + data_len), bss_len + (memp + memp_size - stack_len - /* end brk */
libinfo->lib_list[id].start_brk) + /* start brk */
libinfo->lib_list[0].start_brk) + /* start brk */ stack_len))@@ -871,49 +818,6 @@ static int load_flat_file(struct linux_binprm *bprm, } -/****************************************************************************/ -#ifdef CONFIG_BINFMT_SHARED_FLAT
-/*
- Load a shared library into memory. The library gets its own data
- segment (including bss) but not argv/argc/environ.
- */
-static int load_flat_shared_library(int id, struct lib_info *libs) -{
- /*
* This is a fake bprm struct; only the members "buf", "file" and* "filename" are actually used.*/- struct linux_binprm bprm;
- int res;
- char buf[16];
- loff_t pos = 0;
- memset(&bprm, 0, sizeof(bprm));
- /* Create the file name */
- sprintf(buf, "/lib/lib%d.so", id);
- /* Open the file up */
- bprm.filename = buf;
- bprm.file = open_exec(bprm.filename);
- res = PTR_ERR(bprm.file);
- if (IS_ERR(bprm.file))
return res;- res = kernel_read(bprm.file, bprm.buf, BINPRM_BUF_SIZE, &pos);
- if (res >= 0)
res = load_flat_file(&bprm, libs, id, NULL);- allow_write_access(bprm.file);
- fput(bprm.file);
- return res;
-}
-#endif /* CONFIG_BINFMT_SHARED_FLAT */ /****************************************************************************/ /* @@ -946,7 +850,7 @@ static int load_flat_binary(struct linux_binprm *bprm) stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ stack_len = ALIGN(stack_len, FLAT_STACK_ALIGN);
- res = load_flat_file(bprm, &libinfo, 0, &stack_len);
- res = load_flat_file(bprm, &libinfo, &stack_len); if (res < 0) return res;
@@ -991,20 +895,6 @@ static int load_flat_binary(struct linux_binprm *bprm) */ start_addr = libinfo.lib_list[0].entry; -#ifdef CONFIG_BINFMT_SHARED_FLAT
- for (i = MAX_SHARED_LIBS-1; i > 0; i--) {
if (libinfo.lib_list[i].loaded) {/* Push previos first to call address */unsigned long __user *sp;current->mm->start_stack -= sizeof(unsigned long);sp = (unsigned long __user *)current->mm->start_stack;if (put_user(start_addr, sp))return -EFAULT;start_addr = libinfo.lib_list[i].entry;}- }
-#endif
#ifdef FLAT_PLAT_INIT FLAT_PLAT_INIT(regs); #endif
Apart from the typos mentioned above, looks OK to me.
Reviewed-by: Damien Le Moal damien.lemoal@opensource.wdc.com
Hi Eric,
On 21/4/22 00:58, Eric W. Biederman wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
I can definitely confirm I don't use it on m68k. And I don't know of anyone that has used it in many years.
Was binfmt_flat being enabled on arm and sh the mistake it looks like?
arch/arm/configs/lpc18xx_defconfig | 1 - arch/arm/configs/mps2_defconfig | 1 - arch/arm/configs/stm32_defconfig | 1 - arch/arm/configs/vf610m4_defconfig | 1 -
binfmt_flat works on ARM. I use it all the time. According to those defconfigs those are all non-MMU systems, so having binfmt_flat enabled makes some sense there.
arch/sh/configs/rsk7201_defconfig | 1 - arch/sh/configs/rsk7203_defconfig | 1 - arch/sh/configs/se7206_defconfig | 1 -
Those are all SH2 systems if I am reading the defconfigs correctly. SH2 is non-MMU according to the Kconfig setup. So it makes sense that binfmt_flat is enabled on those too.
Don't forget that binfmt_flat works on many MMU enabled systems too. I regularly test/check it on MMU enabled m68k systems for example.
Regards Greg
fs/Kconfig.binfmt | 6 - fs/binfmt_flat.c | 190 ++++++----------------------- 9 files changed, 40 insertions(+), 163 deletions(-)
diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig index be882ea0eee4..688c9849eec8 100644 --- a/arch/arm/configs/lpc18xx_defconfig +++ b/arch/arm/configs/lpc18xx_defconfig @@ -30,7 +30,6 @@ CONFIG_ARM_APPENDED_DTB=y # CONFIG_BLK_DEV_BSG is not set CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_NET=y CONFIG_PACKET=y diff --git a/arch/arm/configs/mps2_defconfig b/arch/arm/configs/mps2_defconfig index 89f4a6ff30bd..c1e98e33a348 100644 --- a/arch/arm/configs/mps2_defconfig +++ b/arch/arm/configs/mps2_defconfig @@ -23,7 +23,6 @@ CONFIG_PREEMPT_VOLUNTARY=y CONFIG_ZBOOT_ROM_TEXT=0x0 CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set # CONFIG_SUSPEND is not set CONFIG_NET=y diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig index 551db328009d..71d6bfcf4551 100644 --- a/arch/arm/configs/stm32_defconfig +++ b/arch/arm/configs/stm32_defconfig @@ -28,7 +28,6 @@ CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x08008000 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_DEVTMPFS=y CONFIG_DEVTMPFS_MOUNT=y diff --git a/arch/arm/configs/vf610m4_defconfig b/arch/arm/configs/vf610m4_defconfig index a89f035c3b01..70fdbfd83484 100644 --- a/arch/arm/configs/vf610m4_defconfig +++ b/arch/arm/configs/vf610m4_defconfig @@ -18,7 +18,6 @@ CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x0f000080 CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_SUSPEND is not set # CONFIG_UEVENT_HELPER is not set # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7201_defconfig b/arch/sh/configs/rsk7201_defconfig index e41526120be1..619c18699459 100644 --- a/arch/sh/configs/rsk7201_defconfig +++ b/arch/sh/configs/rsk7201_defconfig @@ -25,7 +25,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7203_defconfig b/arch/sh/configs/rsk7203_defconfig index 6af08fa1ddf8..5a54e2b883f0 100644 --- a/arch/sh/configs/rsk7203_defconfig +++ b/arch/sh/configs/rsk7203_defconfig @@ -30,7 +30,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y CONFIG_NET=y diff --git a/arch/sh/configs/se7206_defconfig b/arch/sh/configs/se7206_defconfig index 601d062250d1..122216123e63 100644 --- a/arch/sh/configs/se7206_defconfig +++ b/arch/sh/configs/se7206_defconfig @@ -40,7 +40,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC3,115200 ignore_loglevel earlyprintk=serial" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_BINFMT_MISC=y CONFIG_NET=y CONFIG_PACKET=y diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt index 21c6332fa785..32dff7ba3dda 100644 --- a/fs/Kconfig.binfmt +++ b/fs/Kconfig.binfmt @@ -142,12 +142,6 @@ config BINFMT_ZFLAT help Support FLAT format compressed binaries -config BINFMT_SHARED_FLAT
- bool "Enable shared FLAT support"
- depends on BINFMT_FLAT
- help
Support FLAT shared libraries- config HAVE_AOUT def_bool n
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 0ad2c7bbaddd..82e4412a9665 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -68,11 +68,7 @@ #define RELOC_FAILED 0xff00ff01 /* Relocation incorrect somewhere */ #define UNLOADED_LIB 0x7ff000ff /* Placeholder for unused library */ -#ifdef CONFIG_BINFMT_SHARED_FLAT -#define MAX_SHARED_LIBS (4) -#else -#define MAX_SHARED_LIBS (1) -#endif +#define MAX_SHARED_LIBS (1) #ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET #define DATA_START_OFFSET_WORDS (0) @@ -92,10 +88,6 @@ struct lib_info { } lib_list[MAX_SHARED_LIBS]; }; -#ifdef CONFIG_BINFMT_SHARED_FLAT -static int load_flat_shared_library(int id, struct lib_info *p); -#endif
- static int load_flat_binary(struct linux_binprm *);
static struct linux_binfmt flat_format = { @@ -307,51 +299,18 @@ static int decompress_exec(struct linux_binprm *bprm, loff_t fpos, char *dst, /****************************************************************************/ static unsigned long -calc_reloc(unsigned long r, struct lib_info *p, int curid, int internalp) +calc_reloc(unsigned long r, struct lib_info *p) { unsigned long addr;
- int id; unsigned long start_brk; unsigned long start_data; unsigned long text_len; unsigned long start_code;
-#ifdef CONFIG_BINFMT_SHARED_FLAT
- if (r == 0)
id = curid; /* Relocs of 0 are always self referring */- else {
id = (r >> 24) & 0xff; /* Find ID for this reloc */r &= 0x00ffffff; /* Trim ID off here */- }
- if (id >= MAX_SHARED_LIBS) {
pr_err("reference 0x%lx to shared library %d", r, id);goto failed;- }
- if (curid != id) {
if (internalp) {pr_err("reloc address 0x%lx not in same module ""(%d != %d)", r, curid, id);goto failed;} else if (!p->lib_list[id].loaded &&load_flat_shared_library(id, p) < 0) {pr_err("failed to load library %d", id);goto failed;}/* Check versioning information (i.e. time stamps) */if (p->lib_list[id].build_date && p->lib_list[curid].build_date &&p->lib_list[curid].build_date < p->lib_list[id].build_date) {pr_err("library %d is younger than %d", id, curid);goto failed;}- }
-#else
- id = 0;
-#endif
- start_brk = p->lib_list[id].start_brk;
- start_data = p->lib_list[id].start_data;
- start_code = p->lib_list[id].start_code;
- text_len = p->lib_list[id].text_len;
- start_brk = p->lib_list[0].start_brk;
- start_data = p->lib_list[0].start_data;
- start_code = p->lib_list[0].start_code;
- text_len = p->lib_list[0].text_len;
if (r > start_brk - start_data + text_len) { pr_err("reloc outside program 0x%lx (0 - 0x%lx/0x%lx)", @@ -419,7 +378,7 @@ static void old_reloc(unsigned long rl) /****************************************************************************/ static int load_flat_file(struct linux_binprm *bprm,
struct lib_info *libinfo, int id, unsigned long *extra_stack)
struct lib_info *libinfo, unsigned long *extra_stack)@@ -471,14 +430,6 @@ static int load_flat_file(struct linux_binprm *bprm, goto err; }
- /* Don't allow old format executables to use shared libraries */
- if (rev == OLD_FLAT_VERSION && id != 0) {
pr_err("shared libraries are not available before rev 0x%lx\n",FLAT_VERSION);ret = -ENOEXEC;goto err;- }
- /*
- fix up the flags for the older format, there were all kinds
- of endian hacks, this only works for the simple cases
@@ -529,15 +480,13 @@ static int load_flat_file(struct linux_binprm *bprm, } /* Flush all traces of the currently running executable */
- if (id == 0) {
ret = begin_new_exec(bprm);if (ret)goto err;
- ret = begin_new_exec(bprm);
- if (ret)
goto err;
/* OK, This is the point of no return */set_personality(PER_LINUX_32BIT);setup_new_exec(bprm);- }
- /* OK, This is the point of no return */
- set_personality(PER_LINUX_32BIT);
- setup_new_exec(bprm);
/* * calculate the extra space we need to map in @@ -717,42 +666,40 @@ static int load_flat_file(struct linux_binprm *bprm, text_len -= sizeof(struct flat_hdr); /* the real code len */ /* The main program needs a little extra setup in the task structure */
- if (id == 0) {
current->mm->start_code = start_code;current->mm->end_code = end_code;current->mm->start_data = datapos;current->mm->end_data = datapos + data_len;/** set up the brk stuff, uses any slack left in data/bss/stack* allocation. We put the brk after the bss (between the bss* and stack) like other platforms.* Userspace code relies on the stack pointer starting out at* an address right at the end of a page.*/current->mm->start_brk = datapos + data_len + bss_len;current->mm->brk = (current->mm->start_brk + 3) & ~3;
- current->mm->start_code = start_code;
- current->mm->end_code = end_code;
- current->mm->start_data = datapos;
- current->mm->end_data = datapos + data_len;
- /*
* set up the brk stuff, uses any slack left in data/bss/stack* allocation. We put the brk after the bss (between the bss* and stack) like other platforms.* Userspace code relies on the stack pointer starting out at* an address right at the end of a page.*/- current->mm->start_brk = datapos + data_len + bss_len;
- current->mm->brk = (current->mm->start_brk + 3) & ~3; #ifndef CONFIG_MMU
current->mm->context.end_brk = memp + memp_size - stack_len;
- current->mm->context.end_brk = memp + memp_size - stack_len; #endif
- }
if (flags & FLAT_FLAG_KTRACE) { pr_info("Mapping is %lx, Entry point is %x, data_start is %x\n", textpos, 0x00ffffff&ntohl(hdr->entry), ntohl(hdr->data_start)); pr_info("%s %s: TEXT=%lx-%lx DATA=%lx-%lx BSS=%lx-%lx\n",
id ? "Lib" : "Load", bprm->filename,
"Load", bprm->filename, start_code, end_code, datapos, datapos + data_len, datapos + data_len, (datapos + data_len + bss_len + 3) & ~3);/* Store the current module values into the global library structure */
- libinfo->lib_list[id].start_code = start_code;
- libinfo->lib_list[id].start_data = datapos;
- libinfo->lib_list[id].start_brk = datapos + data_len + bss_len;
- libinfo->lib_list[id].text_len = text_len;
- libinfo->lib_list[id].loaded = 1;
- libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
- libinfo->lib_list[id].build_date = ntohl(hdr->build_date);
- libinfo->lib_list[0].start_code = start_code;
- libinfo->lib_list[0].start_data = datapos;
- libinfo->lib_list[0].start_brk = datapos + data_len + bss_len;
- libinfo->lib_list[0].text_len = text_len;
- libinfo->lib_list[0].loaded = 1;
- libinfo->lib_list[0].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos;
- libinfo->lib_list[0].build_date = ntohl(hdr->build_date);
/* * We just load the allocations into some temporary memory to @@ -774,7 +721,7 @@ static int load_flat_file(struct linux_binprm *bprm, if (rp_val == 0xffffffff) break; if (rp_val) {
addr = calc_reloc(rp_val, libinfo, id, 0);
addr = calc_reloc(rp_val, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err;@@ -810,7 +757,7 @@ static int load_flat_file(struct linux_binprm *bprm, return -EFAULT; relval = ntohl(tmp); addr = flat_get_relocate_addr(relval);
rp = (u32 __user *)calc_reloc(addr, libinfo, id, 1);
rp = (u32 __user *)calc_reloc(addr, libinfo); if (rp == (u32 __user *)RELOC_FAILED) { ret = -ENOEXEC; goto err;@@ -833,7 +780,7 @@ static int load_flat_file(struct linux_binprm *bprm, */ addr = ntohl((__force __be32)addr); }
addr = calc_reloc(addr, libinfo, id, 0);
addr = calc_reloc(addr, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err;@@ -861,7 +808,7 @@ static int load_flat_file(struct linux_binprm *bprm, /* zero the BSS, BRK and stack areas */ if (clear_user((void __user *)(datapos + data_len), bss_len + (memp + memp_size - stack_len - /* end brk */
libinfo->lib_list[id].start_brk) + /* start brk */
libinfo->lib_list[0].start_brk) + /* start brk */ stack_len))@@ -871,49 +818,6 @@ static int load_flat_file(struct linux_binprm *bprm, } -/****************************************************************************/ -#ifdef CONFIG_BINFMT_SHARED_FLAT
-/*
- Load a shared library into memory. The library gets its own data
- segment (including bss) but not argv/argc/environ.
- */
-static int load_flat_shared_library(int id, struct lib_info *libs) -{
- /*
* This is a fake bprm struct; only the members "buf", "file" and* "filename" are actually used.*/- struct linux_binprm bprm;
- int res;
- char buf[16];
- loff_t pos = 0;
- memset(&bprm, 0, sizeof(bprm));
- /* Create the file name */
- sprintf(buf, "/lib/lib%d.so", id);
- /* Open the file up */
- bprm.filename = buf;
- bprm.file = open_exec(bprm.filename);
- res = PTR_ERR(bprm.file);
- if (IS_ERR(bprm.file))
return res;- res = kernel_read(bprm.file, bprm.buf, BINPRM_BUF_SIZE, &pos);
- if (res >= 0)
res = load_flat_file(&bprm, libs, id, NULL);- allow_write_access(bprm.file);
- fput(bprm.file);
- return res;
-}
-#endif /* CONFIG_BINFMT_SHARED_FLAT */ /****************************************************************************/ /* @@ -946,7 +850,7 @@ static int load_flat_binary(struct linux_binprm *bprm) stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ stack_len = ALIGN(stack_len, FLAT_STACK_ALIGN);
- res = load_flat_file(bprm, &libinfo, 0, &stack_len);
- res = load_flat_file(bprm, &libinfo, &stack_len); if (res < 0) return res;
@@ -991,20 +895,6 @@ static int load_flat_binary(struct linux_binprm *bprm) */ start_addr = libinfo.lib_list[0].entry; -#ifdef CONFIG_BINFMT_SHARED_FLAT
- for (i = MAX_SHARED_LIBS-1; i > 0; i--) {
if (libinfo.lib_list[i].loaded) {/* Push previos first to call address */unsigned long __user *sp;current->mm->start_stack -= sizeof(unsigned long);sp = (unsigned long __user *)current->mm->start_stack;if (put_user(start_addr, sp))return -EFAULT;start_addr = libinfo.lib_list[i].entry;}- }
-#endif
- #ifdef FLAT_PLAT_INIT FLAT_PLAT_INIT(regs); #endif
On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer gerg@linux-m68k.org wrote:
On 21/4/22 00:58, Eric W. Biederman wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
I can definitely confirm I don't use it on m68k. And I don't know of anyone that has used it in many years.
Was binfmt_flat being enabled on arm and sh the mistake it looks like?
I think the question was intended to be
Was *binfmt_flat_shared_flat* being enabled on arm and sh the mistake it looks like?
arch/arm/configs/lpc18xx_defconfig | 1 - arch/arm/configs/mps2_defconfig | 1 - arch/arm/configs/stm32_defconfig | 1 - arch/arm/configs/vf610m4_defconfig | 1 -
binfmt_flat works on ARM. I use it all the time. According to those defconfigs those are all non-MMU systems, so having binfmt_flat enabled makes some sense there.
arch/sh/configs/rsk7201_defconfig | 1 - arch/sh/configs/rsk7203_defconfig | 1 - arch/sh/configs/se7206_defconfig | 1 -
Those are all SH2 systems if I am reading the defconfigs correctly. SH2 is non-MMU according to the Kconfig setup. So it makes sense that binfmt_flat is enabled on those too.
I've checked git history, and CONFIG_BINFMT_SHARED_FLAT was enabled in se7206_defconfig in a non-specific defconfig update, so no further info. The other two had it enabled since their introduction, so I guess they were just based on the former.
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Thu, Apr 21, 2022 at 8:52 AM Geert Uytterhoeven geert@linux-m68k.org wrote:
On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer gerg@linux-m68k.org wrote:
On 21/4/22 00:58, Eric W. Biederman wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
I can definitely confirm I don't use it on m68k. And I don't know of anyone that has used it in many years.
Was binfmt_flat being enabled on arm and sh the mistake it looks like?
I think the question was intended to be
Was *binfmt_flat_shared_flat* being enabled on arm and sh the mistake it looks like?arch/arm/configs/lpc18xx_defconfig | 1 - arch/arm/configs/mps2_defconfig | 1 - arch/arm/configs/stm32_defconfig | 1 - arch/arm/configs/vf610m4_defconfig | 1 -
Adding stm32, mps2 and imxrt maintainers to Cc, they are the most active armv7-m users and should know if the shared library support is used anywhere.
Arnd
On 4/21/22 8:12 AM, Arnd Bergmann wrote:
On Thu, Apr 21, 2022 at 8:52 AM Geert Uytterhoeven geert@linux-m68k.org wrote:
On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer gerg@linux-m68k.org wrote:
On 21/4/22 00:58, Eric W. Biederman wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
I can definitely confirm I don't use it on m68k. And I don't know of anyone that has used it in many years.
Was binfmt_flat being enabled on arm and sh the mistake it looks like?
I think the question was intended to be
Was *binfmt_flat_shared_flat* being enabled on arm and sh the mistake it looks like?arch/arm/configs/lpc18xx_defconfig | 1 - arch/arm/configs/mps2_defconfig | 1 - arch/arm/configs/stm32_defconfig | 1 - arch/arm/configs/vf610m4_defconfig | 1 -
Adding stm32, mps2 and imxrt maintainers to Cc, they are the most active armv7-m users and should know if the shared library support is used anywhere.
Never seen shared library in use for flat format, so FWIW
Acked-by: Vladimir Murzin vladimir.murzin@arm.com # ARM
Arnd
On Thu, Apr 21, 2022 at 08:52:59AM +0200, Geert Uytterhoeven wrote:
On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer gerg@linux-m68k.org wrote:
On 21/4/22 00:58, Eric W. Biederman wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
I can definitely confirm I don't use it on m68k. And I don't know of anyone that has used it in many years.
Was binfmt_flat being enabled on arm and sh the mistake it looks like?
I think the question was intended to be
Was *binfmt_flat_shared_flat* being enabled on arm and sh the mistake it looks like?
Early in my work on j2, I tried to research the history of shared flat support on sh, and it turned out the mainline tooling never even supported it, and the out-of-line tooling I eventually found was using all sorts of wrong conditionals for how it did the linking and elf2flt conversion, e.g. mere presence of any PIC-like relocation in any file made it assume the whole program was PIC-compatible. There's no way that stuf was ever used in any meaningful way. It just didn't work.
Quickly dropped that and got plain ELF (no shared text/xip, but no worse than the existing flat support) working, and soon after, FDPIC.
The whole binfmt_flat ecosystem is a mess with no good reason to exist.
Rich
On 4/21/22 07:43, Rich Felker wrote:
On Thu, Apr 21, 2022 at 08:52:59AM +0200, Geert Uytterhoeven wrote:
On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer gerg@linux-m68k.org wrote:
On 21/4/22 00:58, Eric W. Biederman wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier.
[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.or... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com
Can the binfmt_flat folks please verify that the shared library support really isn't used?
I can definitely confirm I don't use it on m68k. And I don't know of anyone that has used it in many years.
Was binfmt_flat being enabled on arm and sh the mistake it looks like?
I think the question was intended to be
Was *binfmt_flat_shared_flat* being enabled on arm and sh the mistake it looks like?Early in my work on j2, I tried to research the history of shared flat support on sh, and it turned out the mainline tooling never even supported it, and the out-of-line tooling I eventually found was using all sorts of wrong conditionals for how it did the linking and elf2flt conversion, e.g. mere presence of any PIC-like relocation in any file made it assume the whole program was PIC-compatible. There's no way that stuf was ever used in any meaningful way. It just didn't work.
Quickly dropped that and got plain ELF (no shared text/xip, but no worse than the existing flat support) working, and soon after, FDPIC.
The whole binfmt_flat ecosystem is a mess with no good reason to exist.
FYI when I had to come up to speed on this in 2014 I did a writeup on my own research:
https://landley.net/notes-2014.html#07-12-2014
The lack of a canonical "upstream" elf2flt repository was probably the biggest problem at the time.
(There's a reason I grabbed fdpic hard and tried to make that work everywhere.)
Rich
Rob
On Wed, 20 Apr 2022 09:58:03 -0500, Eric W. Biederman wrote:
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time.
The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c.
[...]
It seems like there is agreement that the shared library support is unused, so let's test that assumption. :)
With typos fixed up, applied to for-next/execve, thanks!
[1/1] binfmt_flat: Remove shared library support https://git.kernel.org/kees/c/c85b5d88951b
On Thu, 14 Apr 2022 11:10:18 +0200, Niklas Cassel wrote:
bFLT binaries are usually created using elf2flt.
The linker script used by elf2flt has defined the .data section like the following for the last 19 years:
.data : { _sdata = . ; __data_start = . ; data_start = . ; *(.got.plt) *(.got) FILL(0) ; . = ALIGN(0x20) ; LONG(-1) . = ALIGN(0x20) ; ... }
[...]
Applied to for-next/execve, thanks!
[1/1] binfmt_flat: do not stop relocating GOT entries prematurely on riscv https://git.kernel.org/kees/c/a767e6fd68d2
linux-stable-mirror@lists.linaro.org
-
Arnd Bergmann -
Damien Le Moal -
Eric W. Biederman -
Geert Uytterhoeven -
Greg Ungerer -
Kees Cook -
Konstantin Ryabitsev -
Niklas Cassel -
Niklas Cassel
-
Palmer Dabbelt -
Rich Felker -
Rob Landley -
Vladimir Murzin