Disable warnings due to deprecated libselinux symbols used by nss and nscd

Message ID 20200722183849.GA74546@aloka.lostca.se
State Committed
Headers
Series Disable warnings due to deprecated libselinux symbols used by nss and nscd |

Commit Message

Arjun Shankar July 22, 2020, 6:38 p.m. UTC
  From: Arjun Shankar <arjun@redhat.com>

The SELinux API deprecated several symbols in its 3.1 release, including
security_context_t, matchpathcon, avc_init, and sidput, which are used in
makedb and nscd.  While the usage of these should eventually be replaced by
newer interfaces, this commit disables GCC warnings due to the use of the
above symbols.
---
Aurelien recently posted a patch with an identical result, but using #pragma:
https://sourceware.org/pipermail/libc-alpha/2020-July/116523.html

Just put this together so it can hopefully go in soon with both glibc-2.32
being close by and Fedora 33 awaiting this change, since glibc already fails
to build on Fedora rawhide.

Tested on x86_64 with libselinux-3.1 installed.
---
 nscd/selinux.c | 16 ++++++++++++++++
 nss/makedb.c   |  9 +++++++++
 2 files changed, 25 insertions(+)
  

Comments

H.J. Lu July 22, 2020, 6:47 p.m. UTC | #1
On Wed, Jul 22, 2020 at 11:39 AM Arjun Shankar <arjun.is@lostca.se> wrote:
>
> From: Arjun Shankar <arjun@redhat.com>
>
> The SELinux API deprecated several symbols in its 3.1 release, including
> security_context_t, matchpathcon, avc_init, and sidput, which are used in
> makedb and nscd.  While the usage of these should eventually be replaced by
> newer interfaces, this commit disables GCC warnings due to the use of the
> above symbols.
> ---
> Aurelien recently posted a patch with an identical result, but using #pragma:
> https://sourceware.org/pipermail/libc-alpha/2020-July/116523.html
>
> Just put this together so it can hopefully go in soon with both glibc-2.32
> being close by and Fedora 33 awaiting this change, since glibc already fails
> to build on Fedora rawhide.
>
> Tested on x86_64 with libselinux-3.1 installed.
> ---
>  nscd/selinux.c | 16 ++++++++++++++++
>  nss/makedb.c   |  9 +++++++++
>  2 files changed, 25 insertions(+)
>
> diff --git a/nscd/selinux.c b/nscd/selinux.c
> index a4ea8008e2..8802d34265 100644
> --- a/nscd/selinux.c
> +++ b/nscd/selinux.c
> @@ -33,6 +33,7 @@
>  #ifdef HAVE_LIBAUDIT
>  # include <libaudit.h>
>  #endif
> +#include <libc-diag.h>
>
>  #include "dbg_log.h"
>  #include "selinux.h"
> @@ -320,6 +321,12 @@ avc_free_lock (void *lock)
>  }
>
>
> +/* avc_init (along with several other symbols) was marked as deprecated by the
> +   SELinux API starting from version 3.1.  We use it here, but should
> +   eventually switch to the newer API.  */
> +DIAG_PUSH_NEEDS_COMMENT
> +DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");
> +
>  /* Initialize the user space access vector cache (AVC) for NSCD along with
>     log/thread/lock callbacks.  */
>  void
> @@ -335,7 +342,14 @@ nscd_avc_init (void)
>    audit_init ();
>  #endif
>  }
> +DIAG_POP_NEEDS_COMMENT
> +
>
> +/* security_context_t and sidput (along with several other symbols) were marked
> +   as deprecated by the SELinux API starting from version 3.1.  We use them
> +   here, but should eventually switch to the newer API.  */
> +DIAG_PUSH_NEEDS_COMMENT
> +DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");
>
>  /* Check the permission from the caller (via getpeercon) to nscd.
>     Returns 0 if access is allowed, 1 if denied, and -1 on error.
> @@ -411,6 +425,7 @@ nscd_request_avc_has_perm (int fd, request_type req)
>    rc = avc_has_perm (ssid, tsid, sc_nscd, perm, &aeref, NULL) < 0;
>
>  out:
> +

Why this?

>    if (scon)
>      freecon (scon);
>    if (tcon)
> @@ -422,6 +437,7 @@ out:
>
>    return rc;
>  }
> +DIAG_POP_NEEDS_COMMENT
>
>
>  /* Wrapper to get AVC statistics.  */
> diff --git a/nss/makedb.c b/nss/makedb.c
> index 8e389a1683..8e1e8ec9ad 100644
> --- a/nss/makedb.c
> +++ b/nss/makedb.c
> @@ -38,6 +38,7 @@
>  #include <sys/stat.h>
>  #include <sys/uio.h>
>  #include "nss_db/nss_db.h"
> +#include <libc-diag.h>
>
>  /* Get libc version number.  */
>  #include "../version.h"
> @@ -841,6 +842,13 @@ print_database (int fd)
>
>
>  #ifdef HAVE_SELINUX
> +
> +/* security_context_t and matchpathcon (along with several other symbols) were
> +   marked as deprecated by the SELinux API starting from version 3.1.  We use
> +   them here, but should eventually switch to the newer API.  */
> +DIAG_PUSH_NEEDS_COMMENT
> +DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");
> +
>  static void
>  set_file_creation_context (const char *outname, mode_t mode)
>  {
> @@ -870,6 +878,7 @@ set_file_creation_context (const char *outname, mode_t mode)
>        freecon (ctx);
>      }
>  }
> +DIAG_POP_NEEDS_COMMENT
>
>  static void
>  reset_file_creation_context (void)
> --
> 2.26.2
>
  
Arjun Shankar July 22, 2020, 9:07 p.m. UTC | #2
On Wed, Jul 22, 2020 at 11:47:41AM -0700, H.J. Lu wrote:

> > +/* security_context_t and sidput (along with several other symbols) were marked
> > +   as deprecated by the SELinux API starting from version 3.1.  We use them
> > +   here, but should eventually switch to the newer API.  */
> > +DIAG_PUSH_NEEDS_COMMENT
> > +DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");
> >
> >  /* Check the permission from the caller (via getpeercon) to nscd.
> >     Returns 0 if access is allowed, 1 if denied, and -1 on error.
> > @@ -411,6 +425,7 @@ nscd_request_avc_has_perm (int fd, request_type req)
> >    rc = avc_has_perm (ssid, tsid, sc_nscd, perm, &aeref, NULL) < 0;
> >
> >  out:
> > +
> 
> Why this?

Thanks for catching that. It is a stray newline and needs to go.

I originally wanted to restrict the pragma to just the target of the "out:"
label, but then I moved away from that idea (there were other deprecated
symbols used in this function), and forgot to delete the newline I had added.

If this patch gets accepted without further changes, I'll remove the newline
before I commit.
  
Carlos O'Donell July 23, 2020, 3:34 a.m. UTC | #3
On 7/22/20 2:38 PM, Arjun Shankar wrote:
> From: Arjun Shankar <arjun@redhat.com>
> 
> The SELinux API deprecated several symbols in its 3.1 release, including
> security_context_t, matchpathcon, avc_init, and sidput, which are used in
> makedb and nscd.  While the usage of these should eventually be replaced by
> newer interfaces, this commit disables GCC warnings due to the use of the
> above symbols.

OK for 2.32. Please push.

I know that you looked at this briefly and are discussing this with the SELinux
community[1], but it will take longer to get this fixed than we have for the release.
This is a good patch in this regard and we can clean this up when 2.33 is open
and we have a strategy implemented that follows the suggestions provided by the
SELinux community.

[1] https://lore.kernel.org/selinux/39f23208-c9df-c16d-6513-49b3fd234fc7@redhat.com/T/#mea1cf1a07f3a0aa6c8a55ad308cba9b4d53aa81d

No regressions on x86_64 and i686. Builds work now with --with-selinux and
verified have-selinux is true (along with libpcap and audit).

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Tested-by: Carlos O'Donell <carlos@redhat.com>

> ---
> Aurelien recently posted a patch with an identical result, but using #pragma:
> https://sourceware.org/pipermail/libc-alpha/2020-July/116523.html
> 
> Just put this together so it can hopefully go in soon with both glibc-2.32
> being close by and Fedora 33 awaiting this change, since glibc already fails
> to build on Fedora rawhide.
> 
> Tested on x86_64 with libselinux-3.1 installed.
> ---
>  nscd/selinux.c | 16 ++++++++++++++++
>  nss/makedb.c   |  9 +++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/nscd/selinux.c b/nscd/selinux.c
> index a4ea8008e2..8802d34265 100644
> --- a/nscd/selinux.c
> +++ b/nscd/selinux.c
> @@ -33,6 +33,7 @@
>  #ifdef HAVE_LIBAUDIT
>  # include <libaudit.h>
>  #endif
> +#include <libc-diag.h>
>  
>  #include "dbg_log.h"
>  #include "selinux.h"
> @@ -320,6 +321,12 @@ avc_free_lock (void *lock)
>  }
>  
>  
> +/* avc_init (along with several other symbols) was marked as deprecated by the
> +   SELinux API starting from version 3.1.  We use it here, but should
> +   eventually switch to the newer API.  */
> +DIAG_PUSH_NEEDS_COMMENT
> +DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");

OK. The use of GCC 10 is fine, it's what we've seen the issue with.

> +
>  /* Initialize the user space access vector cache (AVC) for NSCD along with
>     log/thread/lock callbacks.  */
>  void
> @@ -335,7 +342,14 @@ nscd_avc_init (void)
>    audit_init ();
>  #endif
>  }
> +DIAG_POP_NEEDS_COMMENT

OK.

> +
>  
> +/* security_context_t and sidput (along with several other symbols) were marked
> +   as deprecated by the SELinux API starting from version 3.1.  We use them
> +   here, but should eventually switch to the newer API.  */
> +DIAG_PUSH_NEEDS_COMMENT
> +DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");
>  
>  /* Check the permission from the caller (via getpeercon) to nscd.
>     Returns 0 if access is allowed, 1 if denied, and -1 on error.
> @@ -411,6 +425,7 @@ nscd_request_avc_has_perm (int fd, request_type req)
>    rc = avc_has_perm (ssid, tsid, sc_nscd, perm, &aeref, NULL) < 0;
>  
>  out:
> +

As indicated you'll remove this.

>    if (scon)
>      freecon (scon);
>    if (tcon)
> @@ -422,6 +437,7 @@ out:
>  
>    return rc;
>  }
> +DIAG_POP_NEEDS_COMMENT

OK.

>  
>  
>  /* Wrapper to get AVC statistics.  */
> diff --git a/nss/makedb.c b/nss/makedb.c
> index 8e389a1683..8e1e8ec9ad 100644
> --- a/nss/makedb.c
> +++ b/nss/makedb.c
> @@ -38,6 +38,7 @@
>  #include <sys/stat.h>
>  #include <sys/uio.h>
>  #include "nss_db/nss_db.h"
> +#include <libc-diag.h>
>  
>  /* Get libc version number.  */
>  #include "../version.h"
> @@ -841,6 +842,13 @@ print_database (int fd)
>  
>  
>  #ifdef HAVE_SELINUX
> +
> +/* security_context_t and matchpathcon (along with several other symbols) were
> +   marked as deprecated by the SELinux API starting from version 3.1.  We use
> +   them here, but should eventually switch to the newer API.  */
> +DIAG_PUSH_NEEDS_COMMENT
> +DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");
> +
>  static void
>  set_file_creation_context (const char *outname, mode_t mode)
>  {
> @@ -870,6 +878,7 @@ set_file_creation_context (const char *outname, mode_t mode)
>        freecon (ctx);
>      }
>  }
> +DIAG_POP_NEEDS_COMMENT

OK.

>  
>  static void
>  reset_file_creation_context (void)
>
  
Aurelien Jarno July 23, 2020, 4:35 a.m. UTC | #4
On 2020-07-22 18:38, Arjun Shankar wrote:
> From: Arjun Shankar <arjun@redhat.com>
> 
> The SELinux API deprecated several symbols in its 3.1 release, including
> security_context_t, matchpathcon, avc_init, and sidput, which are used in
> makedb and nscd.  While the usage of these should eventually be replaced by
> newer interfaces, this commit disables GCC warnings due to the use of the
> above symbols.
> ---
> Aurelien recently posted a patch with an identical result, but using #pragma:
> https://sourceware.org/pipermail/libc-alpha/2020-July/116523.html
> 
> Just put this together so it can hopefully go in soon with both glibc-2.32
> being close by and Fedora 33 awaiting this change, since glibc already fails
> to build on Fedora rawhide.
> 
> Tested on x86_64 with libselinux-3.1 installed.

Thanks a lot, I confirm it works.

Aurelien
  

Patch

diff --git a/nscd/selinux.c b/nscd/selinux.c
index a4ea8008e2..8802d34265 100644
--- a/nscd/selinux.c
+++ b/nscd/selinux.c
@@ -33,6 +33,7 @@ 
 #ifdef HAVE_LIBAUDIT
 # include <libaudit.h>
 #endif
+#include <libc-diag.h>
 
 #include "dbg_log.h"
 #include "selinux.h"
@@ -320,6 +321,12 @@  avc_free_lock (void *lock)
 }
 
 
+/* avc_init (along with several other symbols) was marked as deprecated by the
+   SELinux API starting from version 3.1.  We use it here, but should
+   eventually switch to the newer API.  */
+DIAG_PUSH_NEEDS_COMMENT
+DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");
+
 /* Initialize the user space access vector cache (AVC) for NSCD along with
    log/thread/lock callbacks.  */
 void
@@ -335,7 +342,14 @@  nscd_avc_init (void)
   audit_init ();
 #endif
 }
+DIAG_POP_NEEDS_COMMENT
+
 
+/* security_context_t and sidput (along with several other symbols) were marked
+   as deprecated by the SELinux API starting from version 3.1.  We use them
+   here, but should eventually switch to the newer API.  */
+DIAG_PUSH_NEEDS_COMMENT
+DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");
 
 /* Check the permission from the caller (via getpeercon) to nscd.
    Returns 0 if access is allowed, 1 if denied, and -1 on error.
@@ -411,6 +425,7 @@  nscd_request_avc_has_perm (int fd, request_type req)
   rc = avc_has_perm (ssid, tsid, sc_nscd, perm, &aeref, NULL) < 0;
 
 out:
+
   if (scon)
     freecon (scon);
   if (tcon)
@@ -422,6 +437,7 @@  out:
 
   return rc;
 }
+DIAG_POP_NEEDS_COMMENT
 
 
 /* Wrapper to get AVC statistics.  */
diff --git a/nss/makedb.c b/nss/makedb.c
index 8e389a1683..8e1e8ec9ad 100644
--- a/nss/makedb.c
+++ b/nss/makedb.c
@@ -38,6 +38,7 @@ 
 #include <sys/stat.h>
 #include <sys/uio.h>
 #include "nss_db/nss_db.h"
+#include <libc-diag.h>
 
 /* Get libc version number.  */
 #include "../version.h"
@@ -841,6 +842,13 @@  print_database (int fd)
 
 
 #ifdef HAVE_SELINUX
+
+/* security_context_t and matchpathcon (along with several other symbols) were
+   marked as deprecated by the SELinux API starting from version 3.1.  We use
+   them here, but should eventually switch to the newer API.  */
+DIAG_PUSH_NEEDS_COMMENT
+DIAG_IGNORE_NEEDS_COMMENT (10, "-Wdeprecated-declarations");
+
 static void
 set_file_creation_context (const char *outname, mode_t mode)
 {
@@ -870,6 +878,7 @@  set_file_creation_context (const char *outname, mode_t mode)
       freecon (ctx);
     }
 }
+DIAG_POP_NEEDS_COMMENT
 
 static void
 reset_file_creation_context (void)