mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] [PATCH] ldso: add basic ifunc support
@ 2022-08-23 18:41 Ben Noordhuis
  2022-08-23 20:03 ` Szabolcs Nagy
  0 siblings, 1 reply; 3+ messages in thread
From: Ben Noordhuis @ 2022-08-23 18:41 UTC (permalink / raw)
  To: musl; +Cc: Ben Noordhuis

Not perfect yet because it doesn't handle resolver functions that need
relocations themselves but basic functionality works. That is, this
works:

    typedef void (*Func)(void);
    void f(void) __attribute__((ifunc("g")));
    Func g(void) { return h; }
    void h(void) { abort(); }

But returning an extern function does not:

    typedef void (*Func)(void);
    void f(void) __attribute__((ifunc("g")));
    Func g(void) { return abort; } // segfaults
---
 arch/x86_64/reloc.h    | 1 +
 ldso/dynlink.c         | 4 ++++
 src/internal/dynlink.h | 1 +
 3 files changed, 6 insertions(+)

diff --git a/arch/x86_64/reloc.h b/arch/x86_64/reloc.h
index fac0c0ae..fb6be3ea 100644
--- a/arch/x86_64/reloc.h
+++ b/arch/x86_64/reloc.h
@@ -10,6 +10,7 @@
 #define REL_DTPOFF      R_X86_64_DTPOFF64
 #define REL_TPOFF       R_X86_64_TPOFF64
 #define REL_TLSDESC     R_X86_64_TLSDESC
+#define REL_IRELATIVE   R_X86_64_IRELATIVE
 
 #define CRTJMP(pc,sp) __asm__ __volatile__( \
 	"mov %1,%%rsp ; jmp *%0" : : "r"(pc), "r"(sp) : "memory" )
diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index fd09ca69..659cb113 100644
--- a/ldso/dynlink.c
+++ b/ldso/dynlink.c
@@ -507,6 +507,10 @@ static void do_relocs(struct dso *dso, size_t *rel, size_t rel_size, size_t stri
 			reloc_addr[1] = tmp;
 #endif
 			break;
+		case REL_IRELATIVE:
+			*reloc_addr = (size_t)base + addend;
+			*reloc_addr = ((size_t(*)(void))*reloc_addr)();
+			break;
 		default:
 			error("Error relocating %s: unsupported relocation type %d",
 				dso->name, type);
diff --git a/src/internal/dynlink.h b/src/internal/dynlink.h
index 830354eb..c67bc35e 100644
--- a/src/internal/dynlink.h
+++ b/src/internal/dynlink.h
@@ -41,6 +41,7 @@ enum {
 	REL_TPOFF,
 	REL_TPOFF_NEG,
 	REL_TLSDESC,
+	REL_IRELATIVE,
 	REL_FUNCDESC,
 	REL_FUNCDESC_VAL,
 };
-- 
2.34.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] [PATCH] ldso: add basic ifunc support
  2022-08-23 18:41 [musl] [PATCH] ldso: add basic ifunc support Ben Noordhuis
@ 2022-08-23 20:03 ` Szabolcs Nagy
  2022-08-23 20:29   ` Rich Felker
  0 siblings, 1 reply; 3+ messages in thread
From: Szabolcs Nagy @ 2022-08-23 20:03 UTC (permalink / raw)
  To: Ben Noordhuis; +Cc: musl

* Ben Noordhuis <info@bnoordhuis.nl> [2022-08-23 20:41:57 +0200]:
> Not perfect yet because it doesn't handle resolver functions that need
> relocations themselves but basic functionality works. That is, this
> works:
> 
>     typedef void (*Func)(void);
>     void f(void) __attribute__((ifunc("g")));
>     Func g(void) { return h; }
>     void h(void) { abort(); }
> 
> But returning an extern function does not:
> 
>     typedef void (*Func)(void);
>     void f(void) __attribute__((ifunc("g")));
>     Func g(void) { return abort; } // segfaults

there are a lot more problems than that.

the patch only works if f is hidden or binds locally in some other
way (e.g. in an executable) otherwise references to f will generate
symbolic relocations not IRELATIVE.

resolving symbolic relocation for ifunc symbols requires the ldso
to look at the symbol type (STT_GNU_IFUNC) and do the indirect call
based on that.

the main issue is that code is executed before the module is fully
relocated (in fact it may execute code in other libs before those are
relocated because symbol lookup can process not yet relcated libs)
which is problematic if the resolver function itself depends on
relocations (e.g. return h only works if constructing the address
of h does not require dynamic relocs or those relocs happen to get
resolved before the IRELATIVE, glibc tries a bit to shuffle ifunc
relocs to the end of reloc processing, it was never fixed completely
iirc because copy relocs and circular deps made things complicated).

and of course static linking is not addressed (where there are
further ordering issues since it is target dependent if ifunc has
to be resolved before or after TLS is set up).

there are further minor issues like the prototype of the resolver
is target dependent (usually hwcap is passed as argument) so the same
c code wont work across targets. or that ldd mode is not expected to
execute any code in the application but it is supposed to do reloc
processing (some might say this is a security issue).


> ---
>  arch/x86_64/reloc.h    | 1 +
>  ldso/dynlink.c         | 4 ++++
>  src/internal/dynlink.h | 1 +
>  3 files changed, 6 insertions(+)
> 
> diff --git a/arch/x86_64/reloc.h b/arch/x86_64/reloc.h
> index fac0c0ae..fb6be3ea 100644
> --- a/arch/x86_64/reloc.h
> +++ b/arch/x86_64/reloc.h
> @@ -10,6 +10,7 @@
>  #define REL_DTPOFF      R_X86_64_DTPOFF64
>  #define REL_TPOFF       R_X86_64_TPOFF64
>  #define REL_TLSDESC     R_X86_64_TLSDESC
> +#define REL_IRELATIVE   R_X86_64_IRELATIVE
>  
>  #define CRTJMP(pc,sp) __asm__ __volatile__( \
>  	"mov %1,%%rsp ; jmp *%0" : : "r"(pc), "r"(sp) : "memory" )
> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> index fd09ca69..659cb113 100644
> --- a/ldso/dynlink.c
> +++ b/ldso/dynlink.c
> @@ -507,6 +507,10 @@ static void do_relocs(struct dso *dso, size_t *rel, size_t rel_size, size_t stri
>  			reloc_addr[1] = tmp;
>  #endif
>  			break;
> +		case REL_IRELATIVE:
> +			*reloc_addr = (size_t)base + addend;
> +			*reloc_addr = ((size_t(*)(void))*reloc_addr)();
> +			break;
>  		default:
>  			error("Error relocating %s: unsupported relocation type %d",
>  				dso->name, type);
> diff --git a/src/internal/dynlink.h b/src/internal/dynlink.h
> index 830354eb..c67bc35e 100644
> --- a/src/internal/dynlink.h
> +++ b/src/internal/dynlink.h
> @@ -41,6 +41,7 @@ enum {
>  	REL_TPOFF,
>  	REL_TPOFF_NEG,
>  	REL_TLSDESC,
> +	REL_IRELATIVE,
>  	REL_FUNCDESC,
>  	REL_FUNCDESC_VAL,
>  };
> -- 
> 2.34.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] [PATCH] ldso: add basic ifunc support
  2022-08-23 20:03 ` Szabolcs Nagy
@ 2022-08-23 20:29   ` Rich Felker
  0 siblings, 0 replies; 3+ messages in thread
From: Rich Felker @ 2022-08-23 20:29 UTC (permalink / raw)
  To: Ben Noordhuis, musl

On Tue, Aug 23, 2022 at 10:03:32PM +0200, Szabolcs Nagy wrote:
> * Ben Noordhuis <info@bnoordhuis.nl> [2022-08-23 20:41:57 +0200]:
> > Not perfect yet because it doesn't handle resolver functions that need
> > relocations themselves but basic functionality works. That is, this
> > works:
> > 
> >     typedef void (*Func)(void);
> >     void f(void) __attribute__((ifunc("g")));
> >     Func g(void) { return h; }
> >     void h(void) { abort(); }
> > 
> > But returning an extern function does not:
> > 
> >     typedef void (*Func)(void);
> >     void f(void) __attribute__((ifunc("g")));
> >     Func g(void) { return abort; } // segfaults
> 
> there are a lot more problems than that.
> 
> the patch only works if f is hidden or binds locally in some other
> way (e.g. in an executable) otherwise references to f will generate
> symbolic relocations not IRELATIVE.
> 
> resolving symbolic relocation for ifunc symbols requires the ldso
> to look at the symbol type (STT_GNU_IFUNC) and do the indirect call
> based on that.
> 
> the main issue is that code is executed before the module is fully
> relocated (in fact it may execute code in other libs before those are
> relocated because symbol lookup can process not yet relcated libs)
> which is problematic if the resolver function itself depends on
> relocations (e.g. return h only works if constructing the address
> of h does not require dynamic relocs or those relocs happen to get
> resolved before the IRELATIVE, glibc tries a bit to shuffle ifunc
> relocs to the end of reloc processing, it was never fixed completely
> iirc because copy relocs and circular deps made things complicated).
> 
> and of course static linking is not addressed (where there are
> further ordering issues since it is target dependent if ifunc has
> to be resolved before or after TLS is set up).

Yes, there are multiple reasons ifunc is not just a missing feature
but rejected as an anti-feature. It has issues with the order
relocations take place in and exposes that as a contract with the
application, especially if (like musl) you do not do lazy binding,
which slightly relaxes the ordering requirement. It does not have a
well-defined or definable contract for what you can actually do from
the resovler functions, isn't compatible with static linking, etc.
Everything that can be done badly with ifunc can be done just as well,
fully portably, with function pointers initialized to a generic
version of the function replaced by ctors, or to a self-initializing
version of the function that replaces itself on first call (modulo
some technicalities about how you do the atomics, but this is
basically solvable with relaxed-mode atomics or call_once). In a
normal dynamic-linked context, this is just as fast as an ifunc call
too -- you just load the function pointer from a PC-relative address
outside the GOT rather than a PC-relative address in the GOT (and
you get to bypass the PLT thunk too).

Rich

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-08-23 20:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-23 18:41 [musl] [PATCH] ldso: add basic ifunc support Ben Noordhuis
2022-08-23 20:03 ` Szabolcs Nagy
2022-08-23 20:29   ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).