[SSSD] [PATCH] Do not segfault selinux_child if semanage_connect fails

Lukas Slebodnik lslebodn at redhat.com
Mon May 11 07:34:01 UTC 2015


On (07/05/15 12:01), Jakub Hrozek wrote:
>Hi,
>
>the attached patch fixes https://fedorahosted.org/sssd/ticket/2649.
>
>I couldn't reproduce the bug with regular testing, but only by overriding the
>return value from semanage_connect()

>From 4116f54398994ed15a5506b7927f24ffdca3d19c Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Wed, 6 May 2015 08:40:12 +0200
>Subject: [PATCH] SELINUX: Avoid disconnecting disconnected handle
>
>Resolves:
>    https://fedorahosted.org/sssd/ticket/2649
>
>libsemanage is very strict about its API usage and actually doesn't
>allow disconnecting a handle that is not connected. The unpatched code
>would fail with:
>
>    selinux_child: handle.c:231: semanage_disconnect: Assertion `sh !=
>    ((void *)0) && sh->funcs != ((void *)0) && sh->funcs->disconnect !=
>    ((void *)0)' failed.
>
>If semanage_connect() failed.
>---
> src/util/sss_semanage.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/src/util/sss_semanage.c b/src/util/sss_semanage.c
>index 01a2f41d8752e127f2aa1b72faa61c23f315edd7..d1d03988c05dc011dbd465051d50fe6acca4f845 100644
>--- a/src/util/sss_semanage.c
>+++ b/src/util/sss_semanage.c
>@@ -70,8 +70,13 @@ static void sss_semanage_error_callback(void *varg,
> 
> static void sss_semanage_close(semanage_handle_t *handle)
> {
>-    /* Calling disconnect on a disconnected handle is safe */
>-    semanage_disconnect(handle);
>+    if (handle == NULL) {
>+        return;     /* semanage uses asserts */
>+    }
>+
>+    if (semanage_is_connected(handle)) {
>+        semanage_disconnect(handle);
>+    }
>     semanage_handle_destroy(handle);
Althought handle could be NULL for function semanage_handle_destroy.
The documentation is clear.

   Deallocate all space associated with a semanage_handle_t, including
   the pointer itself.  CAUTION: this function does not disconnect
   from the backend; be sure that a semanage_disconnect() was
   previously called if the handle was connected. */
   void semanage_handle_destroy(semanage_handle_t *);

But neither semanage_is_connected nor semanage_disconnect accept NULL.
There is an assert in code but documentation does not say so.

   Disconnect from the manager given by the handle.  If already
   disconnected then this function does nothing.  Return 0 if
   disconnected properly or already disconnected, negative value on
   error. */
   int semanage_disconnect(semanage_handle_t *);

ACK
http://sssd-ci.duckdns.org/logs/job/14/97/summary.html

LS



More information about the sssd-devel mailing list