All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] libsepol: Get rid of the old and duplicated symbols
@ 2020-09-30 14:50 Petr Lautrbach
  2020-09-30 14:50 ` [PATCH 2/4] libsepol: Bump libsepol.so version Petr Lautrbach
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Petr Lautrbach @ 2020-09-30 14:50 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

Versioned duplicate symbols cause problems for LTO. These symbols were
introduced during the CIL integration several releases ago and were only
consumed by other SELinux userspace components.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 libsepol/cil/src/cil.c       | 84 ------------------------------------
 libsepol/src/libsepol.map.in |  5 ---
 2 files changed, 89 deletions(-)

diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index a3c6a2934c72..95bdb5e5854c 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -51,27 +51,6 @@
 #include "cil_policy.h"
 #include "cil_strpool.h"
 
-#if !defined(SHARED) || defined(ANDROID) || defined(__APPLE__)
-    #define DISABLE_SYMVER 1
-#endif
-
-#ifndef DISABLE_SYMVER
-asm(".symver cil_build_policydb_pdb,        cil_build_policydb@LIBSEPOL_1.0");
-asm(".symver cil_build_policydb_create_pdb, cil_build_policydb@@LIBSEPOL_1.1");
-
-asm(".symver cil_compile_pdb,   cil_compile@LIBSEPOL_1.0");
-asm(".symver cil_compile_nopdb, cil_compile@@LIBSEPOL_1.1");
-
-asm(".symver cil_userprefixes_to_string_pdb,   cil_userprefixes_to_string@LIBSEPOL_1.0");
-asm(".symver cil_userprefixes_to_string_nopdb, cil_userprefixes_to_string@@LIBSEPOL_1.1");
-
-asm(".symver cil_selinuxusers_to_string_pdb,   cil_selinuxusers_to_string@LIBSEPOL_1.0");
-asm(".symver cil_selinuxusers_to_string_nopdb, cil_selinuxusers_to_string@@LIBSEPOL_1.1");
-
-asm(".symver cil_filecons_to_string_pdb,   cil_filecons_to_string@LIBSEPOL_1.0");
-asm(".symver cil_filecons_to_string_nopdb, cil_filecons_to_string@@LIBSEPOL_1.1");
-#endif
-
 int cil_sym_sizes[CIL_SYM_ARRAY_NUM][CIL_SYM_NUM] = {
 	{64, 64, 64, 1 << 13, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64},
 	{64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64},
@@ -549,11 +528,7 @@ exit:
 	return rc;
 }
 
-#ifdef DISABLE_SYMVER
 int cil_compile(struct cil_db *db)
-#else
-int cil_compile_nopdb(struct cil_db *db)
-#endif
 {
 	int rc = SEPOL_ERR;
 
@@ -597,33 +572,7 @@ exit:
 	return rc;
 }
 
-#ifndef DISABLE_SYMVER
-int cil_compile_pdb(struct cil_db *db, __attribute__((unused)) sepol_policydb_t *sepol_db)
-{
-	return cil_compile_nopdb(db);
-}
-
-int cil_build_policydb_pdb(cil_db_t *db, sepol_policydb_t *sepol_db)
-{
-	int rc;
-
-	cil_log(CIL_INFO, "Building policy binary\n");
-	rc = cil_binary_create_allocated_pdb(db, sepol_db);
-	if (rc != SEPOL_OK) {
-		cil_log(CIL_ERR, "Failed to generate binary\n");
-		goto exit;
-	}
-
-exit:
-	return rc;
-}
-#endif
-
-#ifdef DISABLE_SYMVER
 int cil_build_policydb(cil_db_t *db, sepol_policydb_t **sepol_db)
-#else
-int cil_build_policydb_create_pdb(cil_db_t *db, sepol_policydb_t **sepol_db)
-#endif
 {
 	int rc;
 
@@ -1371,11 +1320,7 @@ const char * cil_node_to_string(struct cil_tree_node *node)
 	return "<unknown>";
 }
 
-#ifdef DISABLE_SYMVER
 int cil_userprefixes_to_string(struct cil_db *db, char **out, size_t *size)
-#else
-int cil_userprefixes_to_string_nopdb(struct cil_db *db, char **out, size_t *size)
-#endif
 {
 	int rc = SEPOL_ERR;
 	size_t str_len = 0;
@@ -1420,13 +1365,6 @@ exit:
 
 }
 
-#ifndef DISABLE_SYMVER
-int cil_userprefixes_to_string_pdb(struct cil_db *db, __attribute__((unused)) sepol_policydb_t *sepol_db, char **out, size_t *size)
-{
-	return cil_userprefixes_to_string_nopdb(db, out, size);
-}
-#endif
-
 static int cil_cats_to_ebitmap(struct cil_cats *cats, struct ebitmap* cats_ebitmap)
 {
 	int rc = SEPOL_ERR;
@@ -1614,11 +1552,7 @@ static int __cil_level_to_string(struct cil_level *lvl, char *out)
 	return str_tmp - out;
 }
 
-#ifdef DISABLE_SYMVER
 int cil_selinuxusers_to_string(struct cil_db *db, char **out, size_t *size)
-#else
-int cil_selinuxusers_to_string_nopdb(struct cil_db *db, char **out, size_t *size)
-#endif
 {
 	size_t str_len = 0;
 	int buf_pos = 0;
@@ -1675,18 +1609,7 @@ int cil_selinuxusers_to_string_nopdb(struct cil_db *db, char **out, size_t *size
 	return SEPOL_OK;
 }
 
-#ifndef DISABLE_SYMVER
-int cil_selinuxusers_to_string_pdb(struct cil_db *db, __attribute__((unused)) sepol_policydb_t *sepol_db, char **out, size_t *size)
-{
-	return cil_selinuxusers_to_string_nopdb(db, out, size);
-}
-#endif
-
-#ifdef DISABLE_SYMVER
 int cil_filecons_to_string(struct cil_db *db, char **out, size_t *size)
-#else
-int cil_filecons_to_string_nopdb(struct cil_db *db, char **out, size_t *size)
-#endif
 {
 	uint32_t i = 0;
 	int buf_pos = 0;
@@ -1804,13 +1727,6 @@ int cil_filecons_to_string_nopdb(struct cil_db *db, char **out, size_t *size)
 	return SEPOL_OK;
 }
 
-#ifndef DISABLE_SYMVER
-int cil_filecons_to_string_pdb(struct cil_db *db, __attribute__((unused)) sepol_policydb_t *sepol_db, char **out, size_t *size)
-{
-	return cil_filecons_to_string_nopdb(db, out, size);
-}
-#endif
-
 void cil_set_disable_dontaudit(struct cil_db *db, int disable_dontaudit)
 {
 	db->disable_dontaudit = disable_dontaudit;
diff --git a/libsepol/src/libsepol.map.in b/libsepol/src/libsepol.map.in
index f08c2a861693..98da9789b71b 100644
--- a/libsepol/src/libsepol.map.in
+++ b/libsepol/src/libsepol.map.in
@@ -1,19 +1,14 @@
 LIBSEPOL_1.0 {
   global:
 	cil_add_file;
-	cil_build_policydb;
-	cil_compile;
 	cil_db_destroy;
 	cil_db_init;
-	cil_filecons_to_string;
-	cil_selinuxusers_to_string;
 	cil_set_disable_dontaudit;
 	cil_set_disable_neverallow;
 	cil_set_handle_unknown;
 	cil_set_log_handler;
 	cil_set_log_level;
 	cil_set_preserve_tunables;
-	cil_userprefixes_to_string;
 	expand_module_avrules;
 	sepol_bool_clone;
 	sepol_bool_compare;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] libsemanage: Bump libsemanage.so version
@ 2020-10-07  9:50 Petr Lautrbach
  2020-10-07 12:35 ` Stephen Smalley
  0 siblings, 1 reply; 16+ messages in thread
From: Petr Lautrbach @ 2020-10-07  9:50 UTC (permalink / raw)
  To: SElinux list; +Cc: Stephen Smalley, Nicolas Iooss

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

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

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

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

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

And there's also

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

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

/* End deprecated */

which is used on few places internally.


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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-10-08 12:14 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-30 14:50 [PATCH 1/4] libsepol: Get rid of the old and duplicated symbols Petr Lautrbach
2020-09-30 14:50 ` [PATCH 2/4] libsepol: Bump libsepol.so version Petr Lautrbach
2020-09-30 14:50 ` [PATCH 3/4] libsemanage: Remove legacy and duplicate symbols Petr Lautrbach
2020-09-30 14:50 ` [PATCH 4/4] libsemanage: Bump libsemanage.so version Petr Lautrbach
2020-09-30 15:22   ` Stephen Smalley
2020-09-30 15:56     ` Petr Lautrbach
2020-10-01 14:18       ` Stephen Smalley
2020-10-01 16:55         ` Petr Lautrbach
2020-10-01 17:08           ` Stephen Smalley
2020-10-01 17:48             ` Petr Lautrbach
2020-10-02  6:53               ` Nicolas Iooss
2020-10-02 14:49                 ` Stephen Smalley
2020-10-02 15:41                   ` Nicolas Iooss
  -- strict thread matches above, loose matches on Subject: below --
2020-10-07  9:50 Petr Lautrbach
2020-10-07 12:35 ` Stephen Smalley
2020-10-08 12:13   ` Dominick Grift

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.