2019-05-24 20:20:11

by Jann Horn

[permalink] [raw]
Subject: [PATCH] binfmt_flat: make load_flat_shared_library() work

load_flat_shared_library() is broken: It only calls load_flat_file() if
prepare_binprm() returns zero, but prepare_binprm() returns the number of
bytes read - so this only happens if the file is empty.

Instead, call into load_flat_file() if the number of bytes read is
non-negative. (Even if the number of bytes is zero - in that case,
load_flat_file() will see nullbytes and return a nice -ENOEXEC.)

In addition, remove the code related to bprm creds and stop using
prepare_binprm() - this code is loading a library, not a main executable,
and it only actually uses the members "buf", "file" and "filename" of the
linux_binprm struct. Instead, call kernel_read() directly.

Cc: [email protected]
Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
Signed-off-by: Jann Horn <[email protected]>
---
I only found the bug by looking at the code, I have not verified its
existence at runtime.
Also, this patch is compile-tested only.
It would be nice if someone who works with nommu Linux could have a
look at this patch.
akpm's tree is the right one for this patch, right?

fs/binfmt_flat.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 82a48e830018..e4b59e76afb0 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -856,9 +856,14 @@ static int load_flat_file(struct linux_binprm *bprm,

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));

@@ -872,25 +877,11 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
if (IS_ERR(bprm.file))
return res;

- bprm.cred = prepare_exec_creds();
- res = -ENOMEM;
- if (!bprm.cred)
- goto out;
-
- /* We don't really care about recalculating credentials at this point
- * as we're past the point of no return and are dealing with shared
- * libraries.
- */
- bprm.called_set_creds = 1;
+ res = kernel_read(bprm.file, bprm.buf, BINPRM_BUF_SIZE, &pos);

- res = prepare_binprm(&bprm);
-
- if (!res)
+ if (res >= 0)
res = load_flat_file(&bprm, libs, id, NULL);

- abort_creds(bprm.cred);
-
-out:
allow_write_access(bprm.file);
fput(bprm.file);

--
2.22.0.rc1.257.g3120a18244-goog


2019-05-25 21:47:03

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On Fri, 24 May 2019 22:18:17 +0200 Jann Horn <[email protected]> wrote:

> load_flat_shared_library() is broken: It only calls load_flat_file() if
> prepare_binprm() returns zero, but prepare_binprm() returns the number of
> bytes read - so this only happens if the file is empty.

ouch.

> Instead, call into load_flat_file() if the number of bytes read is
> non-negative. (Even if the number of bytes is zero - in that case,
> load_flat_file() will see nullbytes and return a nice -ENOEXEC.)
>
> In addition, remove the code related to bprm creds and stop using
> prepare_binprm() - this code is loading a library, not a main executable,
> and it only actually uses the members "buf", "file" and "filename" of the
> linux_binprm struct. Instead, call kernel_read() directly.
>
> Cc: [email protected]
> Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
> Signed-off-by: Jann Horn <[email protected]>
> ---
> I only found the bug by looking at the code, I have not verified its
> existence at runtime.
> Also, this patch is compile-tested only.
> It would be nice if someone who works with nommu Linux could have a
> look at this patch.

287980e49ffc was three years ago! Has it really been broken for all
that time? If so, it seems a good source of freed disk space...

2019-05-27 13:40:12

by Jann Horn

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On Sat, May 25, 2019 at 11:43 PM Andrew Morton
<[email protected]> wrote:
> On Fri, 24 May 2019 22:18:17 +0200 Jann Horn <[email protected]> wrote:
> > load_flat_shared_library() is broken: It only calls load_flat_file() if
> > prepare_binprm() returns zero, but prepare_binprm() returns the number of
> > bytes read - so this only happens if the file is empty.
>
> ouch.
>
> > Instead, call into load_flat_file() if the number of bytes read is
> > non-negative. (Even if the number of bytes is zero - in that case,
> > load_flat_file() will see nullbytes and return a nice -ENOEXEC.)
> >
> > In addition, remove the code related to bprm creds and stop using
> > prepare_binprm() - this code is loading a library, not a main executable,
> > and it only actually uses the members "buf", "file" and "filename" of the
> > linux_binprm struct. Instead, call kernel_read() directly.
> >
> > Cc: [email protected]
> > Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
> > Signed-off-by: Jann Horn <[email protected]>
> > ---
> > I only found the bug by looking at the code, I have not verified its
> > existence at runtime.
> > Also, this patch is compile-tested only.
> > It would be nice if someone who works with nommu Linux could have a
> > look at this patch.
>
> 287980e49ffc was three years ago! Has it really been broken for all
> that time? If so, it seems a good source of freed disk space...

Maybe... but I didn't want to rip it out without having one of the
maintainers confirm that this really isn't likely to be used anymore.

2019-05-27 14:40:35

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On Mon, 27 May 2019, Jann Horn wrote:

> On Sat, May 25, 2019 at 11:43 PM Andrew Morton
> <[email protected]> wrote:
> > On Fri, 24 May 2019 22:18:17 +0200 Jann Horn <[email protected]> wrote:
> > > load_flat_shared_library() is broken: It only calls load_flat_file() if
> > > prepare_binprm() returns zero, but prepare_binprm() returns the number of
> > > bytes read - so this only happens if the file is empty.
> >
> > ouch.
> >
> > > Instead, call into load_flat_file() if the number of bytes read is
> > > non-negative. (Even if the number of bytes is zero - in that case,
> > > load_flat_file() will see nullbytes and return a nice -ENOEXEC.)
> > >
> > > In addition, remove the code related to bprm creds and stop using
> > > prepare_binprm() - this code is loading a library, not a main executable,
> > > and it only actually uses the members "buf", "file" and "filename" of the
> > > linux_binprm struct. Instead, call kernel_read() directly.
> > >
> > > Cc: [email protected]
> > > Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
> > > Signed-off-by: Jann Horn <[email protected]>
> > > ---
> > > I only found the bug by looking at the code, I have not verified its
> > > existence at runtime.
> > > Also, this patch is compile-tested only.
> > > It would be nice if someone who works with nommu Linux could have a
> > > look at this patch.
> >
> > 287980e49ffc was three years ago! Has it really been broken for all
> > that time? If so, it seems a good source of freed disk space...
>
> Maybe... but I didn't want to rip it out without having one of the
> maintainers confirm that this really isn't likely to be used anymore.

Last time I played with this, I couldn't figure out the toolchain to
produce shared libs. Only static executables worked fine. If I recall,
existing binfmt_flat distros don't use shared libs either.

For shared lib support on no-MMU target, binfmt_elf_fdpic is a much
saner choice.


Nicolas

2019-05-28 11:00:00

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work


On 27/5/19 11:38 pm, Jann Horn wrote:
> On Sat, May 25, 2019 at 11:43 PM Andrew Morton
> <[email protected]> wrote:
>> On Fri, 24 May 2019 22:18:17 +0200 Jann Horn <[email protected]> wrote:
>>> load_flat_shared_library() is broken: It only calls load_flat_file() if
>>> prepare_binprm() returns zero, but prepare_binprm() returns the number of
>>> bytes read - so this only happens if the file is empty.
>>
>> ouch.
>>
>>> Instead, call into load_flat_file() if the number of bytes read is
>>> non-negative. (Even if the number of bytes is zero - in that case,
>>> load_flat_file() will see nullbytes and return a nice -ENOEXEC.)
>>>
>>> In addition, remove the code related to bprm creds and stop using
>>> prepare_binprm() - this code is loading a library, not a main executable,
>>> and it only actually uses the members "buf", "file" and "filename" of the
>>> linux_binprm struct. Instead, call kernel_read() directly.
>>>
>>> Cc: [email protected]
>>> Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
>>> Signed-off-by: Jann Horn <[email protected]>
>>> ---
>>> I only found the bug by looking at the code, I have not verified its
>>> existence at runtime.
>>> Also, this patch is compile-tested only.
>>> It would be nice if someone who works with nommu Linux could have a
>>> look at this patch.
>>
>> 287980e49ffc was three years ago! Has it really been broken for all
>> that time? If so, it seems a good source of freed disk space...
>
> Maybe... but I didn't want to rip it out without having one of the
> maintainers confirm that this really isn't likely to be used anymore.

I have not used shared libraries on m68k non-mmu setups for
a very long time. At least 10 years I would think.

Regards
Greg



2019-05-28 11:07:41

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work


On 27/5/19 11:38 pm, Jann Horn wrote:
> On Sat, May 25, 2019 at 11:43 PM Andrew Morton
> <[email protected]> wrote:
>> On Fri, 24 May 2019 22:18:17 +0200 Jann Horn <[email protected]> wrote:
>>> load_flat_shared_library() is broken: It only calls load_flat_file() if
>>> prepare_binprm() returns zero, but prepare_binprm() returns the number of
>>> bytes read - so this only happens if the file is empty.
>>
>> ouch.
>>
>>> Instead, call into load_flat_file() if the number of bytes read is
>>> non-negative. (Even if the number of bytes is zero - in that case,
>>> load_flat_file() will see nullbytes and return a nice -ENOEXEC.)
>>>
>>> In addition, remove the code related to bprm creds and stop using
>>> prepare_binprm() - this code is loading a library, not a main executable,
>>> and it only actually uses the members "buf", "file" and "filename" of the
>>> linux_binprm struct. Instead, call kernel_read() directly.
>>>
>>> Cc: [email protected]
>>> Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
>>> Signed-off-by: Jann Horn <[email protected]>
>>> ---
>>> I only found the bug by looking at the code, I have not verified its
>>> existence at runtime.
>>> Also, this patch is compile-tested only.
>>> It would be nice if someone who works with nommu Linux could have a
>>> look at this patch.
>>
>> 287980e49ffc was three years ago! Has it really been broken for all
>> that time? If so, it seems a good source of freed disk space...
>
> Maybe... but I didn't want to rip it out without having one of the
> maintainers confirm that this really isn't likely to be used anymore.

I have not used shared libraries on m68k non-mmu setups for
a very long time. At least 10 years I would think.

Regards
Greg



2019-05-29 11:54:08

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On Tue, May 28, 2019 at 12:56 PM Greg Ungerer
<[email protected]> wrote:
> On 27/5/19 11:38 pm, Jann Horn wrote:
> > On Sat, May 25, 2019 at 11:43 PM Andrew Morton
> > <[email protected]> wrote:
> >> On Fri, 24 May 2019 22:18:17 +0200 Jann Horn <[email protected]> wrote:
> >>> load_flat_shared_library() is broken: It only calls load_flat_file() if
> >>> prepare_binprm() returns zero, but prepare_binprm() returns the number of
> >>> bytes read - so this only happens if the file is empty.
> >>
> >>> Instead, call into load_flat_file() if the number of bytes read is
> >>> non-negative. (Even if the number of bytes is zero - in that case,
> >>> load_flat_file() will see nullbytes and return a nice -ENOEXEC.)
> >>>
> >>> In addition, remove the code related to bprm creds and stop using
> >>> prepare_binprm() - this code is loading a library, not a main executable,
> >>> and it only actually uses the members "buf", "file" and "filename" of the
> >>> linux_binprm struct. Instead, call kernel_read() directly.
> >>>
> >>> Cc: [email protected]
> >>> Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
> >>> Signed-off-by: Jann Horn <[email protected]>
> >>> ---
> >>> I only found the bug by looking at the code, I have not verified its
> >>> existence at runtime.
> >>> Also, this patch is compile-tested only.
> >>> It would be nice if someone who works with nommu Linux could have a
> >>> look at this patch.
> >>
> >> 287980e49ffc was three years ago! Has it really been broken for all
> >> that time? If so, it seems a good source of freed disk space...
> >
> > Maybe... but I didn't want to rip it out without having one of the
> > maintainers confirm that this really isn't likely to be used anymore.
>
> I have not used shared libraries on m68k non-mmu setups for
> a very long time. At least 10 years I would think.
>
> Regards
> Greg
>
>
>

2019-05-29 12:07:26

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On Tue, May 28, 2019 at 12:56 PM Greg Ungerer <[email protected]> wrote:
> On 27/5/19 11:38 pm, Jann Horn wrote:
> > On Sat, May 25, 2019 at 11:43 PM Andrew Morton
> > <[email protected]> wrote:
> >> On Fri, 24 May 2019 22:18:17 +0200 Jann Horn <[email protected]> wrote:
> >>> load_flat_shared_library() is broken: It only calls load_flat_file() if
> >>> prepare_binprm() returns zero, but prepare_binprm() returns the number of
> >>> bytes read - so this only happens if the file is empty.
> >>
> >> ouch.
> >>
> >>> Instead, call into load_flat_file() if the number of bytes read is
> >>> non-negative. (Even if the number of bytes is zero - in that case,
> >>> load_flat_file() will see nullbytes and return a nice -ENOEXEC.)
> >>>
> >>> In addition, remove the code related to bprm creds and stop using
> >>> prepare_binprm() - this code is loading a library, not a main executable,
> >>> and it only actually uses the members "buf", "file" and "filename" of the
> >>> linux_binprm struct. Instead, call kernel_read() directly.
> >>>
> >>> Cc: [email protected]
> >>> Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
> >>> Signed-off-by: Jann Horn <[email protected]>
> >>> ---
> >>> I only found the bug by looking at the code, I have not verified its
> >>> existence at runtime.
> >>> Also, this patch is compile-tested only.
> >>> It would be nice if someone who works with nommu Linux could have a
> >>> look at this patch.
> >>
> >> 287980e49ffc was three years ago! Has it really been broken for all
> >> that time? If so, it seems a good source of freed disk space...
> >
> > Maybe... but I didn't want to rip it out without having one of the
> > maintainers confirm that this really isn't likely to be used anymore.
>
> I have not used shared libraries on m68k non-mmu setups for
> a very long time. At least 10 years I would think.

I think Emcraft have a significant customer base running ARM NOMMU
Linux, I wonder whether they would have run into this (adding
Sergei to Cc).
My suspicion is that they use only binfmt-elf-fdpic, not binfmt-flat.

The only architectures I see that enable binfmt-flat are sh, xtensa
and h8300, but only arch/sh uses CONFIG_BINFMT_SHARED_FLAT
for a few machine specific configurations, and I'm in turn fairly sure
those machines have not run a recent kernel in many years.

The one SH nommu platform likely to have users is j2, and that is
probably always used with musl-libc with elf-fdpic (given that
Rich Felker maintains both the kernel port and the library).

Arnd

2019-05-29 12:31:00

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work


On 29/5/19 10:05 pm, Arnd Bergmann wrote:
> On Tue, May 28, 2019 at 12:56 PM Greg Ungerer <[email protected]> wrote:
>> On 27/5/19 11:38 pm, Jann Horn wrote:
>>> On Sat, May 25, 2019 at 11:43 PM Andrew Morton
>>> <[email protected]> wrote:
>>>> On Fri, 24 May 2019 22:18:17 +0200 Jann Horn <[email protected]> wrote:
>>>>> load_flat_shared_library() is broken: It only calls load_flat_file() if
>>>>> prepare_binprm() returns zero, but prepare_binprm() returns the number of
>>>>> bytes read - so this only happens if the file is empty.
>>>>
>>>> ouch.
>>>>
>>>>> Instead, call into load_flat_file() if the number of bytes read is
>>>>> non-negative. (Even if the number of bytes is zero - in that case,
>>>>> load_flat_file() will see nullbytes and return a nice -ENOEXEC.)
>>>>>
>>>>> In addition, remove the code related to bprm creds and stop using
>>>>> prepare_binprm() - this code is loading a library, not a main executable,
>>>>> and it only actually uses the members "buf", "file" and "filename" of the
>>>>> linux_binprm struct. Instead, call kernel_read() directly.
>>>>>
>>>>> Cc: [email protected]
>>>>> Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
>>>>> Signed-off-by: Jann Horn <[email protected]>
>>>>> ---
>>>>> I only found the bug by looking at the code, I have not verified its
>>>>> existence at runtime.
>>>>> Also, this patch is compile-tested only.
>>>>> It would be nice if someone who works with nommu Linux could have a
>>>>> look at this patch.
>>>>
>>>> 287980e49ffc was three years ago! Has it really been broken for all
>>>> that time? If so, it seems a good source of freed disk space...
>>>
>>> Maybe... but I didn't want to rip it out without having one of the
>>> maintainers confirm that this really isn't likely to be used anymore.
>>
>> I have not used shared libraries on m68k non-mmu setups for
>> a very long time. At least 10 years I would think.
>
> I think Emcraft have a significant customer base running ARM NOMMU
> Linux, I wonder whether they would have run into this (adding
> Sergei to Cc).
> My suspicion is that they use only binfmt-elf-fdpic, not binfmt-flat.
>
> The only architectures I see that enable binfmt-flat are sh, xtensa
> and h8300, but only arch/sh uses CONFIG_BINFMT_SHARED_FLAT

m68k uses enables it too. It is the only binary format supported
when running no-mmu on m68k. (You can use it with MMU enabled too
if you really want too).

The shared flat format has been used on m68k in the past (it was
originally developed on m68k platforms). But I haven't used them
for a long time (probably 10 years at least) on m68k.

Regards
Greg

Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On 5/28/19 12:56 PM, Greg Ungerer wrote:
>> Maybe... but I didn't want to rip it out without having one of the
>> maintainers confirm that this really isn't likely to be used anymore.
>
> I have not used shared libraries on m68k non-mmu setups for
> a very long time. At least 10 years I would think.
We use shared libraries in Debian on m68k and Andreas Schwab uses them
on openSUSE/m68k.

So, they should keep working.

Thanks,
Adrian

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2019-05-29 12:40:30

by Jann Horn

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On Wed, May 29, 2019 at 2:32 PM John Paul Adrian Glaubitz
<[email protected]> wrote:
> On 5/28/19 12:56 PM, Greg Ungerer wrote:
> >> Maybe... but I didn't want to rip it out without having one of the
> >> maintainers confirm that this really isn't likely to be used anymore.
> >
> > I have not used shared libraries on m68k non-mmu setups for
> > a very long time. At least 10 years I would think.
> We use shared libraries in Debian on m68k and Andreas Schwab uses them
> on openSUSE/m68k.

And you're using FLAT shared libraries, not ELF / FDPIC ELF shared
libraries? See <https://lore.kernel.org/lkml/[email protected]/>
for context - this thread is about CONFIG_BINFMT_SHARED_FLAT.

2019-05-29 12:43:33

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work



On 29/5/19 10:32 pm, John Paul Adrian Glaubitz wrote:
> On 5/28/19 12:56 PM, Greg Ungerer wrote:
>>> Maybe... but I didn't want to rip it out without having one of the
>>> maintainers confirm that this really isn't likely to be used anymore.
>>
>> I have not used shared libraries on m68k non-mmu setups for
>> a very long time. At least 10 years I would think.
> We use shared libraries in Debian on m68k and Andreas Schwab uses them
> on openSUSE/m68k.

When used on no-mmu platforms?

Regards
Greg


> So, they should keep working.
>
> Thanks,
> Adrian
>

Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On 5/29/19 2:38 PM, Jann Horn wrote:
>>> I have not used shared libraries on m68k non-mmu setups for
>>> a very long time. At least 10 years I would think.
>> We use shared libraries in Debian on m68k and Andreas Schwab uses them
>> on openSUSE/m68k.
>
> And you're using FLAT shared libraries, not ELF / FDPIC ELF shared
> libraries? See <https://lore.kernel.org/lkml/[email protected]/>
> for context - this thread is about CONFIG_BINFMT_SHARED_FLAT.

No, we're using ELF binaries only:

root@pacman:~# grep CONFIG_BINFMT /boot/config-$(uname -r)
CONFIG_BINFMT_ELF=y
CONFIG_BINFMT_SCRIPT=y
# CONFIG_BINFMT_FLAT is not set
# CONFIG_BINFMT_AOUT is not set
CONFIG_BINFMT_MISC=m
root@pacman:~#

Thanks,
Adrian

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2019-05-29 13:18:39

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On Mai 29 2019, John Paul Adrian Glaubitz <[email protected]> wrote:

> On 5/28/19 12:56 PM, Greg Ungerer wrote:
>>> Maybe... but I didn't want to rip it out without having one of the
>>> maintainers confirm that this really isn't likely to be used anymore.
>>
>> I have not used shared libraries on m68k non-mmu setups for
>> a very long time. At least 10 years I would think.
> We use shared libraries in Debian on m68k and Andreas Schwab uses them
> on openSUSE/m68k.

Nope, I don't use non-mmu.

Andreas.

--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On 5/29/19 3:16 PM, Andreas Schwab wrote:
>>> I have not used shared libraries on m68k non-mmu setups for
>>> a very long time. At least 10 years I would think.
>> We use shared libraries in Debian on m68k and Andreas Schwab uses them
>> on openSUSE/m68k.
>
> Nope, I don't use non-mmu.

Sorry, I missed the "non-mmu" part in Greg's mail :).

Adrian

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2019-05-29 13:44:21

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

On Wed, May 29, 2019 at 2:29 PM Greg Ungerer <[email protected]> wrote:
> On 29/5/19 10:05 pm, Arnd Bergmann wrote:
> > On Tue, May 28, 2019 at 12:56 PM Greg Ungerer <[email protected]> wrote:
> >> On 27/5/19 11:38 pm, Jann Horn wrote:
> >>> On Sat, May 25, 2019 at 11:43 PM Andrew Morton
> >>> <[email protected]> wrote:
> >>> Maybe... but I didn't want to rip it out without having one of the
> >>> maintainers confirm that this really isn't likely to be used anymore.
> >>
> >> I have not used shared libraries on m68k non-mmu setups for
> >> a very long time. At least 10 years I would think.
> >
> > I think Emcraft have a significant customer base running ARM NOMMU
> > Linux, I wonder whether they would have run into this (adding
> > Sergei to Cc).
> > My suspicion is that they use only binfmt-elf-fdpic, not binfmt-flat.
> >
> > The only architectures I see that enable binfmt-flat are sh, xtensa
> > and h8300, but only arch/sh uses CONFIG_BINFMT_SHARED_FLAT
>
> m68k uses enables it too. It is the only binary format supported
> when running no-mmu on m68k. (You can use it with MMU enabled too
> if you really want too).

My mistake, I meant to write 'the only architectures /other than m68k/",
which you had already mentioned above.

Arnd

2019-06-02 08:41:24

by Sergei Poselenov

[permalink] [raw]
Subject: Re: [PATCH] binfmt_flat: make load_flat_shared_library() work

Hello Arnd, all,

On Wed, 2019-05-29 at 14:05 +0200, Arnd Bergmann wrote:
> On Tue, May 28, 2019 at 12:56 PM Greg Ungerer <[email protected]>
> wrote:
> > On 27/5/19 11:38 pm, Jann Horn wrote:
> > > On Sat, May 25, 2019 at 11:43 PM Andrew Morton
> > > <[email protected]> wrote:
> > > > On Fri, 24 May 2019 22:18:17 +0200 Jann Horn <[email protected]>
> > > > wrote:
> > > > > load_flat_shared_library() is broken: It only calls
> > > > > load_flat_file() if
> > > > > prepare_binprm() returns zero, but prepare_binprm() returns
> > > > > the number of
> > > > > bytes read - so this only happens if the file is empty.
> > > >
> > > > ouch.
> > > >
> > > > > Instead, call into load_flat_file() if the number of bytes
> > > > > read is
> > > > > non-negative. (Even if the number of bytes is zero - in that
> > > > > case,
> > > > > load_flat_file() will see nullbytes and return a nice
> > > > > -ENOEXEC.)
> > > > >
> > > > > In addition, remove the code related to bprm creds and stop
> > > > > using
> > > > > prepare_binprm() - this code is loading a library, not a main
> > > > > executable,
> > > > > and it only actually uses the members "buf", "file" and
> > > > > "filename" of the
> > > > > linux_binprm struct. Instead, call kernel_read() directly.
> > > > >
> > > > > Cc: [email protected]
> > > > > Fixes: 287980e49ffc ("remove lots of IS_ERR_VALUE abuses")
> > > > > Signed-off-by: Jann Horn <[email protected]>
> > > > > ---
> > > > > I only found the bug by looking at the code, I have not
> > > > > verified its
> > > > > existence at runtime.
> > > > > Also, this patch is compile-tested only.
> > > > > It would be nice if someone who works with nommu Linux could
> > > > > have a
> > > > > look at this patch.
> > > >
> > > > 287980e49ffc was three years ago! Has it really been broken
> > > > for all
> > > > that time? If so, it seems a good source of freed disk
> > > > space...
> > >
> > > Maybe... but I didn't want to rip it out without having one of
> > > the
> > > maintainers confirm that this really isn't likely to be used
> > > anymore.
> >
> > I have not used shared libraries on m68k non-mmu setups for
> > a very long time. At least 10 years I would think.
>
> I think Emcraft have a significant customer base running ARM NOMMU
> Linux, I wonder whether they would have run into this (adding
> Sergei to Cc).
> My suspicion is that they use only binfmt-elf-fdpic, not binfmt-flat.
>

We use both, acutally, but all-static. We don't support shared
libraries with bFLT or ELF FDPIC.

Kind regards,
Sergei
> The only architectures I see that enable binfmt-flat are sh, xtensa
> and h8300, but only arch/sh uses CONFIG_BINFMT_SHARED_FLAT
> for a few machine specific configurations, and I'm in turn fairly
> sure
> those machines have not run a recent kernel in many years.
>
> The one SH nommu platform likely to have users is j2, and that is
> probably always used with musl-libc with elf-fdpic (given that
> Rich Felker maintains both the kernel port and the library).
>
> Arnd
>