[SSSD] [PATCH] Do not segfault selinux_child if semanage_connect fails
Jakub Hrozek
jhrozek at redhat.com
Mon May 11 08:24:58 UTC 2015
On Mon, May 11, 2015 at 09:34:01AM +0200, Lukas Slebodnik wrote:
> 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
Thanks, the comment in semanage header is exactly why we had the comment
in the code that I removed with this patch..
Pushed upstream:
* master: 589a8760b38d9e2dfa278764af12d59e1487fe07
* sssd-1-12: 2ed3ac5190cd8f92d671e00837a360bd92cf150c
More information about the sssd-devel
mailing list