Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version

[Petr Lautrbach <plautrba@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4196 bytes --]

Bcc: 
Subject: Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version
Reply-To: 
In-Reply-To: <CAJfZ7=m5bwek=R_6gObMGdAKXNYSUwZU-rWv7EPAhw8SQU_vfg@mail.gmail.com>

On Fri, Oct 02, 2020 at 05:41:53PM +0200, Nicolas Iooss wrote:
> On Fri, Oct 2, 2020 at 4:50 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Fri, Oct 2, 2020 at 2:53 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> > >
> > > Hello,
> > > I have another question: why is bumping the .so version needed? As we
> > > are not changing the ABI of a "linked symbol" (thanks to using
> > > versioned symbols, with .map files), if we do not bump the .so
> > > version, programs that were built with libsepol.so from years ago will
> > > just stop working due to missing symbols, with an error message which
> > > will be quite clear (and this only if they were using deprecated
> > > symbols). In my humble opinion, bumping the .so version is most
> > > required when the calling convention of a non-versioned symbol
> > > changes, but this leads to unexpected execution paths.
> > >
> > > Nevertheless I did not spend time to search for a document that would
> > > explain why bumping the .so version would be recommended when removing
> > > symbols. If you know one, could you please add a reference to it in
> > > the commit description ("Following guidelines from https://...) and/or
> > > to some documentation?
> > >
> > > >From a "distro maintainer point of view" (for Arch Linux), having just
> > > spent a considerable amount of time due to breaking changes in the
> > > last release of PAM, I am not eager to spend time dealing with finding
> > > clever ways to smoothly upgrade the system if there is no
> > > easy&straightforward way to do this. Introducing a transition package
> > > for each library which is bumped is acceptable to me, but if the
> > > release after the next one bumps the version again, introducing
> > > another set of transition packages will begin to be quite painful. In
> > > short: I agree to remove the deprecated functions in order to "bump
> > > the .so version only once", as suggested.
> >
> > My original understanding of library ABI compat requirements came from
> > Ulrich Drepper's paper,
> > https://www.akkadia.org/drepper/dsohowto.pdf
> >
> > Looks like Debian's policy is here:
> > https://www.debian.org/doc/debian-policy/ch-sharedlibs.html#run-time-shared-libraries
> >
> > IIUC, if we follow the rules laid out by both, removing an old symbol
> > version entirely is incompatible and requires a SONAME change.  That
> > said, commit c3f9492d7ff05bdc8581817655ad05bc1e1174b8 ("selinux:
> > Remove legacy local boolean and user code") was technically an
> > incompatible change; it left the symbols in place but made them always
> > fail or ignore no-longer-used parameters, which isn't truly
> > compatible, and we didn't change the SONAMEs then.
> >
> > I'd personally be ok with not changing the SONAME as long as these
> > interfaces were only ever used by the selinux userspace code itself or
> > only by really ancient code that is no longer in use by any supported
> > distribution but I don't think that flies with e.g. the Debian policy.
> 
> Thanks for the details. Debian policy makes sense and I agree with
> bumping the version in the SONAME.
> 
> Nicolas
> 

Thanks, I'll prepare another patchset with improved commit messages.

In the mean time I'm looking into removing deprecated symbols from libsepol as it's
supposed to be required only by selinux components. So far I've found that
deprecated sepol_check_context() is used in chkcon utility. As Fedora doesn't ship
this tool I incline to remove it from libsepol as well.

And there's also

/* Deprecated */
struct sepol_handle sepol_compat_handle = {
	.msg_callback = sepol_msg_default_handler,
	.msg_callback_arg = NULL,
};

void sepol_debug(int on)
{
	sepol_compat_handle.msg_callback = (on) ?
	    sepol_msg_default_handler : NULL;
}

/* End deprecated */

which is used on few places internally.


Later I'll check whether sssd uses any on deprecated libsemanage symbol and
decide what to do.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]