Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
Hi! On Thu, 2024-02-15 at 16:48:43 -0800, Steve Langasek wrote: > Control: forwarded -1 seli...@vger.kernel.org > > Patch now forwarded upstream for review. > > https://lore.kernel.org/selinux/zc6tzkpsyzric...@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
Processed: Re: Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
Processing control commands: > forwarded -1 seli...@vger.kernel.org Bug #1063329 [libselinux1t64] libselinux1t64: breaks system in upgrade from unstable Set Bug forwarded-to-address to 'seli...@vger.kernel.org'. -- 1063329: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1063329 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
Control: forwarded -1 seli...@vger.kernel.org
Patch now forwarded upstream for review.
https://lore.kernel.org/selinux/zc6tzkpsyzric...@homer.dodds.net/T/#t
On Wed, Feb 14, 2024 at 11:25:26PM -0800, Steve Langasek wrote:
> 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/
> slanga...@ubuntu.com vor...@debian.org
--
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/
slanga...@ubuntu.com vor...@debian.org
signature.asc
Description: PGP signature
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
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/
slanga...@ubuntu.com vor...@debian.org
signature.asc
Description: PGP signature
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
> "Helmut" == Helmut Grohne 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 signature.asc Description: PGP signature
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
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
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
07.02.2024 11:06, Helmut Grohne :
..
pam seems difficult:
| extern time_t pam_misc_conv_warn_time; /* time that we should warn user */
| extern time_t pam_misc_conv_die_time; /* cut-off time for input */
Attached is a sketch to make pam compatible.
I had a more complete and *tested* fix 2 days ago but I forgot
it was in /tmp and I rebooted the machine, so had to do it again
yesterday.
The idea is to have both die_time and die_time64, and keep them
in sync (using minimum between two values which is !=0).
This is a sketch using a #define, though better is to use symbol
versioning here and have time32 compat stuff for old programs
and 64bit time stuff for new, using redirection at the link time,
instead of the #defines which makes whole thing rather difficult
to read, - that's extra several lines of code, also to the .map
file.
What the whole thing needs is the criteria to use to enable the
trick. Right now I used #ifdef NEED_TIME64_COMPAT which should
be defined somehow, - since I don't know the precise list of
architectures where this has to be done. This is an externally-
controlled thing, there's no way to determine this directly from
the .c code (short of using architecture list in the .h file),
so it must be some symbol substituted at the package build time,
like Provides: t64:Compat (or whatever it is) is substituted in
d/control.
This is a less-intrusve-to-original-code version of the sketch,
ie, I tried to keep all changes outside of the original code as
possible, keeping all the original logic as it is.
/mjtdiff --git a/libpam_misc/include/security/pam_misc.h b/libpam_misc/include/security/pam_misc.h
index fca2422..922341c 100644
--- a/libpam_misc/include/security/pam_misc.h
+++ b/libpam_misc/include/security/pam_misc.h
@@ -21,6 +21,15 @@ extern int misc_conv(int num_msg, const struct pam_message **msgm,
#include
+#if NEED_TIME64_COMPAT
+
+extern time32_t pam_misc_conv_warn_time;
+extern time32_t pam_misc_conv_die_time;
+#define pam_misc_conv_warn_time pam_misc_conv_warn_time64
+#define pam_misc_conv_die_time pam_misc_conv_die_time64
+
+#endif
+
extern time_t pam_misc_conv_warn_time; /* time that we should warn user */
extern time_t pam_misc_conv_die_time; /* cut-off time for input */
extern const char *pam_misc_conv_warn_line; /* warning notice */
diff --git a/libpam_misc/misc_conv.c b/libpam_misc/misc_conv.c
index 908ee89..a02d491 100644
--- a/libpam_misc/misc_conv.c
+++ b/libpam_misc/misc_conv.c
@@ -30,6 +30,9 @@
time_t pam_misc_conv_warn_time = 0; /* time when we warn */
time_t pam_misc_conv_die_time = 0; /* time when we timeout */
+static void fixup_compat_time();
+static void reset_warn_time();
+
const char *pam_misc_conv_warn_line = N_("...Time is running out...\n");
const char *pam_misc_conv_die_line = N_("...Sorry, your time is up!\n");
@@ -96,6 +99,8 @@ static int get_delay(void)
expired = 0;/* reset flag */
(void) time();
+fixup_compat_time();
+
/* has the quit time past? */
if (pam_misc_conv_die_time && now >= pam_misc_conv_die_time) {
fprintf(stderr,"%s",pam_misc_conv_die_line);
@@ -107,7 +112,7 @@ static int get_delay(void)
/* has the warning time past? */
if (pam_misc_conv_warn_time && now >= pam_misc_conv_warn_time) {
fprintf(stderr, "%s", pam_misc_conv_warn_line);
- pam_misc_conv_warn_time = 0;/* reset warn_time */
+ reset_warn_time();
/* indicate remaining delay - if any */
@@ -399,3 +404,36 @@ failed_conversation:
return PAM_CONV_ERR;
}
+
+#ifdef NEED_TIME64_COMPAT
+
+#undef pam_misc_conv_warn_time
+#undef pam_misc_conv_die_time
+
+int pam_misc_conv_warn_time, pam_misc_conv_die_time;
+
+/* in compat 32/64 case, we have 2 values: time and time64.
+ We perform all operations with time64
+ But we try to keep them in sync, whiciever is smaller but !0 */
+#define fixup(t, t32) \
+if (t32 && (!t || t > t32)) t = t32; /* if t32 fires sooner */ \
+if (t < 0x7fff) t32 = t /* only if t can fit in t32 */
+
+static void fixup_compat_time() {
+fixup(pam_misc_conv_warn_time64, pam_misc_conv_warn_time);
+fixup(pam_misc_conv_die_time64, pam_misc_conv_die_time);
+}
+static void reset_warn_time() {
+ pam_misc_conv_warn_time = 0;
+ pam_misc_conv_warn_time64 = 0;
+}
+
+#else
+
+static void fixup_compat_time() {
+}
+static void reset_warn_time() {
+ pam_misc_conv_warn_time = 0;
+}
+
+#endif
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
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
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
On 2024-02-06 Helmut Grohne wrote:
> Package: 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,
[...]
> both dpkg and tar are now broken. This is pretty bad. To make matters
> worse, the situation arises from the combination of Breaks + Provides
[...]
Hello,
color me stupid but isn't this fishy?
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)".
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.
Otherwise the plan looks flawed.
cu Andreas
--
`What a good friend you are to him, Dr. Maturin. His other friends are
so grateful to you.'
`I sew his ears on from time to time, sure'
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
Hi Guillem,
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.
> 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.
My real question here though is what's the downsides of providing two
variants of this symbol (whether with symbol versioning or name
redirection). From my pov, this effectively is your option 3 and what I
sketched is the most stupid implementation of it. My sketch did assume
that libselinux would be built with LFS support everywhere including
i386. Enabling that on the upstream side definitely is even better,
because it gets us to not have a Debian-specific ABI.
> I think there are only three ways to go about this, excluding the t64
> attempt:
Thanks for confirming that I've reported a real problem.
> If you'd like assistance with trying to get a proposal for 3 to
> present upstream I could look into that. But I think they should be
> involved early on to see what they'd like to see and what they might
> outright reject.
>From my naive point of view, this option 3 is the clear winner. Though
it all depends on what upstream says. If upstream cooperates on any
option, that's better still as we avoid ABI deviation.
Going from here, I also looked a bit into whether we could additionally
use an upstream-cooperating approach for other packages central to
Debian to avoid t64 bumps.
pam seems difficult:
| extern time_t pam_misc_conv_warn_time; /* time that we should warn user */
| extern time_t pam_misc_conv_die_time; /* cut-off time for input */
We cannot symbol-version these in a reasonable way. All we could do is
ask upstream for a real soname bump. We have a slight advantage here: On
little endian (such as armhf), we can extend this to 64bit and 32bit
accesses will continue to work for small values. However, doing this to
m68k would break horribly. I also couldn't find any in-Debian users of
these symbols (super merely vendors pam source), so just bumping it and
accepting breakage (Guillems option 1) might be worth a go?
For libaudit1, I fail to understand why we bump it at all. Both reports
look fine to me:
https://adrien.dcln.fr/misc/armhf-time_t/2024-02-01T09:53:00/compat_reports/libaudit-dev/base_to_lfs/compat_report.html
https://adrien.dcln.fr/misc/armhf-time_t/2024-02-01T09:53:00/compat_reports/libaudit-dev/lfs_to_time_t/compat_report.html
This does not extend to libauparse0 where the report gives a reason, but
libaudit1 is the one that interacts with /usr-move and libauparse0 not,
so can we skip the dance for libaudit1?
For libtirpc, it is only about rpcb_gettime, which returns time via a
time_t* and can indicate success/failure via return. It seems fairly
simple to implement ABI duality here and libtirpc already does symbol
versioning. Maybe we can also approach upstream about this?
For libfuse2, I think the ABI analysis is broken. The base-to-lfs report
supposedly is ok
https://adrien.dcln.fr/misc/armhf-time_t/2024-02-01T09:53:00/compat_reports/libfuse-dev/base_to_lfs/compat_report.html
and then going lfs-to-time changes ino_t
https://adrien.dcln.fr/misc/armhf-time_t/2024-02-01T09:53:00/compat_reports/libfuse-dev/lfs_to_time_t/compat_report.html
while I would have expected ino_t to change with lfs already. Are we
sure about this? In any case, this is more of an academic question as
adding ABI-duality would be more involved here. Moreover, I don't see
any ACC report for libfuse3-dev. Did that fail to analyze?
libiw30 only has one affected symbol:
iw_print_timeval ( char* buffer, int buflen, struct timeval const* time, struct
timezone const* tz )
Providing ABI duality for this seems doable. Moreover, libiw30 already
has soname 30, so maybe upstream is open to bumping it again? The
resulting library transition is
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
Hi! On Tue, 2024-02-06 at 15:42:33 +0100, Helmut Grohne wrote: > On Tue, Feb 06, 2024 at 11:34:07AM +0100, Adrien Nader wrote: > > 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. > > Can you elaborate? 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. 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. > Keep in mind that for all the 64bit architectures, there is no abi > difference as the symbol is only added for those architectures, that > currently use a 32bit ino_t. > > 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=1=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. > > And here you are arguing that there are no practical users of the added > symbol, so with luck, we'd be adding an unused symbol on armhf. With bad > luck, we'd have some users and for those users we'd be ABI-incompatible > with the rest of the world on armhf. I think there are only three ways to go about this, excluding the t64 attempt: 1) Build the library with LFS unconditionally (except on i386). As there are no users in Debian, this would not break there, but would break for any external packages and locally unpackaged users of the library. 2) Bump the SONAME, ideally coordinate with upstream or alternatively with a Debian specific one. This does not break locally built packages nor locally unpackaged code linking against the library. 3) Build the library with LFS support (everywhere including i386), and on systems w/o built-in LFS, make the old symbol use 32-bit ino_t, and add a new symbol that uses 64-bit ino_t. This preserves the ABI, for external packages and locally unpackaged code linking against the library. I think the three options would cause no upgrade issues, as they include either no SONAME bump (option 1 and 3), or an actual SONAME bump (option 2) with no file conflicts involved. Personally I'd like to be able to cleanly and safely build dpkg with time64 everywhere (including i386, otherwise the port will not be even installable), so my preference is for options that make that possible (2 and 3), and from those the one with best backwards compatibility with was the main concern for excluding i386 from the time64 transition would be option 3. So I think that would be the preferred option here. If you'd like assistance with trying to get a proposal for 3 to present upstream I could look into that. But I think they should be involved early on to see what they'd like to see and what they might outright reject. Thanks, Guillem
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
On Tue, Feb 06, 2024 at 11:34:07AM +0100, Adrien Nader wrote: > 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. Can you elaborate? Keep in mind that for all the 64bit architectures, there is no abi difference as the symbol is only added for those architectures, that currently use a 32bit ino_t. > 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=1=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. And here you are arguing that there are no practical users of the added symbol, so with luck, we'd be adding an unused symbol on armhf. With bad luck, we'd have some users and for those users we'd be ABI-incompatible with the rest of the world on armhf. Helmut
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
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=1=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
Bug#1063329: libselinux1t64: breaks system in upgrade from unstable
Package: libselinux1t64 Version: 3.5-2.1~exp1 Severity: grave X-Debbugs-Cc: vor...@debian.org, debian-de...@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
