[SSSD] [PATCH] Guard against segfault in ping_check()
Jakub Hrozek
jhrozek at redhat.com
Tue Dec 13 18:46:34 UTC 2011
On Tue, Dec 13, 2011 at 10:18:10AM -0500, Stephen Gallagher wrote:
> On Tue, 2011-12-13 at 16:01 +0100, Jakub Hrozek wrote:
> > https://fedorahosted.org/sssd/ticket/1090
> >
> > Unfortunately there is not enough info in rhbz #754114 where the bug
> > originated, so just band-aid the crash for now.
>
>
> Nack.
>
> Looking at this closely, I think what probably happened is that the
> child died while waiting for the ping_check to time out, so that when it
> finally returned, the memory for the mt_svc object had been freed.
> Because we then use talloc_get_type() to retrieve that memory, it
> detects that it's invalid and simply returns NULL (which we then
> dereference and crash on).
>
> The real solution here would be to ensure that we cancel the
> DBusPendingCall with dbus_pending_call_cancel() if the mt_svc is freed.
>
> So we need to modify the mt_svc structure to contain a DBusPendingCall *
> attribute that we assign to it when service_send_ping() calls
> sbus_conn_send() (currently, we just pass NULL for the 'pending' return
> to ignore it).
>
> Then, we need to make sure that the mt_svc destructor will call
> dbus_pending_call_cancel() on mt_svc->pending, which will prevent us
> from reaching the ping_check() function.
>
> (Obviously, we need to NULLify the mt_svc->pending attribute once we
> reach the ping_check() function correctly).
>
> There's certainly no reason we can't ALSO include the band-aid of this
> patch, but I'd rather fix the underlying problem.
Thank you for the detailed explanation. A new patch is attached.
-------------- next part --------------
From 6e7ead1b3fe85786f36de7bcb4d9c68104213d05 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 13 Dec 2011 15:41:09 +0100
Subject: [PATCH] Cancel ping_check if service goes away
https://fedorahosted.org/sssd/ticket/1090
---
src/monitor/monitor.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index a0a0cc2f7a153b2b61cfb2a26939f310d5f5d704..61786ea5edc0f3474592a2c540c59a9c6d0b58d3 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -91,6 +91,7 @@ struct mt_svc {
int restarts;
time_t last_restart;
int failed_pongs;
+ DBusPendingCall *pending;
int debug_level;
@@ -315,6 +316,11 @@ static int svc_destructor(void *mem)
DLIST_REMOVE(svc->mt_ctx->svc_list, svc);
}
+ /* Cancel any pending pings */
+ if (svc->pending) {
+ dbus_pending_call_cancel(svc->pending);
+ }
+
/* svc is beeing freed, neutralize the spy */
if (svc->conn_spy) {
talloc_set_destructor((TALLOC_CTX *)svc->conn_spy, NULL);
@@ -2190,7 +2196,7 @@ static int service_send_ping(struct mt_svc *svc)
ret = sbus_conn_send(svc->conn, msg,
svc->ping_time * 1000, /* milliseconds */
- ping_check, svc, NULL);
+ ping_check, svc, &svc->pending);
dbus_message_unref(msg);
return ret;
}
@@ -2204,6 +2210,13 @@ static void ping_check(DBusPendingCall *pending, void *data)
int type;
svc = talloc_get_type(data, struct mt_svc);
+ if (!svc) {
+ /* The connection probably went down before the callback fired.
+ * Not much we can do. */
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Invalid service pointer.\n"));
+ return;
+ }
+ svc->pending = NULL;
reply = dbus_pending_call_steal_reply(pending);
if (!reply) {
--
1.7.7.4
More information about the sssd-devel
mailing list