- Fix a couple of segfaults that may happen on reload

sssd-0.4.1-reload_conf.patch

@@ -0,0 +1,113 @@
+From 673c2ce9b3371241de872b2bd206f732485888cb Mon Sep 17 00:00:00 2001
+From: Stephen Gallagher <sgallagh@redhat.com>
+Date: Fri, 19 Jun 2009 11:09:33 -0400
+Subject: [PATCH] Fix segfault in update_monitor_config
+
+We were stealing the memory context of only the first value in
+the linked-list of domains (and also services). This patch adds a
+memory context to hold the lists so that can be stolen along with
+all of the entries.
+---
+ server/confdb/confdb.c   |    4 ++++
+ server/monitor/monitor.c |   34 ++++++++++++++++++++++++++--------
+ 2 files changed, 30 insertions(+), 8 deletions(-)
+
+diff --git a/server/confdb/confdb.c b/server/confdb/confdb.c
+index 8eefcfb..8b8dc74 100644
+--- a/server/confdb/confdb.c
++++ b/server/confdb/confdb.c
+@@ -709,6 +709,10 @@ int confdb_get_domain(struct confdb_ctx *cdb,
+     }
+ 
+     domain = talloc_zero(mem_ctx, struct sss_domain_info);
++    if (!domain) {
++        ret = ENOMEM;
++        goto done;
++    }
+ 
+     tmp = ldb_msg_find_attr_as_string(res->msgs[0], "cn", NULL);
+     if (!tmp) {
+diff --git a/server/monitor/monitor.c b/server/monitor/monitor.c
+index 906d157..e4fca25 100644
+--- a/server/monitor/monitor.c
++++ b/server/monitor/monitor.c
+@@ -84,7 +84,9 @@ struct mt_svc {
+ struct mt_ctx {
+     struct tevent_context *ev;
+     struct confdb_ctx *cdb;
++    TALLOC_CTX *domain_ctx; /* Memory context for domain list */
+     struct sss_domain_info *domains;
++    TALLOC_CTX *service_ctx; /* Memory context for services */
+     char **services;
+     struct mt_svc *svc_list;
+     struct sbus_srv_ctx *sbus_srv;
+@@ -619,8 +621,13 @@ int get_monitor_config(struct mt_ctx *ctx)
+         return ret;
+     }
+ 
+-    ret = confdb_get_string_as_list(ctx->cdb, ctx, SERVICE_CONF_ENTRY,
+-                                    "activeServices", &ctx->services);
++    ctx->service_ctx = talloc_new(ctx);
++    if(!ctx->service_ctx) {
++        return ENOMEM;
++    }
++    ret = confdb_get_string_as_list(ctx->cdb, ctx->service_ctx,
++                                    SERVICE_CONF_ENTRY, "activeServices",
++                                    &ctx->services);
+     if (ret != EOK) {
+         DEBUG(0, ("No services configured!\n"));
+         return EINVAL;
+@@ -631,7 +638,11 @@ int get_monitor_config(struct mt_ctx *ctx)
+         return ret;
+     }
+ 
+-    ret = confdb_get_domains(ctx->cdb, ctx, &ctx->domains);
++    ctx->domain_ctx = talloc_new(ctx);
++    if(!ctx->domain_ctx) {
++        return ENOMEM;
++    }
++    ret = confdb_get_domains(ctx->cdb, ctx->domain_ctx, &ctx->domains);
+     if (ret != EOK) {
+         DEBUG(2, ("No domains configured. LOCAL should always exist!\n"));
+         return ret;
+@@ -861,7 +872,12 @@ static int update_monitor_config(struct mt_ctx *ctx)
+     struct mt_svc *cur_svc;
+     struct mt_svc *new_svc;
+     struct sss_domain_info *dom, *new_dom;
+-    struct mt_ctx *new_config = talloc_zero(NULL, struct mt_ctx);
++    struct mt_ctx *new_config;
++
++    new_config = talloc_zero(NULL, struct mt_ctx);
++    if(!new_config) {
++        return ENOMEM;
++    }
+ 
+     new_config->ev = ctx->ev;
+     new_config->cdb = ctx->cdb;
+@@ -953,8 +969,9 @@ static int update_monitor_config(struct mt_ctx *ctx)
+     }
+ 
+     /* Replace the old service list with the new one */
+-    talloc_free(ctx->services);
+-    ctx->services = talloc_steal(ctx, new_config->services);
++    talloc_free(ctx->service_ctx);
++    ctx->service_ctx = talloc_steal(ctx, new_config->service_ctx);
++    ctx->services = new_config->services;
+ 
+     /* Compare data providers */
+     /* Have any providers been disabled? */
+@@ -1040,8 +1057,9 @@ static int update_monitor_config(struct mt_ctx *ctx)
+     }
+ 
+     /* Replace the old domain list with the new one */
+-    talloc_free(ctx->domains);
+-    ctx->domains = talloc_steal(ctx, new_config->domains);
++    talloc_free(ctx->domain_ctx);
++    ctx->domain_ctx = talloc_steal(ctx, new_config->domain_ctx);
++    ctx->domains = new_config->domains;
+ 
+     /* Signal all services to reload their configuration */
+     for(cur_svc = ctx->svc_list; cur_svc; cur_svc = cur_svc->next) {
+-- 
+1.6.2.2
+

sssd-0.4.1-reload_conf_2.patch

@@ -0,0 +1,36 @@
+From 12cbba5545aefa59e27f683e17e05b8e80063718 Mon Sep 17 00:00:00 2001
+From: Stephen Gallagher <sgallagh@redhat.com>
+Date: Fri, 19 Jun 2009 11:28:49 -0400
+Subject: [PATCH] Protect against segfault in service_signal_reload
+
+There is a potential race condition where the monitor may attempt
+to signal a reload of a child process before the communication
+sbus channel is available. If this happens, we will just exit this
+function and let the monitor kill and restart the child process.
+---
+ server/monitor/monitor.c |    9 +++++++++
+ 1 files changed, 9 insertions(+), 0 deletions(-)
+
+diff --git a/server/monitor/monitor.c b/server/monitor/monitor.c
+index e4fca25..5cc73c8 100644
+--- a/server/monitor/monitor.c
++++ b/server/monitor/monitor.c
+@@ -525,6 +525,15 @@ static int service_signal_reload(struct mt_svc *svc)
+         return EOK;
+     }
+ 
++    if (!svc->mt_conn) {
++        /* Avoid a race condition where we are trying to
++         * order a service to reload that hasn't started
++         * yet.
++         */
++        DEBUG(1,("Could not reload service [%s].\n", svc->name));
++        return EIO;
++    }
++
+     conn = sbus_get_connection(svc->mt_conn->conn_ctx);
+     msg = dbus_message_new_method_call(NULL,
+                                        SERVICE_PATH,
+-- 
+1.6.2.2
+

sssd.spec

@@ -1,6 +1,6 @@
 Name: sssd
 Version: 0.4.1
-Release: 1%{?dist}
+Release: 2%{?dist}
 Group: Applications/System
 Summary: System Security Services Daemon
 
@@ -14,6 +14,8 @@ BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
 ### Patches ###
 Patch010: sssd-0.4.1-debug_fn.patch
 Patch011: sssd-0.4.1-conf_check.patch
+Patch012: sssd-0.4.1-reload_conf.patch
+Patch013: sssd-0.4.1-reload_conf_2.patch
 
 ### Dependencies ###
 
@@ -58,6 +60,8 @@ services for projects like FreeIPA.
 
 %patch010 -p1 -b .debug_fn
 %patch011 -p1 -b .conf_check
+%patch012 -p1 -b .reload_conf
+%patch013 -p1 -b .reload_conf_2
 
 %build
 %configure \
@@ -131,6 +135,9 @@ if [ $1 -ge 1 ] ; then
 fi
 
 %changelog
+* Mon Jun 22 2009 Simo Sorce <ssorce@redhat.com> - 0.4.1-2
+- Fix a couple of segfaults that may happen on reload
+
 * Thu Jun 11 2009 Simo Sorce <ssorce@redhat.com> - 0.4.1-1
 - add missing configure check that broke stopping the daemon
 - also fix default config to add a missing required option