Re: Annotate Deprecated Functions in libselinux

From: Ondrej Mosnacek <omosnace@redhat.com>
To: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: William Roberts <bill.c.roberts@gmail.com>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	Petr Lautrbach <plautrba@redhat.com>,
	SElinux list <selinux@vger.kernel.org>,
	Ulrich Drepper <drepper@redhat.com>
Subject: Re: Annotate Deprecated Functions in libselinux
Date: Thu, 27 Feb 2020 21:03:53 +0100	[thread overview]
Message-ID: <CAFqZXNtRmp-TiNupX3xgOWiBYun4gK0E3TJcoo4-_b2RF2+Duw@mail.gmail.com> (raw)
In-Reply-To: <CAEjxPJ4wZw=g1QW9gSPL_2tu9E12oJnX2OYPmTKCZqZQH6StKw@mail.gmail.com>

(Adding Ulrich himself to Cc...)

On Thu, Feb 27, 2020 at 8:47 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Thu, Feb 27, 2020 at 1:41 PM William Roberts
> <bill.c.roberts@gmail.com> wrote:
> > It doesn't get us around all the issues, anything marked deprecated in selinux.h
> > and marked hidden_proto in selinux_internal.h, will still need a pragma.
> >
> > This would be much simpler if we drop the whole hidden_proto stuff, I guess
> > kloczek is proposing patches? Who is that, I see the Github name as:
> > Tomasz Kłoczko
> >
> > But I can't find any matching author in the git logs to CC them.
>
> I don't actually know him myself but he has opened a number of issues
> on the selinux userspace and appears to be involved in Fedora in some
> capacity.
>
> > I'm looking at those macros:
> > hidden_def
> > hidden_proto
> >
> > They both seem to take a function and create an _internal symbol, and
> > set the global symbol equal to the internal one. Essentially, both the
> > normal and _internal functions are pointing to the same address.
> >
> > $ readelf -s ./src/libselinux.so | grep setexeccon_raw
> >    176: 0000000000017c26    63 FUNC    GLOBAL DEFAULT   12 setexeccon_raw
> >    279: 0000000000017c26    63 FUNC    LOCAL  DEFAULT   12
> > setexeccon_raw_internal
> >    606: 0000000000017c26    63 FUNC    GLOBAL DEFAULT   12 setexeccon_raw
> >
> > The hidden one, additionally
> > sets the visibility to hidden, which if IIUC, just requires one to
> > statically link. We can see above the
> > visibility is local.
> >
> > What I don't understand, is why one would do this? What actual
> > performance enhancement do we get?
>
> The hidden_def/hidden_proto stuff originally came from Ulrich Drepper,
> glibc maintainer at the time, to eliminate unnecessary runtime
> relocations and PLT entries being used for local symbols.  Per the
> comments in https://github.com/SELinuxProject/selinux/issues/204,
> these might not be needed anymore and are breaking building with LTO.
> Willing to get rid of them if it doesn't produce a significant
> regression.

Ulrich, could you help us understand the macros you proposed to add to
the SELinux libraries (probably a very long time ago)? Specifically,
we are talking about those defined in "dso.h" header files such as
this one [1]. See also GH issue 204 [2] for related discussion.

Thanks,

[1] https://github.com/SELinuxProject/selinux/blob/master/libselinux/src/dso.h
[2] https://github.com/SELinuxProject/selinux/issues/204

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.