Address of nested function crashes with mold

[marxin]

The following crashes:

$ cat nested.c
void callme (void (*callback) (void));

int
main (void)
{
  int ok = 0;
  void callback (void) { ok = 1; }

  callme (&callback);

  if (!ok)
   __builtin_abort ();
  return 0;
}

__attribute__((noinline, noclone))
void
callme (void (*callback) (void))
{
  (*callback) ();
}
$ gcc nested.c && ./a.out
Segmentation fault (core dumped)

[rui314]

GCC's nested function is implemented by dynamically constructing stub code in the stack area to jump from the stack to an actual function. So the stack area must be executable to use it, which is very rare nowadays for the obvious security reason.

mold unconditionally makes the stack area non-executable. You need to pass -z execstack to mold to make it executable.

GNU linker makes the stack area executable if at least one input object file has a section .note.GNU-stack with SHF_EXECINSTR. GCC emits such section if the code uses nested functions.

To me, the GNU linker's behavior seems dangerous. If one person accidentally uses a nested function at somewhere in the entire codebase, or if you statically-link a library that uses a nested function, it makes your program's entire stack area executable without any notice. That would be a big surprise to users, and it feels like that's planting a time bomb waiting to explode.

I think this feature should be re-implemented with an assumption that the stack area is not executable, or if you can't, it should be deprecated because it's dangerous to use.

[marxin]

Well, I think various projects are leaving usage of nested functions and Clang does not support them if I'm correct. However, GCC still emits them and e.g. Fortran code contains quite many of them.

I respect your design decision, but on the other hand, you claim mold is a GNU-compatible linker. So, would it be possible to add a build option that would allow GNU behavior w/o need to pass -z execstack?

[rui314]

Currently, there's no compile-time options to customize mold's run-time behavior, and that helps me a lot to support users. As long as mold binaries are built at a specific git commit, they are guaranteed to behave the same (modulo a possible compiler bug). In addition to that, I don't want to make -z execstack implicit for the reason as explained above. So, I don't think that that's a right decision to make it configurable.

I committed d0e4eee so that users can change their code for mold. Isn't this enough?

This is security-related. It's hard to make a decision to make it vulnerable to a simple buffer overflow attack.

[marxin]

Well, I'm speaking from a distribution perspective where it would be unpleasant to use -Wl,-z,execstack for C{,}FLAGS.
For executable stack we have a rpmlint check that reports usage of the feature for individual packages:
https://rpmlint.opensuse.org/openSUSE:Factory/x86_64/standard?rule=executable-stack

I committed d0e4eee so that users can change their code for mold. Isn't this enough?

As explained, I don't like the divergence from GNU-compatible linkers, which is something you claim:
mold 1.1.1 (compatible with GNU ld).

If you don't agree with the selected approach how nested functions are supported, can you please open a bug report against BFD, GOLD, and maybe LLD?

[rui314]

Perhaps not surprisingly, lld already behaves the same way as mold. Let me file a bug against GNU linker, but in reality, I think what we can request is to not change the default behavior but instead just print out a warning message.

Note that even though we are trying to make mold compatible with GNU linkers, we still sometimes make a choice not to make it compatible. For example, -m option cannot be omitted for mold but can for GNU ld. GNU gold is not fully compatible with GNU ld too.

[marxin]

All right. I accept the approach, but please give me an option on how to change the behavior.
And the easiest one is a configure option, you can then have it part of -v so that you can judge based on configure options:

gcc -v
...
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++,objc,fortran,obj-c++,ada,go,d,jit --enable-offload-targets=nvptx-none,amdgcn-amdhsa, --without-cuda-driver --enable-host-shared --enable-checking=release --disable-werror --with-gxx-include-dir=/usr/include/c++/11 --enable-ssp --disable-libssp --disable-libvtv --enable-cet=auto --disable-libcc1 --enable-plugin --with-bugurl=https://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --with-slibdir=/lib64 --with-system-zlib --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-libphobos --enable-version-specific-runtime-libs --with-gcc-major-version-only --enable-linker-build-id --enable-linux-futex --enable-gnu-indirect-function --program-suffix=-11 --without-system-libunwind --enable-multilib --with-arch-32=x86-64 --with-tune=generic --with-build-config=bootstrap-lto-lean --enable-link-mutex --build=x86_64-suse-linux --host=x86_64-suse-linux
...

[rui314]

That's the build-time option, and for the sake of build reproducibility, I don't want to have it. I'm sorry, but the policy I want to keep is that as long as you pass the exact same command line options and input object files, mold of the same version behaves exactly the same regardless of the host environment. This is a very useful property.

[rui314]

Filed an issue to GNU ld: https://sourceware.org/bugzilla/show_bug.cgi?id=29072

[marxin]

I've noticed that I would need something like mold execstack-if-needed option because otherwise making -Wl,-z,execstack would result in everything having stack executable. That's definitely undesirable.

[marxin]

Filed an issue to GNU ld: https://sourceware.org/bugzilla/show_bug.cgi?id=29072

Thanks for it, let's see what can be done for it.

[rui314]

I've noticed that I would need something like mold execstack-if-needed option because otherwise making -Wl,-z,execstack would result in everything having stack executable. That's definitely undesirable.

I implemented -z execstack-if-needed option in the above commit. Your input is very valuable. Thank you for your suggestion!

[marxin]

Great, works for me. Appreciate we found a solution that works for both of us. Kuddos!