[PATCH] rtld-audit.7: Clarify la_version handshake

[Ben Coyote Woodard]

I think that you are correctly characterizing the original intent of the 
solaris developers.

However, I believe that their fundamental design was a poor design. 
Doing this kind of magic number handshake and then imbuing a number with 
some deeper semantic meaning that exists outside of the interface itself 
is a bad design and prone to errors over the long run.

Then there is the problem that you have a pair of special functions for 
each architecture la_pltenter() and la_pltexit() which are dependent on 
the size and layout of structures in that are implied by the same 
interface number.

This is what I would do:

1) require that audit libs be compiled with -g. "No DWARF no worky"
2) Iterate through all the la_* functions in the audit library and 
compare the DWARF for their function declarations to the DWARF from the 
function prototypes that defined the calls that you make in the runtime 
linker. This would also mean that the types types for the parameters to 
these functions would be checked. That way you would be able to detect 
if something like La_*_regs changed which is the problem that you have 
with ABI variations.
3) Because you are checking the parameter types for the la_* functions, 
the types for the preserved registers and the return values could vary 
across architectures with no conflict. Just make the structures La_regs 
and La_retval.
   a) or backward compatibility typedef La_<arch>_regs and 
La_<arch>_retval if you like.
4) The fact that you have the same named parameter types for all the 
architectures allows you to get rid of the architecturally specific 
versions of la_pltenter and la_pltexit.
5) Switch to C++ name mangling then and then the differences between the 
32b and 64b versions of the interface can be implemented with a template 
instantiation.
6) Changing to C++ name mangled interfaces would also allow us to deal 
with things like SVE or ABI changes more easily. For example:
    For ARM there would be two overloaded la_pltenter() and la_pltexit() 
calls. One would be:

   la_pltenter (ElfW(Sym) *__sym, unsigned int __ndx,
              uintptr_t *__refcook,
              uintptr_t *__defcook,
              La_regs *__regs,
              unsigned int *__flags,
              const char *__symname,
              long int *__framesizep);

and the other would be:

   la_pltenter (ElfW(Sym) *__sym, unsigned int __ndx,
              uintptr_t *__refcook,
              uintptr_t *__defcook,
              La_sve_regs *__regs, // <- different type here
              unsigned int *__flags,
              const char *__symname,
              long int *__framesizep);

Then the handler for STO_AARCH64_VARIANT_PCS could be wired up to call 
the SVE version of la_pltenter and la_pltexit. The same sort of trick 
could be used for architecture ABI breaks. Having the handler key off of 
the ELF ABI version.

The overall point is we can do so much better now. Pedantically, 
adhering to a crufty 40 year old interface what was not well thought 
through to begin with and which hasn't been refined because so few 
people use it, is really not a good way to ensure that GNU/Linux 
continues to be viable into the future.

-ben


On 9/23/20 4:38 AM, Florian Weimer via Libc-alpha wrote:
> Returning its argument without further checks is almost always
> wrong for la_version.
>
> Signed-off-by: Florian Weimer <fweimer@redhat.com>
>
> ---
>   man7/rtld-audit.7 | 31 ++++++++++++++++++++++---------
>   1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/man7/rtld-audit.7 b/man7/rtld-audit.7
> index b1b7dfebc..ca8afa752 100644
> --- a/man7/rtld-audit.7
> +++ b/man7/rtld-audit.7
> @@ -70,17 +70,30 @@ the auditing library.
>   When invoking this function, the dynamic linker passes, in
>   .IR version ,
>   the highest version of the auditing interface that the linker supports.
> -If necessary, the auditing library can check that this version
> -is sufficient for its requirements.
>   .PP
> -As its function result,
> -this function should return the version of the auditing interface
> -that this auditing library expects to use (returning
> +A typical implementation of this function simply returns the constant
> +.BR LAV_CURRENT ,
> +which indicates the version of
> +.I <link.h>
> +that was used to build the audit module.  If the dynamic linker does
> +not support this version of the audit interface, it will refuse to
> +activate this audit module.  If the function returns zero, the dynamic
> +linker also does not activate this audit module.
> +.PP
> +In order to enable backwards compatibility with older dynamic linkers,
> +an audit module can examine the
> +.I version
> +argument and return an earlier version than
> +.BR LAV_CURRENT ,
> +assuming the module can adjust its implement to match the requirements
> +of the previous version of the audit interface.  The
> +.B la_version
> +function should not return the value of
>   .I version
> -is acceptable).
> -If the returned value is 0,
> -or a version that is greater than that supported by the dynamic linker,
> -then the audit library is ignored.
> +without further checks because it could correspond to an interface
> +that does not match the
> +.I <link.h>
> +definitions used to build the audit module.
>   .SS la_objsearch()
>   \&
>   .nf
>