Re: [PATCH v5 02/13] x86/um: nommu: elf loader for fdpic

["Eric W. Biederman" <ebiederm@xmission.com>]

Hajime Tazaki <thehajime@gmail.com> writes:

> On Sat, 14 Dec 2024 05:01:58 +0900,
> Eric W. Biederman wrote:
>
>> >> Last time I looked the regular binfmt_elf works just fine
>> >> without an mmu.  I looked again and at a quick skim the
>> >> regular elf loader still looks like it will work without
>> >> an MMU.
>> >
>> > I'm wondering how you looked at it and how you see that it works
>> > without MMU.
>> 
>> I got as far as seeing that vm_mmap should work.  As all of the
>> bits for mmap to work, are present in both mmu and nommu.
>
> hmm, at least MAP_FIXED doesn't work in current mm/nommu.c.
> # also documented at Documentation/admin-guide/mm/nommu-mmap.rst.

Yes, and that fundamentally makes sense.

>> > I also wish to use the regular binfmt_elf, but it doesn't allow me to
>> > compile with !CONFIG_MMU right now.
>> 
>> Then I may simply be confused.  Where does the compile fail?
>> Is it somewhere in Kconfig?
>> 
>> I could be completely confused.  It has happened before.
>
> If I applied to below in addition to my whole patchset,
>
> diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
> index 419ba0282806..b34d0578a22f 100644
> --- a/fs/Kconfig.binfmt
> +++ b/fs/Kconfig.binfmt
> @@ -4,7 +4,6 @@ menu "Executable file formats"
>  
>  config BINFMT_ELF
>         bool "Kernel support for ELF binaries"
> -       depends on MMU
>         select ELFCORE
>         default y
>         help
> @@ -58,7 +57,7 @@ config ARCH_USE_GNU_PROPERTY
>  config BINFMT_ELF_FDPIC
>         bool "Kernel support for FDPIC ELF binaries"
>         default y if !BINFMT_ELF
> -       depends on ARM || ((M68K || RISCV || SUPERH || UML || XTENSA) && !MMU)
> +       depends on ARM || ((M68K || RISCV || SUPERH || XTENSA) && !MMU)
>         select ELFCORE
>         help
>           ELF FDPIC binaries are based on ELF, but allow the individual load

You have my apologies I was most definitely confused.  BINFMT_ELF
currently does not work without an MMU.

> this is the output from `make ARCH=um`.
>
>   GEN     Makefile
>   CALL    ../scripts/checksyscalls.sh
>   CC      fs/binfmt_elf.o
> In file included from ./arch/x86/include/generated/asm/rwonce.h:1,
>                  from ../include/linux/compiler.h:317,
>                  from ../include/linux/build_bug.h:5,
>                  from ../include/linux/container_of.h:5,
>                  from ../include/linux/list.h:5,
>                  from ../include/linux/module.h:12,
>                  from ../fs/binfmt_elf.c:13:
> ../fs/binfmt_elf.c: In function ‘load_elf_binary’:
> ../include/asm-generic/rwonce.h:44:71: error: lvalue required as unary ‘&’ operand
>    44 | #define __READ_ONCE(x)  (*(const volatile __unqual_scalar_typeof(x) *)&(x))
>       |                                                                       ^
> ../include/asm-generic/rwonce.h:50:9: note: in expansion of macro ‘__READ_ONCE’
>    50 |         __READ_ONCE(x);                                                 \
>       |         ^~~~~~~~~~~
> ../fs/binfmt_elf.c:1006:49: note: in expansion of macro ‘READ_ONCE’
>  1006 |         const int snapshot_randomize_va_space = READ_ONCE(randomize_va_space);
>       |   
>
> I avoided this issue (with nasty MAP_FIXED workaround) but there seems
> to be still a lot of things that I need to fix to work with nommu.

Yes, at a minimum all of the MAP_FIXED code would need to be
conditionalized on having an MMU.
>
>> I just react a little strongly to the assertion that elf_fdpic is
>> the only path when I don't see why that should be.
>> 
>> Especially for an architecture like user-mode-linux where I would expect
>> it to run the existing binaries for a port.
>
> I understand your concern, and will try to work on improving this
> situation a bit.
>
> Another naive question: are there any past attempts to do the similar
> thing (binfmt_elf without MMU) ?

At this point what I would recommend is:

Merge your original patch.  Get nommu UML working with binfmt_elf_fdpic.c.
I think it is a proper superset of ELF functionality.

Then I would make it a long term goal to see about removing redundancy
between binfmt_elf.c and binfmt_elf_fdpic.c with a view to merging them
in the long term.

There is a lot of mostly duplicate code between the two and
binfmt_elf_fdpic.c does not get half the attention and use binfmt_elf.c
gets.

Eric