From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id AE0D3394C01A for ; Wed, 23 Sep 2020 20:16:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AE0D3394C01A Received: from mail-oo1-f71.google.com (mail-oo1-f71.google.com [209.85.161.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-112-D7L6dFrXN4iw7fPIqrLUjA-1; Wed, 23 Sep 2020 16:15:59 -0400 X-MC-Unique: D7L6dFrXN4iw7fPIqrLUjA-1 Received: by mail-oo1-f71.google.com with SMTP id a21so336644oos.5 for ; Wed, 23 Sep 2020 13:15:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=CpaJVw1DEcKyqGr5JVZgxEpC7YXuUUG14sIYCNWWcLc=; b=cjdKOZC+vyv0NuLw9Dy+JhvMMsvqkyTe9PEHdKFNzrHShNnlcYWR/BPZQlAgBFd36W bVDYIVhEOh3ny+4jHUZdPB5o2s0YdJq7PORXbwwroRT0qow+bFGaW3lH7QWi2eolDDea WQdzop5iRmt7OVMA7H25lnAwAcLP/vLprwElSF5rkckYPxaPltXBAKii4k4ipLBXDQEi 6aCvIRfmxKuyboYHQQ4Qs3dUWOU9/pTy4/7aM2OYaGB7PX6b8uqHtpKSZQNrB6I4j7f/ DMX45zJry4WmTfnYveYISQVbR8aCJxwCpUMQgDKXFGyjMWyk6oHW2sqzpmEzAVINtCDN n+sw== X-Gm-Message-State: AOAM531CrcbcdVX3mnLW+JptaHyECsn+HMAEW+dGIsvht8Y9IaXjok7c nXQwwAiBCKnBSKsBbuBM/W1fa+sBSSvolgZUev21G4RrCbS0apRrYPohN3/qmznjuyViOppQN47 CfuO3rnoXt7+LCI3s/Hl7 X-Received: by 2002:aca:c144:: with SMTP id r65mr716852oif.166.1600892158504; Wed, 23 Sep 2020 13:15:58 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxlIlAvBbtS0MlsTJD/SfSL9fIdnFE4d7vf3hkvczuQuJ5fZ5nCvofS67sBf65EdhVpuB36lQ== X-Received: by 2002:aca:c144:: with SMTP id r65mr716832oif.166.1600892157997; Wed, 23 Sep 2020 13:15:57 -0700 (PDT) Received: from [192.168.1.234] (47-208-193-143.trckcmtc01.res.dyn.suddenlink.net. [47.208.193.143]) by smtp.gmail.com with ESMTPSA id t7sm234061ooq.0.2020.09.23.13.15.57 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 23 Sep 2020 13:15:57 -0700 (PDT) Subject: Re: [PATCH] rtld-audit.7: Clarify la_version handshake To: libc-alpha@sourceware.org References: <874knosoyq.fsf@oldenburg2.str.redhat.com> From: Ben Coyote Woodard Message-ID: Date: Wed, 23 Sep 2020 13:15:55 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <874knosoyq.fsf@oldenburg2.str.redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Sep 2020 20:16:04 -0000 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__regs and La__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 > > --- > 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 > +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 > +definitions used to build the audit module. > .SS la_objsearch() > \& > .nf >