* [PATCH] Always build for LFS mode on 32-bit archs. @ 2024-02-16 0:32 Steve Langasek 2024-02-16 0:35 ` Steve Langasek 0 siblings, 1 reply; 12+ messages in thread From: Steve Langasek @ 2024-02-16 0:32 UTC (permalink / raw) To: selinux Hello, In Debian and Ubuntu, we are working to move future releases of the OS on 32-bit architectures (predominantly: armhf AKA arm-linux-gnueabihf) to use 64-bit time_t natively in anticipation of the y2038 event horizon. While for most libraries we are taking the approach to rebuild in-place with changed compiler flags and declaring incompatibility with previous package builds via the package manager, libselinux is a sufficiently core part of the OS (as a dependency of the package manager itself) that this is not tenable. Therefore I propose the following patch to libselinux to make it dual-ABI for the single LFS-sensitive entry point, congruent to the way glibc itself handles such changes. The particular implementation doesn't use as many asm extension / symbol version map tricks as glibc to make "nice" symbol names in the resulting binary, it just gives us matchpathcon_filespec_add() and matchpathcon_filespec_add64() as entrypoints. If there is a preference for more glibc-style handling with symbol versions I am happy to rework to accomodate. As this problem has been discovered rather late in our transition process, I have a bit of a time crunch on my side which is not your problem, but I would ask that whether or not the patch is ready to land, we reach a consensus ASAP on: - whether it is acceptable to introduce a new entry point for this on 32-bit architectures - the name this new entry point should use (including symbol version) - that it is acceptable to upstream that we proceed on implementing this at the distro level in advance of the patch landing upstream. Thanks! ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-16 0:32 [PATCH] Always build for LFS mode on 32-bit archs Steve Langasek @ 2024-02-16 0:35 ` Steve Langasek 2024-02-17 21:54 ` Kees Cook 0 siblings, 1 reply; 12+ messages in thread From: Steve Langasek @ 2024-02-16 0:35 UTC (permalink / raw) To: selinux [-- Attachment #1.1: Type: text/plain, Size: 371 bytes --] Sorry, I clearly don't know how to operate git send-email. -- 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 [-- Attachment #1.2: 0001-Always-build-for-LFS-mode-on-32-bit-archs.patch --] [-- Type: text/x-diff, Size: 4733 bytes --] From 76049abc81d6209e0692fa96e8cfb4053b717201 Mon Sep 17 00:00:00 2001 From: Steve Langasek <steve.langasek@canonical.com> Date: Thu, 15 Feb 2024 15:22:45 -0800 Subject: [PATCH] Always build for LFS mode on 32-bit archs. Maintains the type signature of the existing matchpathcon_filespec_add() entry point on 32-bit archs but maps the API to a new matchpathcon_filespec_add64() entry point that takes a 64-bit ino_t argument instead. Software on 32-bit Linux ports which historically use a 32-bit time_t (thus affected by the y2038 problem) have, as a precondition of migrating to 64-bit time_t, that they also migrate to large filesystem support because glibc does not provide entry points for the cross-product of (LFS: yes, LFS: no) x (time_t: 32, time_t: 64). In order to support smooth migration of such operating systems from 32-bit time_t to 64-bit time_t, it is useful for libselinux to: - provide entry points on 32-bit systems for both LFS and non-LFS variants of the API (as glibc itself does) - use LFS internally for all filesystem calls (just in case) - map the API call to the correct implementation based on the build environment of the caller. Signed-off-by: Steve Langasek <steve.langasek@canonical.com> --- libselinux/include/selinux/selinux.h | 7 ++++++- libselinux/src/Makefile | 1 + libselinux/src/libselinux.map | 1 + libselinux/src/matchpathcon.c | 15 ++++++++++++++- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h index a0948853..78953870 100644 --- a/libselinux/include/selinux/selinux.h +++ b/libselinux/include/selinux/selinux.h @@ -1,6 +1,7 @@ #ifndef _SELINUX_H_ #define _SELINUX_H_ +#include <stdint.h> #include <sys/types.h> #include <stdarg.h> @@ -521,7 +522,11 @@ extern int matchpathcon_index(const char *path, with the same inode (e.g. due to multiple hard links). If so, then use the latter of the two specifications based on their order in the file contexts configuration. Return the used specification index. */ -extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file); +typedef uint64_t libselinux_ino_t; +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 +#define matchpathcon_filespec_add matchpathcon_filespec_add64 +#endif +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file); /* Destroy any inode associations that have been added, e.g. to restart for a new filesystem. */ diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile index d3b981fc..267291aa 100644 --- a/libselinux/src/Makefile +++ b/libselinux/src/Makefile @@ -87,6 +87,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ -fasynchronous-unwind-tables -fdiagnostics-show-option \ -Werror -Wno-aggregate-return \ + -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 \ $(EXTRA_CFLAGS) LD_SONAME_FLAGS=-soname,$(LIBSO),--version-script=libselinux.map,-z,defs,-z,relro diff --git a/libselinux/src/libselinux.map b/libselinux/src/libselinux.map index 5e00f45b..88c9b3e5 100644 --- a/libselinux/src/libselinux.map +++ b/libselinux/src/libselinux.map @@ -251,4 +251,5 @@ LIBSELINUX_3.5 { global: getpidprevcon; getpidprevcon_raw; + matchpathcon_filespec_add64; } LIBSELINUX_3.4; diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c index e44734c3..189e00fb 100644 --- a/libselinux/src/matchpathcon.c +++ b/libselinux/src/matchpathcon.c @@ -195,7 +195,8 @@ static file_spec_t *fl_head; * then use the specification that occurs later in the * specification array. */ -int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) +int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, + const char *file) { file_spec_t *prevfl, *fl; int h, ret; @@ -261,6 +262,18 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) return -1; } +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 +/* alias defined in the public header but we undefine it here */ +#undef matchpathcon_filespec_add + +/* ABI backwards-compatible shim for non-LFS 32-bit systems */ +int matchpathcon_filespec_add(unsigned long ino, int specind, + const char *file) +{ + return matchpathcon_filespec_add64(ino, specind, file); +} +#endif + /* * Evaluate the association hash table distribution. */ -- 2.40.1 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-16 0:35 ` Steve Langasek @ 2024-02-17 21:54 ` Kees Cook 2024-02-18 3:08 ` Steve Langasek 0 siblings, 1 reply; 12+ messages in thread From: Kees Cook @ 2024-02-17 21:54 UTC (permalink / raw) To: Steve Langasek; +Cc: selinux On Thu, Feb 15, 2024 at 04:35:24PM -0800, Steve Langasek wrote: > index a0948853..78953870 100644 > --- a/libselinux/include/selinux/selinux.h > +++ b/libselinux/include/selinux/selinux.h > @@ -1,6 +1,7 @@ > #ifndef _SELINUX_H_ > #define _SELINUX_H_ > > +#include <stdint.h> > #include <sys/types.h> > #include <stdarg.h> > > @@ -521,7 +522,11 @@ extern int matchpathcon_index(const char *path, > with the same inode (e.g. due to multiple hard links). If so, then > use the latter of the two specifications based on their order in the > file contexts configuration. Return the used specification index. */ > -extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file); > +typedef uint64_t libselinux_ino_t; > +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 > +#define matchpathcon_filespec_add matchpathcon_filespec_add64 > +#endif > +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file); What's the ABI goal here? I think the type is wrong -- doesn't this need to be uint32_t not a uint64_t for the wrapper? -- Kees Cook ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-17 21:54 ` Kees Cook @ 2024-02-18 3:08 ` Steve Langasek 2024-02-25 6:45 ` Steve Langasek 0 siblings, 1 reply; 12+ messages in thread From: Steve Langasek @ 2024-02-18 3:08 UTC (permalink / raw) To: Kees Cook; +Cc: selinux [-- Attachment #1.1: Type: text/plain, Size: 2709 bytes --] Hi Kees, On Sat, Feb 17, 2024 at 01:54:14PM -0800, Kees Cook wrote: > On Thu, Feb 15, 2024 at 04:35:24PM -0800, Steve Langasek wrote: > > index a0948853..78953870 100644 > > --- a/libselinux/include/selinux/selinux.h > > +++ b/libselinux/include/selinux/selinux.h > > @@ -1,6 +1,7 @@ > > #ifndef _SELINUX_H_ > > #define _SELINUX_H_ > > > > +#include <stdint.h> > > #include <sys/types.h> > > #include <stdarg.h> > > > > @@ -521,7 +522,11 @@ extern int matchpathcon_index(const char *path, > > with the same inode (e.g. due to multiple hard links). If so, then > > use the latter of the two specifications based on their order in the > > file contexts configuration. Return the used specification index. */ > > -extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file); > > +typedef uint64_t libselinux_ino_t; > > +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 > > +#define matchpathcon_filespec_add matchpathcon_filespec_add64 > > +#endif > > +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file); > What's the ABI goal here? I think the type is wrong -- doesn't this need > to be uint32_t not a uint64_t for the wrapper? Thanks for the review! The intent is that there are 3 cases: - on 64-bit archs, matchpathcon_filespec_add64() is not defined because the ino_t passed to matchpathcon_filespec_add() is already 64-bit - on 32-bit archs when LFS support is defined, references to matchpathcon_filespec_add() will be rewritten to matchpathcon_filespec_add64() because the ino_t argument passed to _add() is* 32-bit - on 32-bit archs when LFS support is not defined, the existing matchpathcon_filespec_add() entry point should be used as-is. And in the process of articulating this, I've come to recognize there was a bug here, because in the case where our arch is 32-bit and our caller is NOT being invoked with LFS build flags, we are exposing the previous matchpathcon_filespec_add() but have wrongly modified the prototype here to take a 64-bit ino_t. I believe the attached revised patch corrects this. -- 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 * may be. There are some 32-bit archs defined in glibc/Linux that do not treat 32-bit time_t as an option and AIUI thus also have 64-bit ino_t; in which case the additional entry point is unnecessary but also of low impact. [-- Attachment #1.2: 0001-Always-build-for-LFS-mode-on-32-bit-archs.patch --] [-- Type: text/x-diff, Size: 4794 bytes --] From b23c9044b542b8807f57f4691f4bd1149cbee04c Mon Sep 17 00:00:00 2001 From: Steve Langasek <steve.langasek@canonical.com> Date: Thu, 15 Feb 2024 15:22:45 -0800 Subject: [PATCH] Always build for LFS mode on 32-bit archs. Maintains the type signature of the existing matchpathcon_filespec_add() entry point on 32-bit archs but maps the API to a new matchpathcon_filespec_add64() entry point that takes a 64-bit ino_t argument instead. Software on 32-bit Linux ports which historically use a 32-bit time_t (thus affected by the y2038 problem) have, as a precondition of migrating to 64-bit time_t, that they also migrate to large filesystem support because glibc does not provide entry points for the cross-product of (LFS: yes, LFS: no) x (time_t: 32, time_t: 64). In order to support smooth migration of such operating systems from 32-bit time_t to 64-bit time_t, it is useful for libselinux to: - provide entry points on 32-bit systems for both LFS and non-LFS variants of the API (as glibc itself does) - use LFS internally for all filesystem calls (just in case) - map the API call to the correct implementation based on the build environment of the caller. Signed-off-by: Steve Langasek <steve.langasek@canonical.com> --- libselinux/include/selinux/selinux.h | 11 ++++++++++- libselinux/src/Makefile | 1 + libselinux/src/libselinux.map | 1 + libselinux/src/matchpathcon.c | 15 ++++++++++++++- 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h index a0948853..040629c3 100644 --- a/libselinux/include/selinux/selinux.h +++ b/libselinux/include/selinux/selinux.h @@ -1,6 +1,7 @@ #ifndef _SELINUX_H_ #define _SELINUX_H_ +#include <stdint.h> #include <sys/types.h> #include <stdarg.h> @@ -521,7 +522,15 @@ extern int matchpathcon_index(const char *path, with the same inode (e.g. due to multiple hard links). If so, then use the latter of the two specifications based on their order in the file contexts configuration. Return the used specification index. */ -extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file); +#if _FILE_OFFSET_BITS == 64 +typedef uint64_t libselinux_ino_t; +#if __BITS_PER_LONG < 64 +#define matchpathcon_filespec_add matchpathcon_filespec_add64 +#endif +#else +typedef uint32_t libselinux_ino_t; +#endif +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file); /* Destroy any inode associations that have been added, e.g. to restart for a new filesystem. */ diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile index d3b981fc..267291aa 100644 --- a/libselinux/src/Makefile +++ b/libselinux/src/Makefile @@ -87,6 +87,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ -fasynchronous-unwind-tables -fdiagnostics-show-option \ -Werror -Wno-aggregate-return \ + -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 \ $(EXTRA_CFLAGS) LD_SONAME_FLAGS=-soname,$(LIBSO),--version-script=libselinux.map,-z,defs,-z,relro diff --git a/libselinux/src/libselinux.map b/libselinux/src/libselinux.map index 5e00f45b..88c9b3e5 100644 --- a/libselinux/src/libselinux.map +++ b/libselinux/src/libselinux.map @@ -251,4 +251,5 @@ LIBSELINUX_3.5 { global: getpidprevcon; getpidprevcon_raw; + matchpathcon_filespec_add64; } LIBSELINUX_3.4; diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c index e44734c3..189e00fb 100644 --- a/libselinux/src/matchpathcon.c +++ b/libselinux/src/matchpathcon.c @@ -195,7 +195,8 @@ static file_spec_t *fl_head; * then use the specification that occurs later in the * specification array. */ -int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) +int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, + const char *file) { file_spec_t *prevfl, *fl; int h, ret; @@ -261,6 +262,18 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) return -1; } +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 +/* alias defined in the public header but we undefine it here */ +#undef matchpathcon_filespec_add + +/* ABI backwards-compatible shim for non-LFS 32-bit systems */ +int matchpathcon_filespec_add(unsigned long ino, int specind, + const char *file) +{ + return matchpathcon_filespec_add64(ino, specind, file); +} +#endif + /* * Evaluate the association hash table distribution. */ -- 2.40.1 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-18 3:08 ` Steve Langasek @ 2024-02-25 6:45 ` Steve Langasek 2024-02-26 17:52 ` Christian Göttsche 0 siblings, 1 reply; 12+ messages in thread From: Steve Langasek @ 2024-02-25 6:45 UTC (permalink / raw) To: James Carter, Christian Göttsche, selinux; +Cc: selinux [-- Attachment #1: Type: text/plain, Size: 5719 bytes --] Hi, Adding James and Christian directly to the email, as frequent contributors to libselinux, for visibility. Debian and Ubuntu are going to have to do something with libselinux within the week, and I'd greatly prefer that the "something" not be breaking ABI compatibility with upstream. Thanks, -- 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 On Sat, Feb 17, 2024 at 07:08:14PM -0800, Steve Langasek wrote: > From b23c9044b542b8807f57f4691f4bd1149cbee04c Mon Sep 17 00:00:00 2001 > From: Steve Langasek <steve.langasek@canonical.com> > Date: Thu, 15 Feb 2024 15:22:45 -0800 > Subject: [PATCH] Always build for LFS mode on 32-bit archs. > > Maintains the type signature of the existing matchpathcon_filespec_add() > entry point on 32-bit archs but maps the API to a new > matchpathcon_filespec_add64() entry point that takes a 64-bit ino_t argument > instead. > > Software on 32-bit Linux ports which historically use a 32-bit time_t (thus > affected by the y2038 problem) have, as a precondition of migrating to > 64-bit time_t, that they also migrate to large filesystem support because > glibc does not provide entry points for the cross-product of > (LFS: yes, LFS: no) x (time_t: 32, time_t: 64). > > In order to support smooth migration of such operating systems from 32-bit > time_t to 64-bit time_t, it is useful for libselinux to: > > - provide entry points on 32-bit systems for both LFS and non-LFS variants > of the API (as glibc itself does) > - use LFS internally for all filesystem calls (just in case) > - map the API call to the correct implementation based on the build > environment of the caller. > > Signed-off-by: Steve Langasek <steve.langasek@canonical.com> > --- > libselinux/include/selinux/selinux.h | 11 ++++++++++- > libselinux/src/Makefile | 1 + > libselinux/src/libselinux.map | 1 + > libselinux/src/matchpathcon.c | 15 ++++++++++++++- > 4 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h > index a0948853..040629c3 100644 > --- a/libselinux/include/selinux/selinux.h > +++ b/libselinux/include/selinux/selinux.h > @@ -1,6 +1,7 @@ > #ifndef _SELINUX_H_ > #define _SELINUX_H_ > > +#include <stdint.h> > #include <sys/types.h> > #include <stdarg.h> > > @@ -521,7 +522,15 @@ extern int matchpathcon_index(const char *path, > with the same inode (e.g. due to multiple hard links). If so, then > use the latter of the two specifications based on their order in the > file contexts configuration. Return the used specification index. */ > -extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file); > +#if _FILE_OFFSET_BITS == 64 > +typedef uint64_t libselinux_ino_t; > +#if __BITS_PER_LONG < 64 > +#define matchpathcon_filespec_add matchpathcon_filespec_add64 > +#endif > +#else > +typedef uint32_t libselinux_ino_t; > +#endif > +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file); > > /* Destroy any inode associations that have been added, e.g. to restart > for a new filesystem. */ > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > index d3b981fc..267291aa 100644 > --- a/libselinux/src/Makefile > +++ b/libselinux/src/Makefile > @@ -87,6 +87,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi > -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ > -fasynchronous-unwind-tables -fdiagnostics-show-option \ > -Werror -Wno-aggregate-return \ > + -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 \ > $(EXTRA_CFLAGS) > > LD_SONAME_FLAGS=-soname,$(LIBSO),--version-script=libselinux.map,-z,defs,-z,relro > diff --git a/libselinux/src/libselinux.map b/libselinux/src/libselinux.map > index 5e00f45b..88c9b3e5 100644 > --- a/libselinux/src/libselinux.map > +++ b/libselinux/src/libselinux.map > @@ -251,4 +251,5 @@ LIBSELINUX_3.5 { > global: > getpidprevcon; > getpidprevcon_raw; > + matchpathcon_filespec_add64; > } LIBSELINUX_3.4; > diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c > index e44734c3..189e00fb 100644 > --- a/libselinux/src/matchpathcon.c > +++ b/libselinux/src/matchpathcon.c > @@ -195,7 +195,8 @@ static file_spec_t *fl_head; > * then use the specification that occurs later in the > * specification array. > */ > -int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) > +int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, > + const char *file) > { > file_spec_t *prevfl, *fl; > int h, ret; > @@ -261,6 +262,18 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) > return -1; > } > > +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 > +/* alias defined in the public header but we undefine it here */ > +#undef matchpathcon_filespec_add > + > +/* ABI backwards-compatible shim for non-LFS 32-bit systems */ > +int matchpathcon_filespec_add(unsigned long ino, int specind, > + const char *file) > +{ > + return matchpathcon_filespec_add64(ino, specind, file); > +} > +#endif > + > /* > * Evaluate the association hash table distribution. > */ > -- > 2.40.1 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-25 6:45 ` Steve Langasek @ 2024-02-26 17:52 ` Christian Göttsche 2024-02-27 7:00 ` Steve Langasek 0 siblings, 1 reply; 12+ messages in thread From: Christian Göttsche @ 2024-02-26 17:52 UTC (permalink / raw) To: Steve Langasek; +Cc: James Carter, selinux On Sun, 25 Feb 2024 at 07:45, Steve Langasek <vorlon@debian.org> wrote: > > Hi, > > Adding James and Christian directly to the email, as frequent contributors > to libselinux, for visibility. > > Debian and Ubuntu are going to have to do something with libselinux within > the week, and I'd greatly prefer that the "something" not be breaking ABI > compatibility with upstream. > > Thanks, > -- > 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 > > On Sat, Feb 17, 2024 at 07:08:14PM -0800, Steve Langasek wrote: > > From b23c9044b542b8807f57f4691f4bd1149cbee04c Mon Sep 17 00:00:00 2001 > > From: Steve Langasek <steve.langasek@canonical.com> > > Date: Thu, 15 Feb 2024 15:22:45 -0800 > > Subject: [PATCH] Always build for LFS mode on 32-bit archs. > > > > Maintains the type signature of the existing matchpathcon_filespec_add() > > entry point on 32-bit archs but maps the API to a new > > matchpathcon_filespec_add64() entry point that takes a 64-bit ino_t argument > > instead. > > > > Software on 32-bit Linux ports which historically use a 32-bit time_t (thus > > affected by the y2038 problem) have, as a precondition of migrating to > > 64-bit time_t, that they also migrate to large filesystem support because > > glibc does not provide entry points for the cross-product of > > (LFS: yes, LFS: no) x (time_t: 32, time_t: 64). > > > > In order to support smooth migration of such operating systems from 32-bit > > time_t to 64-bit time_t, it is useful for libselinux to: > > > > - provide entry points on 32-bit systems for both LFS and non-LFS variants > > of the API (as glibc itself does) > > - use LFS internally for all filesystem calls (just in case) > > - map the API call to the correct implementation based on the build > > environment of the caller. > > > > Signed-off-by: Steve Langasek <steve.langasek@canonical.com> > > --- > > libselinux/include/selinux/selinux.h | 11 ++++++++++- > > libselinux/src/Makefile | 1 + > > libselinux/src/libselinux.map | 1 + > > libselinux/src/matchpathcon.c | 15 ++++++++++++++- > > 4 files changed, 26 insertions(+), 2 deletions(-) > > > > diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h > > index a0948853..040629c3 100644 > > --- a/libselinux/include/selinux/selinux.h > > +++ b/libselinux/include/selinux/selinux.h > > @@ -1,6 +1,7 @@ > > #ifndef _SELINUX_H_ > > #define _SELINUX_H_ > > > > +#include <stdint.h> I needed <asm/bitsperlong.h> aswell for __BITS_PER_LONG > > #include <sys/types.h> > > #include <stdarg.h> > > > > @@ -521,7 +522,15 @@ extern int matchpathcon_index(const char *path, > > with the same inode (e.g. due to multiple hard links). If so, then > > use the latter of the two specifications based on their order in the > > file contexts configuration. Return the used specification index. */ > > -extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file); > > +#if _FILE_OFFSET_BITS == 64 One should probably check for existence: #if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 > > +typedef uint64_t libselinux_ino_t; > > +#if __BITS_PER_LONG < 64 > > +#define matchpathcon_filespec_add matchpathcon_filespec_add64 > > +#endif > > +#else > > +typedef uint32_t libselinux_ino_t; > > +#endif Is the typedef libselinux_ino_t really necessary, isn't it always just equal to ino_t? > > +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file); > > > > /* Destroy any inode associations that have been added, e.g. to restart > > for a new filesystem. */ > > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > > index d3b981fc..267291aa 100644 > > --- a/libselinux/src/Makefile > > +++ b/libselinux/src/Makefile > > @@ -87,6 +87,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi > > -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ > > -fasynchronous-unwind-tables -fdiagnostics-show-option \ > > -Werror -Wno-aggregate-return \ > > + -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 \ This won't work if CFLAGS are customized during build (e.g. by dpkg). Also utils/ is unaffected. Maybe use something like: libselinux/Makefile: USE_LFS ?= y export USE_LFS libselinux/{src,utils}/Makefile: ifeq ($(USE_LFS),y) override CFLAGS += -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 endif > > $(EXTRA_CFLAGS) > > > > LD_SONAME_FLAGS=-soname,$(LIBSO),--version-script=libselinux.map,-z,defs,-z,relro > > diff --git a/libselinux/src/libselinux.map b/libselinux/src/libselinux.map > > index 5e00f45b..88c9b3e5 100644 > > --- a/libselinux/src/libselinux.map > > +++ b/libselinux/src/libselinux.map > > @@ -251,4 +251,5 @@ LIBSELINUX_3.5 { > > global: > > getpidprevcon; > > getpidprevcon_raw; > > + matchpathcon_filespec_add64; > > } LIBSELINUX_3.4; For downstream this seems fine, for upstream this should go into a new LIBSELINUX_3.6 section. > > diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c > > index e44734c3..189e00fb 100644 > > --- a/libselinux/src/matchpathcon.c > > +++ b/libselinux/src/matchpathcon.c > > @@ -195,7 +195,8 @@ static file_spec_t *fl_head; > > * then use the specification that occurs later in the > > * specification array. > > */ > > -int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) > > +int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, > > + const char *file) > > { > > file_spec_t *prevfl, *fl; > > int h, ret; > > @@ -261,6 +262,18 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) > > return -1; > > } > > > > +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 > > +/* alias defined in the public header but we undefine it here */ > > +#undef matchpathcon_filespec_add > > + > > +/* ABI backwards-compatible shim for non-LFS 32-bit systems */ Missing extern declaration to avoid a missing-prototype warning. > > +int matchpathcon_filespec_add(unsigned long ino, int specind, Are there 32-bit architectures we like to support where unsigned long is not 4 (but 8) bytes in size? > > + const char *file) > > +{ > > + return matchpathcon_filespec_add64(ino, specind, file); > > +} > > +#endif > > + > > /* > > * Evaluate the association hash table distribution. > > */ > > -- > > 2.40.1 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-26 17:52 ` Christian Göttsche @ 2024-02-27 7:00 ` Steve Langasek 2024-02-27 17:13 ` Christian Göttsche 0 siblings, 1 reply; 12+ messages in thread From: Steve Langasek @ 2024-02-27 7:00 UTC (permalink / raw) To: Christian Göttsche; +Cc: James Carter, selinux [-- Attachment #1: Type: text/plain, Size: 6159 bytes --] Thanks for the feedback. On Mon, Feb 26, 2024 at 06:52:10PM +0100, Christian Göttsche wrote: > > > +#include <stdint.h> > I needed <asm/bitsperlong.h> aswell for __BITS_PER_LONG Can you tell me about the platform you were building on? In my tests, the code with this patch behaved as intended on Ubuntu 24.04 pre-release with glibc 2.38, on both armhf and amd64, with no additional includes required. > > > +#if _FILE_OFFSET_BITS == 64 > One should probably check for existence: > #if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 Should we not consider it an error if this is not defined? I can't think of any reason why _FILE_OFFSET_BITS would legitimately be missing and in my view it's ambiguous what we should do in this circumstance. Using the define without checking for existence first implicitly gives an error if it's missing. If preferred, we could be explicit as in: #if !defined(_FILE_OFFSET_BITS) #error [...] #endif #if _FILE_OFFSET_BITS == 64 [...] > > > +typedef uint64_t libselinux_ino_t; > > > +#if __BITS_PER_LONG < 64 > > > +#define matchpathcon_filespec_add matchpathcon_filespec_add64 > > > +#endif > > > +#else > > > +typedef uint32_t libselinux_ino_t; > > > +#endif > Is the typedef libselinux_ino_t really necessary, isn't it always just > equal to ino_t? When _FILE_OFFSET_BITS == 64, ino_t is a 64-bit type and we need to explicitly declare a 32-bit type for the compatibility interface. From sys/stat.h: # ifndef __ino_t_defined # ifndef __USE_FILE_OFFSET64 typedef __ino_t ino_t; # else typedef __ino64_t ino_t; # endif # define __ino_t_defined # endif > > > +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file); > > > > > > /* Destroy any inode associations that have been added, e.g. to restart > > > for a new filesystem. */ > > > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > > > index d3b981fc..267291aa 100644 > > > --- a/libselinux/src/Makefile > > > +++ b/libselinux/src/Makefile > > > @@ -87,6 +87,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi > > > -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ > > > -fasynchronous-unwind-tables -fdiagnostics-show-option \ > > > -Werror -Wno-aggregate-return \ > > > + -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 \ > This won't work if CFLAGS are customized during build (e.g. by dpkg). > Also utils/ is unaffected. > Maybe use something like: > libselinux/Makefile: > > USE_LFS ?= y > export USE_LFS > > libselinux/{src,utils}/Makefile: > > ifeq ($(USE_LFS),y) > override CFLAGS += -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 > endif Thanks, I'm happy to amend the patch to do that (but will wait to converge on the other points above before I resubmit). I was largely unconcerned about the case of dpkg overriding it since, going forward, dpkg would always be overriding it in the direction we wanted. But build systems for other distros may do otherwise and in that sense I agree we should make it resistant to accidental clobbering. > > > --- a/libselinux/src/libselinux.map > > > +++ b/libselinux/src/libselinux.map > > > @@ -251,4 +251,5 @@ LIBSELINUX_3.5 { > > > global: > > > getpidprevcon; > > > getpidprevcon_raw; > > > + matchpathcon_filespec_add64; > > > } LIBSELINUX_3.4; > For downstream this seems fine, for upstream this should go into a new > LIBSELINUX_3.6 section. For me the ideal outcome is bidirectional ABI compatibility with upstream, which means agreeing on the symbol version in advance before I upload this. For purposes of the upstream submission I'm happy to set this to LIBSELINUX_3.6 (yes, obviously it wouldn't be LIBSELINUX_3.5 anymore since that ABI is already fixed). Barring a committment to land this change as part of LIBSELINUX_3.6, my inclination for the downstream change this week would be to leave it as an unversioned symbol so that consumers would be forwards-compatible with later upstream inclusion regardless of the symbol version used. > > > diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c > > > index e44734c3..189e00fb 100644 > > > --- a/libselinux/src/matchpathcon.c > > > +++ b/libselinux/src/matchpathcon.c > > > @@ -195,7 +195,8 @@ static file_spec_t *fl_head; > > > * then use the specification that occurs later in the > > > * specification array. > > > */ > > > -int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) > > > +int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, > > > + const char *file) > > > { > > > file_spec_t *prevfl, *fl; > > > int h, ret; > > > @@ -261,6 +262,18 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) > > > return -1; > > > } > > > > > > +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 > > > +/* alias defined in the public header but we undefine it here */ > > > +#undef matchpathcon_filespec_add > > > + > > > +/* ABI backwards-compatible shim for non-LFS 32-bit systems */ > Missing extern declaration to avoid a missing-prototype warning. Ack - will fix. > > > +int matchpathcon_filespec_add(unsigned long ino, int specind, > Are there 32-bit architectures we like to support where unsigned long > is not 4 (but 8) bytes in size? I am not aware of any. https://en.wikipedia.org/wiki/LP64#64-bit_data_models discusses the various combinations of 32-bit and 64-bit types that are known to have been used, and while there are configurations that use a 64-bit pointer with a 32-bit long there are no examples shown for configurations that do the opposite. -- 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-27 7:00 ` Steve Langasek @ 2024-02-27 17:13 ` Christian Göttsche 2024-02-28 6:11 ` Steve Langasek 0 siblings, 1 reply; 12+ messages in thread From: Christian Göttsche @ 2024-02-27 17:13 UTC (permalink / raw) To: Steve Langasek; +Cc: James Carter, selinux On Tue, 27 Feb 2024 at 08:00, Steve Langasek <vorlon@debian.org> wrote: > > Thanks for the feedback. > > On Mon, Feb 26, 2024 at 06:52:10PM +0100, Christian Göttsche wrote: > > > > +#include <stdint.h> > > > I needed <asm/bitsperlong.h> aswell for __BITS_PER_LONG > > Can you tell me about the platform you were building on? In my tests, the > code with this patch behaved as intended on Ubuntu 24.04 pre-release with > glibc 2.38, on both armhf and amd64, with no additional includes required. Debian sid. > > > > +#if _FILE_OFFSET_BITS == 64 > > > One should probably check for existence: > > > #if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 > > Should we not consider it an error if this is not defined? I can't think of > any reason why _FILE_OFFSET_BITS would legitimately be missing and in my > view it's ambiguous what we should do in this circumstance. Using the > define without checking for existence first implicitly gives an error if > it's missing. If preferred, we could be explicit as in: > > #if !defined(_FILE_OFFSET_BITS) > #error [...] > #endif > #if _FILE_OFFSET_BITS == 64 > [...] During my testing yesterday _FILE_OFFSET_BITS was not defined by default (gcc-13/gcc-14/clang-18 on Debian sid). > > > > +typedef uint64_t libselinux_ino_t; > > > > +#if __BITS_PER_LONG < 64 > > > > +#define matchpathcon_filespec_add matchpathcon_filespec_add64 > > > > +#endif > > > > +#else > > > > +typedef uint32_t libselinux_ino_t; > > > > +#endif > > > Is the typedef libselinux_ino_t really necessary, isn't it always just > > equal to ino_t? > > When _FILE_OFFSET_BITS == 64, ino_t is a 64-bit type and we need to > explicitly declare a 32-bit type for the compatibility interface. From > sys/stat.h: > > # ifndef __ino_t_defined > # ifndef __USE_FILE_OFFSET64 > typedef __ino_t ino_t; > # else > typedef __ino64_t ino_t; > # endif > # define __ino_t_defined > # endif But isn't this the identical logic as for libselinux_ino_t? If _FILE_OFFSET_BITS is defined to be 64 libselinux_ino_t is typedef'ed to uint64_t and ino_t should also be a 64-bit integer. If _FILE_OFFSET_BITS is not 64 libselinux_ino_t is typedef'ed to uint32_t and ino_t should also be a 32-bit integer. Also if I understand the patch idea correctly the 32-bit compatibility type is only needed on 32-bit-LFS-enabled systems for the exported symbol matchpathcon_filespec_add() to not break the ABI. Changes to the header "only" lead to API breakage, which seems acceptable. > > > > +extern int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, const char *file); > > > > > > > > /* Destroy any inode associations that have been added, e.g. to restart > > > > for a new filesystem. */ > > > > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > > > > index d3b981fc..267291aa 100644 > > > > --- a/libselinux/src/Makefile > > > > +++ b/libselinux/src/Makefile > > > > @@ -87,6 +87,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi > > > > -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ > > > > -fasynchronous-unwind-tables -fdiagnostics-show-option \ > > > > -Werror -Wno-aggregate-return \ > > > > + -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 \ > > > This won't work if CFLAGS are customized during build (e.g. by dpkg). > > Also utils/ is unaffected. > > Maybe use something like: > > > libselinux/Makefile: > > > > USE_LFS ?= y > > export USE_LFS > > > > libselinux/{src,utils}/Makefile: > > > > ifeq ($(USE_LFS),y) > > override CFLAGS += -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 > > endif > > Thanks, I'm happy to amend the patch to do that (but will wait to converge > on the other points above before I resubmit). > > I was largely unconcerned about the case of dpkg overriding it since, going > forward, dpkg would always be overriding it in the direction we wanted. But > build systems for other distros may do otherwise and in that sense I agree > we should make it resistant to accidental clobbering. > > > > > --- a/libselinux/src/libselinux.map > > > > +++ b/libselinux/src/libselinux.map > > > > @@ -251,4 +251,5 @@ LIBSELINUX_3.5 { > > > > global: > > > > getpidprevcon; > > > > getpidprevcon_raw; > > > > + matchpathcon_filespec_add64; > > > > } LIBSELINUX_3.4; > > > For downstream this seems fine, for upstream this should go into a new > > LIBSELINUX_3.6 section. > > For me the ideal outcome is bidirectional ABI compatibility with upstream, > which means agreeing on the symbol version in advance before I upload this. > For purposes of the upstream submission I'm happy to set this to > LIBSELINUX_3.6 (yes, obviously it wouldn't be LIBSELINUX_3.5 anymore since > that ABI is already fixed). Barring a committment to land this change as > part of LIBSELINUX_3.6, my inclination for the downstream change this week > would be to leave it as an unversioned symbol so that consumers would be > forwards-compatible with later upstream inclusion regardless of the symbol > version used. > > > > > diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c > > > > index e44734c3..189e00fb 100644 > > > > --- a/libselinux/src/matchpathcon.c > > > > +++ b/libselinux/src/matchpathcon.c > > > > @@ -195,7 +195,8 @@ static file_spec_t *fl_head; > > > > * then use the specification that occurs later in the > > > > * specification array. > > > > */ > > > > -int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) > > > > +int matchpathcon_filespec_add(libselinux_ino_t ino, int specind, > > > > + const char *file) > > > > { > > > > file_spec_t *prevfl, *fl; > > > > int h, ret; > > > > @@ -261,6 +262,18 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) > > > > return -1; > > > > } > > > > > > > > +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 > > > > +/* alias defined in the public header but we undefine it here */ > > > > +#undef matchpathcon_filespec_add > > > > + > > > > +/* ABI backwards-compatible shim for non-LFS 32-bit systems */ > > > Missing extern declaration to avoid a missing-prototype warning. > > Ack - will fix. > > > > > +int matchpathcon_filespec_add(unsigned long ino, int specind, > > > Are there 32-bit architectures we like to support where unsigned long > > is not 4 (but 8) bytes in size? > > I am not aware of any. > https://en.wikipedia.org/wiki/LP64#64-bit_data_models discusses the various > combinations of 32-bit and 64-bit types that are known to have been used, > and while there are configurations that use a 64-bit pointer with a 32-bit > long there are no examples shown for configurations that do the opposite. > > -- > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-27 17:13 ` Christian Göttsche @ 2024-02-28 6:11 ` Steve Langasek 2024-02-28 9:20 ` Petr Lautrbach 0 siblings, 1 reply; 12+ messages in thread From: Steve Langasek @ 2024-02-28 6:11 UTC (permalink / raw) To: Christian Göttsche; +Cc: James Carter, selinux [-- Attachment #1.1: Type: text/plain, Size: 6096 bytes --] Thanks again for the feedback. Since you weren't cc:ed on my earlier email, I'd like to pull out my comment at the top of the thread: As this problem has been discovered rather late in our transition process, I have a bit of a time crunch on my side which is not your problem, but I would ask that whether or not the patch is ready to land, we reach a consensus ASAP on: - whether it is acceptable to introduce a new entry point for this on 32-bit architectures - the name this new entry point should use (including symbol version) - that it is acceptable to upstream that we proceed on implementing this at the distro level in advance of the patch landing upstream. Since all of your comments have been about the mechanics of the patch landing, am I able to take it as agreed that libselinux will add a new entry point of matchpathcon_filespec_add64@LIBSELINUX_3.6 in the next release, and the rest is details? On Tue, Feb 27, 2024 at 06:13:47PM +0100, Christian Göttsche wrote: > On Tue, 27 Feb 2024 at 08:00, Steve Langasek <vorlon@debian.org> wrote: > > > > Thanks for the feedback. > > > > On Mon, Feb 26, 2024 at 06:52:10PM +0100, Christian Göttsche wrote: > > > > > +#include <stdint.h> > > > I needed <asm/bitsperlong.h> aswell for __BITS_PER_LONG > > Can you tell me about the platform you were building on? In my tests, the > > code with this patch behaved as intended on Ubuntu 24.04 pre-release with > > glibc 2.38, on both armhf and amd64, with no additional includes required. > Debian sid. Ok. https://codesearch.debian.net/search?q=%3Casm%2Fbitsperlong.h%3E&literal=1 shows it's uncommon to include asm/bitsperlong directly from userspace packages, but not unprecedented, so I'll go ahead and make that change. > > > > > +#if _FILE_OFFSET_BITS == 64 > > > One should probably check for existence: > > > #if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 > > Should we not consider it an error if this is not defined? I can't think of > > any reason why _FILE_OFFSET_BITS would legitimately be missing and in my > > view it's ambiguous what we should do in this circumstance. Using the > > define without checking for existence first implicitly gives an error if > > it's missing. If preferred, we could be explicit as in: > > > > #if !defined(_FILE_OFFSET_BITS) > > #error [...] > > #endif > > #if _FILE_OFFSET_BITS == 64 > > [...] > During my testing yesterday _FILE_OFFSET_BITS was not defined by > default (gcc-13/gcc-14/clang-18 on Debian sid). It occurs to me that I only did a test build of libselinux itself with this change, and maybe you're trying to build other software against it and therefore running into the issue when including this public header. Ok, 'defined &&' it is. > > > > > +typedef uint64_t libselinux_ino_t; > > > > > +#if __BITS_PER_LONG < 64 > > > > > +#define matchpathcon_filespec_add matchpathcon_filespec_add64 > > > > > +#endif > > > > > +#else > > > > > +typedef uint32_t libselinux_ino_t; > > > > > +#endif > > > Is the typedef libselinux_ino_t really necessary, isn't it always just > > > equal to ino_t? > > When _FILE_OFFSET_BITS == 64, ino_t is a 64-bit type and we need to > > explicitly declare a 32-bit type for the compatibility interface. From > > sys/stat.h: > > # ifndef __ino_t_defined > > # ifndef __USE_FILE_OFFSET64 > > typedef __ino_t ino_t; > > # else > > typedef __ino64_t ino_t; > > # endif > > # define __ino_t_defined > > # endif > But isn't this the identical logic as for libselinux_ino_t? > If _FILE_OFFSET_BITS is defined to be 64 libselinux_ino_t is > typedef'ed to uint64_t and ino_t should also be a 64-bit integer. > If _FILE_OFFSET_BITS is not 64 libselinux_ino_t is typedef'ed to > uint32_t and ino_t should also be a 32-bit integer. You're right, I overlooked this when refactoring based on Kees's feedback in <https://lore.kernel.org/selinux/Zdrh%2FeuXdvdWlVSp@homer.dodds.net/T/#m1d767f13e043e662555f6b8d8ddbffe033435d33>. We want the entry point exposed to the caller to always be the one using the same ino_t size as the rest of the calling code. So there's no need for this extra typedef; fixed. > > > > > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > > > > > index d3b981fc..267291aa 100644 > > > > > --- a/libselinux/src/Makefile > > > > > +++ b/libselinux/src/Makefile > > > > > @@ -87,6 +87,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi > > > > > -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ > > > > > -fasynchronous-unwind-tables -fdiagnostics-show-option \ > > > > > -Werror -Wno-aggregate-return \ > > > > > + -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 \ > > > This won't work if CFLAGS are customized during build (e.g. by dpkg). > > > Also utils/ is unaffected. > > > Maybe use something like: > > > libselinux/Makefile: > > > USE_LFS ?= y > > > export USE_LFS > > > libselinux/{src,utils}/Makefile: > > > ifeq ($(USE_LFS),y) > > > override CFLAGS += -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 > > > endif > > Thanks, I'm happy to amend the patch to do that (but will wait to converge > > on the other points above before I resubmit). As an aside here, it doesn't actually matter if the utils are built with LFS support for this purpose; the point is to make sure the library isn't broken for callers, whether they have LFS enabled or not. But as long as we're changing the library anyway to always be LFS-compatible, there's no harm in also turning it on for the utils. Attached an updated patch which I think addresses all your feedback. -- 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 [-- Attachment #1.2: 0001-Always-build-for-LFS-mode-on-32-bit-archs.patch --] [-- Type: text/x-diff, Size: 5499 bytes --] From fb2e5ecaa84a2bb3d9f1f3946968add62e0ce5df Mon Sep 17 00:00:00 2001 From: Steve Langasek <steve.langasek@canonical.com> Date: Thu, 15 Feb 2024 15:22:45 -0800 Subject: [PATCH] Always build for LFS mode on 32-bit archs. Maintains the type signature of the existing matchpathcon_filespec_add() entry point on 32-bit archs but maps the API to a new matchpathcon_filespec_add64() entry point that takes a 64-bit ino_t argument instead. Software on 32-bit Linux ports which historically use a 32-bit time_t (thus affected by the y2038 problem) have, as a precondition of migrating to 64-bit time_t, that they also migrate to large filesystem support because glibc does not provide entry points for the cross-product of (LFS: yes, LFS: no) x (time_t: 32, time_t: 64). In order to support smooth migration of such operating systems from 32-bit time_t to 64-bit time_t, it is useful for libselinux to: - provide entry points on 32-bit systems for both LFS and non-LFS variants of the API (as glibc itself does) - use LFS internally for all filesystem calls (just in case) - map the API call to the correct implementation based on the build environment of the caller. Signed-off-by: Steve Langasek <steve.langasek@canonical.com> --- libselinux/Makefile | 6 ++++++ libselinux/include/selinux/selinux.h | 5 +++++ libselinux/src/Makefile | 2 ++ libselinux/src/libselinux.map | 5 +++++ libselinux/src/matchpathcon.c | 16 ++++++++++++++++ libselinux/utils/Makefile | 2 ++ 6 files changed, 36 insertions(+) diff --git a/libselinux/Makefile b/libselinux/Makefile index 6d9e2736..a50b6491 100644 --- a/libselinux/Makefile +++ b/libselinux/Makefile @@ -34,6 +34,12 @@ PCRE_CFLAGS += $(shell $(PKG_CONFIG) --cflags $(PCRE_MODULE)) PCRE_LDLIBS := $(shell $(PKG_CONFIG) --libs $(PCRE_MODULE)) export PCRE_MODULE PCRE_CFLAGS PCRE_LDLIBS +USE_LFS ?= y +ifeq ($(USE_LFS),y) + LFS_CFLAGS := -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 +endif +export LFS_CFLAGS + OS := $(shell uname) export OS diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h index a0948853..080b486c 100644 --- a/libselinux/include/selinux/selinux.h +++ b/libselinux/include/selinux/selinux.h @@ -1,8 +1,10 @@ #ifndef _SELINUX_H_ #define _SELINUX_H_ +#include <stdint.h> #include <sys/types.h> #include <stdarg.h> +#include <asm/bitsperlong.h> #ifdef __cplusplus extern "C" { @@ -521,6 +523,9 @@ extern int matchpathcon_index(const char *path, with the same inode (e.g. due to multiple hard links). If so, then use the latter of the two specifications based on their order in the file contexts configuration. Return the used specification index. */ +#if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 +#define matchpathcon_filespec_add matchpathcon_filespec_add64 +#endif extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file); /* Destroy any inode associations that have been added, e.g. to restart diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile index d3b981fc..9580cce8 100644 --- a/libselinux/src/Makefile +++ b/libselinux/src/Makefile @@ -89,6 +89,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi -Werror -Wno-aggregate-return \ $(EXTRA_CFLAGS) +override CFLAGS += $(LFS_CFLAGS) + LD_SONAME_FLAGS=-soname,$(LIBSO),--version-script=libselinux.map,-z,defs,-z,relro ifeq ($(OS), Darwin) diff --git a/libselinux/src/libselinux.map b/libselinux/src/libselinux.map index 5e00f45b..5176c8a9 100644 --- a/libselinux/src/libselinux.map +++ b/libselinux/src/libselinux.map @@ -252,3 +252,8 @@ LIBSELINUX_3.5 { getpidprevcon; getpidprevcon_raw; } LIBSELINUX_3.4; + +LIBSELINUX_3.6 { + global: + matchpathcon_filespec_add64; +} LIBSELINUX_3.5; diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c index e44734c3..f8ee4b4b 100644 --- a/libselinux/src/matchpathcon.c +++ b/libselinux/src/matchpathcon.c @@ -261,6 +261,22 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) return -1; } +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 +/* alias defined in the public header but we undefine it here */ +#undef matchpathcon_filespec_add + +/* ABI backwards-compatible shim for non-LFS 32-bit systems */ + +extern int matchpathcon_filespec_add(unsigned long ino, int specind, + const char *file); + +int matchpathcon_filespec_add(unsigned long ino, int specind, + const char *file) +{ + return matchpathcon_filespec_add64(ino, specind, file); +} +#endif + /* * Evaluate the association hash table distribution. */ diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile index f3cedc11..0d7095b1 100644 --- a/libselinux/utils/Makefile +++ b/libselinux/utils/Makefile @@ -36,6 +36,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi -Werror -Wno-aggregate-return -Wno-redundant-decls -Wstrict-overflow=5 \ $(EXTRA_CFLAGS) +override CFLAGS += $(LFS_CFLAGS) + ifeq ($(OS), Darwin) override CFLAGS += -I/opt/local/include -I../../libsepol/include override LDFLAGS += -L../../libsepol/src -undefined dynamic_lookup -- 2.40.1 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-28 6:11 ` Steve Langasek @ 2024-02-28 9:20 ` Petr Lautrbach 2024-03-03 8:00 ` Steve Langasek 0 siblings, 1 reply; 12+ messages in thread From: Petr Lautrbach @ 2024-02-28 9:20 UTC (permalink / raw) To: Steve Langasek, Christian Göttsche; +Cc: James Carter, selinux Steve Langasek <vorlon@debian.org> writes: > Thanks again for the feedback. > > Since you weren't cc:ed on my earlier email, I'd like to pull out my comment > at the top of the thread: > > As this problem has been discovered rather late in our transition process, I > have a bit of a time crunch on my side which is not your problem, but I > would ask that whether or not the patch is ready to land, we reach a > consensus ASAP on: > > - whether it is acceptable to introduce a new entry point for this on > 32-bit architectures > - the name this new entry point should use (including symbol version) > - that it is acceptable to upstream that we proceed on implementing this > at the distro level in advance of the patch landing upstream. > > Since all of your comments have been about the mechanics of the patch > landing, am I able to take it as agreed that libselinux will add a new entry > point of matchpathcon_filespec_add64@LIBSELINUX_3.6 in the next release, and > the rest is details? Technical detail: next release will be 3.7 therefore I'd expect matchpathcon_filespec_add64@LIBSELINUX_3.7 > On Tue, Feb 27, 2024 at 06:13:47PM +0100, Christian Göttsche wrote: >> On Tue, 27 Feb 2024 at 08:00, Steve Langasek <vorlon@debian.org> wrote: >> > >> > Thanks for the feedback. >> > >> > On Mon, Feb 26, 2024 at 06:52:10PM +0100, Christian Göttsche wrote: >> > > > > +#include <stdint.h> > >> > > I needed <asm/bitsperlong.h> aswell for __BITS_PER_LONG > >> > Can you tell me about the platform you were building on? In my tests, the >> > code with this patch behaved as intended on Ubuntu 24.04 pre-release with >> > glibc 2.38, on both armhf and amd64, with no additional includes required. > >> Debian sid. > > Ok. https://codesearch.debian.net/search?q=%3Casm%2Fbitsperlong.h%3E&literal=1 > shows it's uncommon to include asm/bitsperlong directly from userspace > packages, but not unprecedented, so I'll go ahead and make that change. > >> > > > > +#if _FILE_OFFSET_BITS == 64 > >> > > One should probably check for existence: > >> > > #if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 > >> > Should we not consider it an error if this is not defined? I can't think of >> > any reason why _FILE_OFFSET_BITS would legitimately be missing and in my >> > view it's ambiguous what we should do in this circumstance. Using the >> > define without checking for existence first implicitly gives an error if >> > it's missing. If preferred, we could be explicit as in: >> > >> > #if !defined(_FILE_OFFSET_BITS) >> > #error [...] >> > #endif >> > #if _FILE_OFFSET_BITS == 64 >> > [...] > >> During my testing yesterday _FILE_OFFSET_BITS was not defined by >> default (gcc-13/gcc-14/clang-18 on Debian sid). > > It occurs to me that I only did a test build of libselinux itself with this > change, and maybe you're trying to build other software against it and > therefore running into the issue when including this public header. Ok, > 'defined &&' it is. > >> > > > > +typedef uint64_t libselinux_ino_t; >> > > > > +#if __BITS_PER_LONG < 64 >> > > > > +#define matchpathcon_filespec_add matchpathcon_filespec_add64 >> > > > > +#endif >> > > > > +#else >> > > > > +typedef uint32_t libselinux_ino_t; >> > > > > +#endif > >> > > Is the typedef libselinux_ino_t really necessary, isn't it always just >> > > equal to ino_t? > >> > When _FILE_OFFSET_BITS == 64, ino_t is a 64-bit type and we need to >> > explicitly declare a 32-bit type for the compatibility interface. From >> > sys/stat.h: > >> > # ifndef __ino_t_defined >> > # ifndef __USE_FILE_OFFSET64 >> > typedef __ino_t ino_t; >> > # else >> > typedef __ino64_t ino_t; >> > # endif >> > # define __ino_t_defined >> > # endif > >> But isn't this the identical logic as for libselinux_ino_t? >> If _FILE_OFFSET_BITS is defined to be 64 libselinux_ino_t is >> typedef'ed to uint64_t and ino_t should also be a 64-bit integer. >> If _FILE_OFFSET_BITS is not 64 libselinux_ino_t is typedef'ed to >> uint32_t and ino_t should also be a 32-bit integer. > > You're right, I overlooked this when refactoring based on Kees's feedback in > <https://lore.kernel.org/selinux/Zdrh%2FeuXdvdWlVSp@homer.dodds.net/T/#m1d767f13e043e662555f6b8d8ddbffe033435d33>. > > We want the entry point exposed to the caller to always be the one using the > same ino_t size as the rest of the calling code. So there's no need for > this extra typedef; fixed. > >> > > > > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile >> > > > > index d3b981fc..267291aa 100644 >> > > > > --- a/libselinux/src/Makefile >> > > > > +++ b/libselinux/src/Makefile >> > > > > @@ -87,6 +87,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi >> > > > > -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ >> > > > > -fasynchronous-unwind-tables -fdiagnostics-show-option \ >> > > > > -Werror -Wno-aggregate-return \ >> > > > > + -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 \ > >> > > This won't work if CFLAGS are customized during build (e.g. by dpkg). >> > > Also utils/ is unaffected. >> > > Maybe use something like: > >> > > libselinux/Makefile: > >> > > USE_LFS ?= y >> > > export USE_LFS > >> > > libselinux/{src,utils}/Makefile: > >> > > ifeq ($(USE_LFS),y) >> > > override CFLAGS += -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 >> > > endif > >> > Thanks, I'm happy to amend the patch to do that (but will wait to converge >> > on the other points above before I resubmit). > > As an aside here, it doesn't actually matter if the utils are built with LFS > support for this purpose; the point is to make sure the library isn't broken > for callers, whether they have LFS enabled or not. But as long as we're > changing the library anyway to always be LFS-compatible, there's no harm in > also turning it on for the utils. > > Attached an updated patch which I think addresses all your feedback. > > -- > 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 > From fb2e5ecaa84a2bb3d9f1f3946968add62e0ce5df Mon Sep 17 00:00:00 2001 > From: Steve Langasek <steve.langasek@canonical.com> > Date: Thu, 15 Feb 2024 15:22:45 -0800 > Subject: [PATCH] Always build for LFS mode on 32-bit archs. > > Maintains the type signature of the existing matchpathcon_filespec_add() > entry point on 32-bit archs but maps the API to a new > matchpathcon_filespec_add64() entry point that takes a 64-bit ino_t argument > instead. > > Software on 32-bit Linux ports which historically use a 32-bit time_t (thus > affected by the y2038 problem) have, as a precondition of migrating to > 64-bit time_t, that they also migrate to large filesystem support because > glibc does not provide entry points for the cross-product of > (LFS: yes, LFS: no) x (time_t: 32, time_t: 64). > > In order to support smooth migration of such operating systems from 32-bit > time_t to 64-bit time_t, it is useful for libselinux to: > > - provide entry points on 32-bit systems for both LFS and non-LFS variants > of the API (as glibc itself does) > - use LFS internally for all filesystem calls (just in case) > - map the API call to the correct implementation based on the build > environment of the caller. > > Signed-off-by: Steve Langasek <steve.langasek@canonical.com> > --- > libselinux/Makefile | 6 ++++++ > libselinux/include/selinux/selinux.h | 5 +++++ > libselinux/src/Makefile | 2 ++ > libselinux/src/libselinux.map | 5 +++++ > libselinux/src/matchpathcon.c | 16 ++++++++++++++++ > libselinux/utils/Makefile | 2 ++ > 6 files changed, 36 insertions(+) > > diff --git a/libselinux/Makefile b/libselinux/Makefile > index 6d9e2736..a50b6491 100644 > --- a/libselinux/Makefile > +++ b/libselinux/Makefile > @@ -34,6 +34,12 @@ PCRE_CFLAGS += $(shell $(PKG_CONFIG) --cflags $(PCRE_MODULE)) > PCRE_LDLIBS := $(shell $(PKG_CONFIG) --libs $(PCRE_MODULE)) > export PCRE_MODULE PCRE_CFLAGS PCRE_LDLIBS > > +USE_LFS ?= y > +ifeq ($(USE_LFS),y) > + LFS_CFLAGS := -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 > +endif > +export LFS_CFLAGS > + > OS := $(shell uname) > export OS > > diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h > index a0948853..080b486c 100644 > --- a/libselinux/include/selinux/selinux.h > +++ b/libselinux/include/selinux/selinux.h > @@ -1,8 +1,10 @@ > #ifndef _SELINUX_H_ > #define _SELINUX_H_ > > +#include <stdint.h> > #include <sys/types.h> > #include <stdarg.h> > +#include <asm/bitsperlong.h> > > #ifdef __cplusplus > extern "C" { > @@ -521,6 +523,9 @@ extern int matchpathcon_index(const char *path, > with the same inode (e.g. due to multiple hard links). If so, then > use the latter of the two specifications based on their order in the > file contexts configuration. Return the used specification index. */ > +#if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 > +#define matchpathcon_filespec_add matchpathcon_filespec_add64 > +#endif > extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file); > > /* Destroy any inode associations that have been added, e.g. to restart > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > index d3b981fc..9580cce8 100644 > --- a/libselinux/src/Makefile > +++ b/libselinux/src/Makefile > @@ -89,6 +89,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi > -Werror -Wno-aggregate-return \ > $(EXTRA_CFLAGS) > > +override CFLAGS += $(LFS_CFLAGS) > + > LD_SONAME_FLAGS=-soname,$(LIBSO),--version-script=libselinux.map,-z,defs,-z,relro > > ifeq ($(OS), Darwin) > diff --git a/libselinux/src/libselinux.map b/libselinux/src/libselinux.map > index 5e00f45b..5176c8a9 100644 > --- a/libselinux/src/libselinux.map > +++ b/libselinux/src/libselinux.map > @@ -252,3 +252,8 @@ LIBSELINUX_3.5 { > getpidprevcon; > getpidprevcon_raw; > } LIBSELINUX_3.4; > + > +LIBSELINUX_3.6 { > + global: > + matchpathcon_filespec_add64; > +} LIBSELINUX_3.5; > diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c > index e44734c3..f8ee4b4b 100644 > --- a/libselinux/src/matchpathcon.c > +++ b/libselinux/src/matchpathcon.c > @@ -261,6 +261,22 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) > return -1; > } > > +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 > +/* alias defined in the public header but we undefine it here */ > +#undef matchpathcon_filespec_add > + > +/* ABI backwards-compatible shim for non-LFS 32-bit systems */ > + > +extern int matchpathcon_filespec_add(unsigned long ino, int specind, > + const char *file); > + > +int matchpathcon_filespec_add(unsigned long ino, int specind, > + const char *file) > +{ > + return matchpathcon_filespec_add64(ino, specind, file); > +} > +#endif > + > /* > * Evaluate the association hash table distribution. > */ > diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile > index f3cedc11..0d7095b1 100644 > --- a/libselinux/utils/Makefile > +++ b/libselinux/utils/Makefile > @@ -36,6 +36,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi > -Werror -Wno-aggregate-return -Wno-redundant-decls -Wstrict-overflow=5 \ > $(EXTRA_CFLAGS) > > +override CFLAGS += $(LFS_CFLAGS) > + > ifeq ($(OS), Darwin) > override CFLAGS += -I/opt/local/include -I../../libsepol/include > override LDFLAGS += -L../../libsepol/src -undefined dynamic_lookup > -- > 2.40.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Always build for LFS mode on 32-bit archs. 2024-02-28 9:20 ` Petr Lautrbach @ 2024-03-03 8:00 ` Steve Langasek 0 siblings, 0 replies; 12+ messages in thread From: Steve Langasek @ 2024-03-03 8:00 UTC (permalink / raw) To: Petr Lautrbach; +Cc: Christian Göttsche, James Carter, selinux [-- Attachment #1.1: Type: text/plain, Size: 824 bytes --] On Wed, Feb 28, 2024 at 10:20:22AM +0100, Petr Lautrbach wrote: > Steve Langasek <vorlon@debian.org> writes: > > Since all of your comments have been about the mechanics of the patch > > landing, am I able to take it as agreed that libselinux will add a new entry > > point of matchpathcon_filespec_add64@LIBSELINUX_3.6 in the next release, and > > the rest is details? > Technical detail: next release will be 3.7 therefore I'd expect > matchpathcon_filespec_add64@LIBSELINUX_3.7 Ok, patch updated. -- 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 [-- Attachment #1.2: 0001-Always-build-for-LFS-mode-on-32-bit-archs.patch --] [-- Type: text/x-diff, Size: 5499 bytes --] From 8f137ba1da9caadc4fafe4dc7c16b6a0e51eeb80 Mon Sep 17 00:00:00 2001 From: Steve Langasek <steve.langasek@canonical.com> Date: Thu, 15 Feb 2024 15:22:45 -0800 Subject: [PATCH] Always build for LFS mode on 32-bit archs. Maintains the type signature of the existing matchpathcon_filespec_add() entry point on 32-bit archs but maps the API to a new matchpathcon_filespec_add64() entry point that takes a 64-bit ino_t argument instead. Software on 32-bit Linux ports which historically use a 32-bit time_t (thus affected by the y2038 problem) have, as a precondition of migrating to 64-bit time_t, that they also migrate to large filesystem support because glibc does not provide entry points for the cross-product of (LFS: yes, LFS: no) x (time_t: 32, time_t: 64). In order to support smooth migration of such operating systems from 32-bit time_t to 64-bit time_t, it is useful for libselinux to: - provide entry points on 32-bit systems for both LFS and non-LFS variants of the API (as glibc itself does) - use LFS internally for all filesystem calls (just in case) - map the API call to the correct implementation based on the build environment of the caller. Signed-off-by: Steve Langasek <steve.langasek@canonical.com> --- libselinux/Makefile | 6 ++++++ libselinux/include/selinux/selinux.h | 5 +++++ libselinux/src/Makefile | 2 ++ libselinux/src/libselinux.map | 5 +++++ libselinux/src/matchpathcon.c | 16 ++++++++++++++++ libselinux/utils/Makefile | 2 ++ 6 files changed, 36 insertions(+) diff --git a/libselinux/Makefile b/libselinux/Makefile index 6d9e2736..a50b6491 100644 --- a/libselinux/Makefile +++ b/libselinux/Makefile @@ -34,6 +34,12 @@ PCRE_CFLAGS += $(shell $(PKG_CONFIG) --cflags $(PCRE_MODULE)) PCRE_LDLIBS := $(shell $(PKG_CONFIG) --libs $(PCRE_MODULE)) export PCRE_MODULE PCRE_CFLAGS PCRE_LDLIBS +USE_LFS ?= y +ifeq ($(USE_LFS),y) + LFS_CFLAGS := -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 +endif +export LFS_CFLAGS + OS := $(shell uname) export OS diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h index a0948853..080b486c 100644 --- a/libselinux/include/selinux/selinux.h +++ b/libselinux/include/selinux/selinux.h @@ -1,8 +1,10 @@ #ifndef _SELINUX_H_ #define _SELINUX_H_ +#include <stdint.h> #include <sys/types.h> #include <stdarg.h> +#include <asm/bitsperlong.h> #ifdef __cplusplus extern "C" { @@ -521,6 +523,9 @@ extern int matchpathcon_index(const char *path, with the same inode (e.g. due to multiple hard links). If so, then use the latter of the two specifications based on their order in the file contexts configuration. Return the used specification index. */ +#if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 +#define matchpathcon_filespec_add matchpathcon_filespec_add64 +#endif extern int matchpathcon_filespec_add(ino_t ino, int specind, const char *file); /* Destroy any inode associations that have been added, e.g. to restart diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile index d3b981fc..9580cce8 100644 --- a/libselinux/src/Makefile +++ b/libselinux/src/Makefile @@ -89,6 +89,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi -Werror -Wno-aggregate-return \ $(EXTRA_CFLAGS) +override CFLAGS += $(LFS_CFLAGS) + LD_SONAME_FLAGS=-soname,$(LIBSO),--version-script=libselinux.map,-z,defs,-z,relro ifeq ($(OS), Darwin) diff --git a/libselinux/src/libselinux.map b/libselinux/src/libselinux.map index 5e00f45b..030d978e 100644 --- a/libselinux/src/libselinux.map +++ b/libselinux/src/libselinux.map @@ -252,3 +252,8 @@ LIBSELINUX_3.5 { getpidprevcon; getpidprevcon_raw; } LIBSELINUX_3.4; + +LIBSELINUX_3.7 { + global: + matchpathcon_filespec_add64; +} LIBSELINUX_3.5; diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c index e44734c3..f8ee4b4b 100644 --- a/libselinux/src/matchpathcon.c +++ b/libselinux/src/matchpathcon.c @@ -261,6 +261,22 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file) return -1; } +#if _FILE_OFFSET_BITS == 64 && __BITS_PER_LONG < 64 +/* alias defined in the public header but we undefine it here */ +#undef matchpathcon_filespec_add + +/* ABI backwards-compatible shim for non-LFS 32-bit systems */ + +extern int matchpathcon_filespec_add(unsigned long ino, int specind, + const char *file); + +int matchpathcon_filespec_add(unsigned long ino, int specind, + const char *file) +{ + return matchpathcon_filespec_add64(ino, specind, file); +} +#endif + /* * Evaluate the association hash table distribution. */ diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile index f3cedc11..0d7095b1 100644 --- a/libselinux/utils/Makefile +++ b/libselinux/utils/Makefile @@ -36,6 +36,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi -Werror -Wno-aggregate-return -Wno-redundant-decls -Wstrict-overflow=5 \ $(EXTRA_CFLAGS) +override CFLAGS += $(LFS_CFLAGS) + ifeq ($(OS), Darwin) override CFLAGS += -I/opt/local/include -I../../libsepol/include override LDFLAGS += -L../../libsepol/src -undefined dynamic_lookup -- 2.43.0 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] Always build for LFS mode on 32-bit archs. @ 2024-02-16 0:18 Steve Langasek 0 siblings, 0 replies; 12+ messages in thread From: Steve Langasek @ 2024-02-16 0:18 UTC (permalink / raw) To: selinux Hello, In Debian and Ubuntu, we are working to move future releases of the OS on 32-bit architectures (predominantly: armhf AKA arm-linux-gnueabihf) to use 64-bit time_t natively in anticipation of the y2038 event horizon. While for most libraries we are taking the approach to rebuild in-place with changed compiler flags and declaring incompatibility with previous package builds via the package manager, libselinux is a sufficiently core part of the OS (as a dependency of the package manager itself) that this is not tenable. Therefore I propose the following patch to libselinux to make it dual-ABI for the single LFS-sensitive entry point, congruent to the way glibc itself handles such changes. The particular implementation doesn't use as many asm extension / symbol version map tricks as glibc to make "nice" symbol names in the resulting binary, it just gives us matchpathcon_filespec_add() and matchpathcon_filespec_add64() as entrypoints. If there is a preference for more glibc-style handling with symbol versions I am happy to rework to accomodate. As this problem has been discovered rather late in our transition process, I have a bit of a time crunch on my side which is not your problem, but I would ask that whether or not the patch is ready to land, we reach a consensus ASAP on: - whether it is acceptable to introduce a new entry point for this on 32-bit architectures - the name this new entry point should use (including symbol version) - that it is acceptable to upstream that we proceed on implementing this at the distro level in advance of the patch landing upstream. Thanks! ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-03-03 8:01 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-02-16 0:32 [PATCH] Always build for LFS mode on 32-bit archs Steve Langasek 2024-02-16 0:35 ` Steve Langasek 2024-02-17 21:54 ` Kees Cook 2024-02-18 3:08 ` Steve Langasek 2024-02-25 6:45 ` Steve Langasek 2024-02-26 17:52 ` Christian Göttsche 2024-02-27 7:00 ` Steve Langasek 2024-02-27 17:13 ` Christian Göttsche 2024-02-28 6:11 ` Steve Langasek 2024-02-28 9:20 ` Petr Lautrbach 2024-03-03 8:00 ` Steve Langasek -- strict thread matches above, loose matches on Subject: below -- 2024-02-16 0:18 Steve Langasek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).