• Bug#1063329: libselinux1t64: breaks system in upgrade from unstable

    From Helmut Grohne@21:1/5 to All on Tue Feb 6 10:40:01 2024
    Package: libselinux1t64
    Version: 3.5-2.1~exp1
    Severity: grave
    X-Debbugs-Cc: vorlon@debian.org, debian-devel@lists.debian.org

    Hi,

    I was looking into performing an upgrade test of libselinux1 with
    piuparts and that didn't go well. I spare you the piuparts stuff and go
    into crafting a minimal reproducer using mmdebstrap:

    mmdebstrap --variant=apt unstable /dev/null "deb http://deb.debian.org/debian unstable main" "deb http://deb.debian.org/debian experimental main" --chrooted-customize-hook="apt-get -y install libselinux1t64"

    This looks fairly innocuous. We create a minimal sid chroot and install libselinux1t64 using apt. What could possibly go wrong? Well, apt thinks
    that it would be a good idea to avoid coinstalling breaking packages and
    first removes libselinux1 before proceeding to install libselinux1t64. Unfortunately, libselinux1 is transitively essential and dpkg links it,
    so this is what you get:

    | Reading package lists... Done
    | Building dependency tree... Done
    | The following packages will be REMOVED:
    | libselinux1
    | The following NEW packages will be installed:
    | libselinux1t64
    | 0 upgraded, 1 newly installed, 1 to remove and 0 not upgraded.
    | Need to get 75.2 kB of archives.
    | After this operation, 4096 B of additional disk space will be used.
    | Get:1 http://deb.debian.org/debian experimental/main amd64 libselinux1t64 amd64 3.5-2.1~exp1 [75.2 kB]
    | Fetched 75.2 kB in 0s (6067 kB/s)
    | debconf: delaying package configuration, since apt-utils is not installed
    | dpkg: libselinux1:amd64: dependency problems, but removing anyway as you requested:
    | util-linux depends on libselinux1 (>= 3.1~).
    | tar depends on libselinux1 (>= 3.1~).
    | sed depends on libselinux1 (>= 3.1~).
    | libpam-modules-bin depends on libselinux1 (>= 3.1~).
    | libpam-modules:amd64 depends on libselinux1 (>= 3.1~).
    | libmount1:amd64 depends on libselinux1 (>= 3.1~).
    | findutils depends on libselinux1 (>= 3.1~).
    | dpkg depends on libselinux1 (>= 3.1~).
    | coreutils depends on libselinux1 (>= 3.1~).
    | base-passwd depends on libselinux1 (>= 3.1~).
    |
    | (Reading database ... 6230 files and directories currently installed.)
    | Removing libselinux1:amd64 (3.5-2) ...
    | /usr/bin/dpkg: error while loading shared libraries: libselinux.so.1: cannot open shared object file: No such file or directory
    | E: Sub-process /usr/bin/dpkg returned an error code (127)

    At that point stuff is fairly broken and we cannot easily recover as
    both dpkg and tar are now broken. This is pretty bad. To make matters
    worse, the situation arises from the combination of Breaks + Provides
    and there is nothing libselinux1t64 could do in maintainer scripts to
    prevent this from happening, because no libselinux1t64 maintainer script
    has been run by the time damage has happened.

    I also looked into whether I could reproduce a similar failure with
    other packages such as libpam0t64 or libaudit1, but in no other case, I
    was able to construct a comparable outcome.

    I also looked into why libselinux was being time-bumped. Do I understand correctly that libselinux is entirely unaffected by time64? https://adrien.dcln.fr/misc/armhf-time_t/2024-02-01T09:53:00/compat_reports/libselinux1-dev/lfs_to_time_t/compat_report.html
    It still is affected by LFS due to using ino_t in the public ABI of matchpathcon_filespec_add: https://adrien.dcln.fr/misc/armhf-time_t/2024-02-01T09:53:00/compat_reports/libselinux1-dev/base_to_lfs/compat_report.html
    Since we also complete the LFS transition here, not bumping it would
    result in an ABI break regarding this symbol. If we were to opt
    libselinux out of the LFS transition (e.g. by removing the flags in debian/rules), then other packages being rebuilt against libselinux-dev
    with these flags enabled would be ABI-incompatible though.

    An option I see here is to provide ABI-duality for libselinux:

    -extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file);
    +typedef unsigned long libselinux_ino_t;
    +typedef uint64_t libselinux_ino64_t;
    +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file);
    +#if defined _FILE_OFFSET_BITS && _FILE_OFFSET_BITS == 64 && sizeof(unsigned long) < 8
    +extern int matchpathcon_filespec_add64(libselinux_ino64_t ino, int specind, const char *file);
    +#define matchpathcon_filespec_add matchpathcon_filespec_add64
    +#endif

    Looking at the implementation, it would be fairly possible to implement
    this. Of course, doing this comes at its own cost: We are extending the libselinux1 ABI in a Debian-specific way and thus programs built on
    Debian will not run on non-Debian.

    Another option of course is doing a proper soname bump of libselinux1 to
    a Debian-specific soname.

    I really hope, I am missing something.

    Helmut

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Adrien Nader@21:1/5 to All on Tue Feb 6 11:40:01 2024
    Hi Helmut,

    Thanks for identifying and raising this issue.
    After Graham mentioned this to me, I also looked at the reports and came
    to the same conclusion: the change is actually LFS due to ino_t in matchpathcon_filespec_add().

    Providing two APIs makes me quite uneasy due to having core components
    that would behave differently from the rest of the distribution. It
    sounds like something that will come back to bite for a long time.

    I checked on codesearch.d.n and there are very few users on this API;
    actually, none I think. Per https://codesearch.debian.net/search?q=matchpathcon_filespec_add&literal=1&perpkg=1
    - box64 mentions that API but the "code" is commented-out,
    - busybox uses it in the "setfiles" applet which isn't built,
    - android-platform-external-libselinux has it in headers but also has
    its own implementation

    That should hopefully give more freedom although I'm not sure what would
    be the preferred route.

    --
    Adrien

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Helmut Grohne@21:1/5 to Andreas Metzler on Wed Feb 7 16:40:01 2024
    Hi Andreas,

    On Wed, Feb 07, 2024 at 03:47:37PM +0100, Andreas Metzler wrote:
    Package: libselinux1t64
    Replaces: libselinux1
    Provides: libselinux1 (= 3.5-2.1~exp1)
    Breaks: libselinux1 (<< 3.5-2.1~exp1)

    Afaiui libselinux1t64 must not fullfill dpkg 1.22.4's dependency on "libselinux1 (>= 3.1~)". dpkg needs to be rebuilt and the rebuilt
    version gets a dep on "libselinux1t64 (>= 3.5)".

    The *t64 libraries only break ABI on some architectures. Notably, on all
    64bit architectures, i386 and x32, the ABI will not change. On the next
    upload after the transition, library dependencies will move to the t64 variants, yes.

    Will ${t64:Provides} stop expanding to "libselinux1 = ${binary:Version
    for real t64-builds? (The ones in experimental are not.) If that is case
    this bug and this way of testing does not make sense.

    No, the t64:Provides will remain that way for all architectures that do
    not break ABI. In theory, we could have skipped the rename on those architectures, but having architecture-dependent package names is
    annoyingly hard. Hence, we rename them on e.g. amd64 as well even though nothing changes there.

    Hope this explains

    Helmut

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Tokarev@21:1/5 to All on Thu Feb 8 06:40:01 2024
    06.02.2024 12:34, Helmut Grohne:
    ...
    An option I see here is to provide ABI-duality for libselinux:

    -extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file);
    +typedef unsigned long libselinux_ino_t;
    +typedef uint64_t libselinux_ino64_t;
    +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file);
    +#if defined _FILE_OFFSET_BITS && _FILE_OFFSET_BITS == 64 && sizeof(unsigned long) < 8

    It's good for a sketch to show an idea but it wont work in practice, -
    you can't use sizeof(foo) in a preprocessor condition. That's what
    WORDSIZE #defines are for. But it's a minor nit.

    glibc already has all the support for LFS which can be used directly,
    by copying code from any glibc header, like eg for lseek definition...

    +extern int matchpathcon_filespec_add64(libselinux_ino64_t ino, int specind, const char *file);
    +#define matchpathcon_filespec_add matchpathcon_filespec_add64
    and keeping this #define here instead of using internal in-glibc
    symbol redirection stuff.

    And ofc we need to define the compat wrapper for matchpathcon_filespec_add
    to the source, and the new 64bit symbol to libselinux.map, with the same arch-specific condition.

    /mjt

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Sam Hartman@21:1/5 to All on Thu Feb 8 16:00:01 2024
    "Helmut" == Helmut Grohne <helmut@subdivi.de> writes:

    Helmut> pam seems difficult: | extern time_t
    Helmut> pam_misc_conv_warn_time; /* time that we should warn user */
    Helmut> | extern time_t pam_misc_conv_die_time; /* cut-off time for
    Helmut> input */

    Helmut> We cannot symbol-version these in a reasonable way. All we
    Helmut> could do is ask upstream for a real soname bump. We have a
    Helmut> slight advantage here: On little endian (such as armhf), we
    Helmut> can extend this to 64bit and 32bit accesses will continue to
    Helmut> work for small values. However, doing this to m68k would
    Helmut> break horribly. I also couldn't find any in-Debian users of
    Helmut> these symbols (super merely vendors pam source), so just
    Helmut> bumping it and accepting breakage (Guillems option 1) might
    Helmut> be worth a go?

    Steve and I are unaware of usage in Debian either.

    --Sam

    -----BEGIN PGP SIGNATURE-----

    iHUEARYIAB0WIQSj2jRwbAdKzGY/4uAsbEw8qDeGdAUCZcTqswAKCRAsbEw8qDeG dCpTAP4m4uLrLeMRg+4B05bZRcon7Sw7V65ayvlXhmp/retUYwEAy4squZnAfCU+ adDoFukeSigSBbxoIT0CAZS9CHc1owk=
    =G4/j
    -----END PGP SIGNATURE-----

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Steve Langasek@21:1/5 to Helmut Grohne on Thu Feb 15 08:40:02 2024
    On Wed, Feb 07, 2024 at 09:06:58AM +0100, Helmut Grohne wrote:
    On Wed, Feb 07, 2024 at 04:32:45AM +0100, Guillem Jover wrote:
    Yes, I'm not sure I understand either. This is what symbol versioning
    makes possible, even providing different variants for the same symbol,
    see for example glibc or libbsd.

    I think symbol versioning is subtly different and glibc does not use
    symbol versioning for e.g. gettimeofday selection. With symbol
    versioning, you select a default version at release time and stick to
    it. In other words, building against the updated libselinux does not
    allow you to use the older 32bit variant of the symbol even if you opt
    out of lfs and time64 and you always get the 64bit symbol. What glibc
    does is a little more fancy than my simplistic #define in that it uses asm("name") instead. Still this approach allows for selecting which
    symbol is being used via macros (e.g. _FILE_OFFSET_BITS). Please correct
    me if I am misrepresenting this as my experience with symbol versioning
    is fairly limited.

    Agreed. libselinux as it happens does use a symbol version map so there is symbol versioning involved in some sense? but not the sense you really mean.

    (We could make the symbol map expose the two different function variants
    under the same name but different symbols; that's fine but I'll leave that
    for upstream to decide.)

    In any case, if going the bi-ABI path, I think upstream should get involved, and the shape of this decided with them. In addition
    the library should also be built with LFS by the upstream build
    system, which it does not currently, to control its ABI.

    I agree that involving upstream is a good idea and my understanding is
    that someone from Canonical is doing that already, which is why the
    schedule was delayed.

    Well, "already" is not exactly correct, but the need to resolve this
    critical showstopper bug in libselinux before making changes to the
    toolchain behavior in unstable and breaking the world has affected the timeline, yes.

    I now have a tested patch that I've raised as an MP in salsa:

    https://salsa.debian.org/selinux-team/libselinux/-/merge_requests/9

    I welcome review from the Debian libselinux maintainers prior to opening a discussion with upstream. (Which I will plan to do sometime Thursday US/Pacific)

    --
    Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developer https://www.debian.org/ slangasek@ubuntu.com vorlon@debian.org

    -----BEGIN PGP SIGNATURE-----

    iQIzBAABCgAdFiEErEg/aN5yj0PyIC/KVo0w8yGyEz0FAmXNvGAACgkQVo0w8yGy Ez3cKg//bSNoOpDIiPvjmVCDXB5AN5jZzG9kMoIv5EXkOu5WEMr29ZQTH4ahXl3R geqi6+KM5mR4tUasRiW7wxX5/BuLk7q44sYsZa9aj3E7x+gzkV5prZIIxxTbgyxq A9D0/Mj7YY/kmH47YpSqV/WiXmpcwE20cPUavl6cxXfQAVpfJt/NTpBj/6VDPM1E 9/cF2OLGAXkPeVwyihflkLsR+rpMRIfPiPk7SjpMSRD9O4qQbYJjyR5Mq7fyaug7 IlHwTGB+T3QgGjdfnXG3fc5S9C+KJCTwe2kx3NRsEAYFU7aHe2B+6jfL2KD7dOgE qWZVQ2lN6jeU6WOpfBj38/GJ0MdmzDlqwmyHwZYPYM8RsqZDJ0p9J0FHx7G6X7hi nJnlDOOpoVvy+Pv4z7PbJmKkrZO9aWs2cnA58iqASXdyM0RcUDHY/4vdjCx4TFWU 3h9Y4ZjSTwo+caEvuqEpCjFIuRStjzVfZOf+FcYO/yxwAvtGfDHhGIw4mn77Jo9N kdB/Gea6Xw08Oh1KRqRPH7L5oFrYRDUOzawSGvWEfArQRtm8ijQmziCkO1b3r9yP q+Ai1Wn/Qm6/rd3aH6xI
  • From Debian Bug Tracking System@21:1/5 to All on Fri Feb 16 02:00:01 2024
    Processing control commands:

    forwarded -1 selinux@vger.kernel.org
    Bug #1063329 [libselinux1t64] libselinux1t64: breaks system in upgrade from unstable
    Set Bug forwarded-to-address to 'selinux@vger.kernel.org'.

    --
    1063329: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1063329
    Debian Bug Tracking System
    Contact owner@bugs.debian.org with problems

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Guillem Jover@21:1/5 to Steve Langasek on Sat Feb 17 02:40:01 2024
    Hi!

    On Thu, 2024-02-15 at 16:48:43 -0800, Steve Langasek wrote:
    Control: forwarded -1 selinux@vger.kernel.org

    Patch now forwarded upstream for review.

    https://lore.kernel.org/selinux/Zc6tzKPsYZRICHya@homer.dodds.net/T/#t

    On Wed, Feb 14, 2024 at 11:25:26PM -0800, Steve Langasek wrote:
    Well, "already" is not exactly correct, but the need to resolve this critical showstopper bug in libselinux before making changes to the toolchain behavior in unstable and breaking the world has affected the timeline, yes.

    I now have a tested patch that I've raised as an MP in salsa:

    https://salsa.debian.org/selinux-team/libselinux/-/merge_requests/9

    I welcome review from the Debian libselinux maintainers prior to opening a discussion with upstream. (Which I will plan to do sometime Thursday US/Pacific)

    Thanks for preparing the patch. I checked it and left a comment on the
    MR there.

    Regards,
    Guillem

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)