From 7522c31552c81a04bf889855671b724d76cd1050 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Jun 22 2009 14:49:00 +0000 Subject: - Fix a couple of segfaults that may happen on reload --- diff --git a/sssd-0.4.1-reload_conf.patch b/sssd-0.4.1-reload_conf.patch new file mode 100644 index 0000000..6bfa9e2 --- /dev/null +++ b/sssd-0.4.1-reload_conf.patch @@ -0,0 +1,113 @@ +From 673c2ce9b3371241de872b2bd206f732485888cb Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +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 + diff --git a/sssd-0.4.1-reload_conf_2.patch b/sssd-0.4.1-reload_conf_2.patch new file mode 100644 index 0000000..136e2a9 --- /dev/null +++ b/sssd-0.4.1-reload_conf_2.patch @@ -0,0 +1,36 @@ +From 12cbba5545aefa59e27f683e17e05b8e80063718 Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +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 + diff --git a/sssd.spec b/sssd.spec index d1d5817..4c0ef18 100644 --- a/sssd.spec +++ b/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 - 0.4.1-2 +- Fix a couple of segfaults that may happen on reload + * Thu Jun 11 2009 Simo Sorce - 0.4.1-1 - add missing configure check that broke stopping the daemon - also fix default config to add a missing required option