Re: What does LAV_CURRENT mean backwards compatibility of LD_AUDIT interface?

[Ben Woodard <woodard at redhat dot com>]

> On Feb 26, 2015, at 4:38 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> 
> At present glibc defines LAV_CURRENT as 1.
> 
> Should we wish to add new features to LD_AUDIT, what do we
> do with this number and what does it mean?

I think that the man page makes this pretty clear when it says:

       As its function result, this function should return the version of
       the auditing interface that this auditing library expects to use
       (returning 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.

So I believe that this documentation indicates that N+1 was intended to backward compatible. An audit library written for an old version of the audit interface simply returns the version of the audit interface that it was designed to use. In the case of sotruss-lib.c the code could be:

/* Audit interface verification.  We also initialize everything if
 everything checks out OK.  */
unsigned int
la_version (unsigned int v)
{
if (v >LAV_CURRENT){
  warn (1, 0, ârecompile for newer audit interface version %u", v);
 }

init ();

/* we donât make use of new rtdl-audit interface version 2 features */
return 1; 
}

If we introduce an interface that is not backward compatible (highly unlikely), then when an audit library's la_version() returns a lower incompatible version number we can either:
a) emulate the older versionâs behavior
b) give the user some sort of error saying that version is not supported.

> 
> I see two solutions:
> 
> (a) Version N+1 is backwards compatible.
> 
> In this scenario we are constrained by previous implementations
> and are forced to make only backwards compatible changes.
> 
> This means that for an application to see a version in la_version
> greater than the version it expects is OK, since everything is
> backwards compatible.
> 
> Code like we have in elf/sotruss-lib.c would then be a bad exmaple
> of how to do things. e.g.
> 
> 121 /* Audit interface verification.  We also initialize everything if
> 122    everything checks out OK.  */
> 123 unsigned int
> 124 la_version (unsigned int v)
> 125 {
> 126   if (v != LAV_CURRENT)
> 127     error (1, 0, "cannot handle interface version %u", v);
> 128 
> 129   init ();
> 130 
> 131   return v;
> 132 }
> 
> Should instead check for `v < LAV_CURRENT` before issuing an error.
> I know that we do this today because sotruss-lib should match the
> library, but users might cut-and-paste some of this code and it should
> showcase best practice or have a comment that talks about it.
> 
> (b) Version N+1 is not backwards compatible.
> 
> In this scenario we are unconstrained by the previous version, and
> modules must look for *exact* version matches when handshaking with
> the dynamic loader or risk undefined behaviour.
> 
> This would make it possible for the implementation to change flexibly,
> but would not allow users to easily write audit modules that work for
> multiple different loaders.
> 
> It should be possible though to have the dynamic loader use versioning
> in the user audit modules to call la_version@GLIBC_AUDIT_$VERSION
> and thus allow users to write single audit modules that could handle
> multiple different versions of the audit interface. Alternatively you
> just change all the function names.
> 
> If we still wanted backwards compatibility in this scheme we could
> have the loader check for the various supported versions of the interface
> in the module, adjust behaviours, and call the newest routines it has
> support for. This would still constrain the implementation, but would
> be more flexible than completely backwards compatible as in (a).
> 
> Thoughts and comments?
> 
> Given that this was designed to be roughly compatible with the Solaris
> implementation of audit modules, I'm inclined to make glibc follow (a)
> and avoid the complexity of (b). Then revisit a wholly new API if
> we feel an incompatible change is required.
> 
> Cheers,
> Carlos.