-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 dbus_connection_send_with_reply() will report success and return a NULL pending_reply when the connection is not open for communication. This patch creates a new wrapper around dbus_connection_send_with_reply() to properly detect this condition and report it as an error.
- -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkuaV0sACgkQeiVVYja6o6PUVQCeN95bAp2YVyarktf13sbWeCXE 6GgAnjg1W5yLroVgFYstL1Wm5pF/srNV =H5Ss -----END PGP SIGNATURE-----
From 6a70d89febf7623c0fa968aa9e6d02f87c049e67 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <[email protected]> Date: Wed, 10 Mar 2010 15:27:59 -0500 Subject: [PATCH] Properly handle dbus send attempts on a closed connection dbus_connection_send_with_reply() will report success and return a NULL pending_reply when the connection is not open for communication. This patch creates a new wrapper around dbus_connection_send_with_reply() to properly detect this condition and report it as an error. --- src/monitor/monitor.c | 54 +++++------------------------ src/monitor/monitor_sbus.c | 25 ++------------ src/providers/dp_auth_util.c | 23 +------------ src/responder/common/responder_dp.c | 32 +++++------------ src/responder/pam/pamsrv_dp.c | 27 +++------------ src/sbus/sssd_dbus.h | 16 ++++++++ src/sbus/sssd_dbus_connection.c | 65 +++++++++++++++++++++++++++++++++++ 7 files changed, 109 insertions(+), 133 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 56a74fb..849697e 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -650,16 +650,16 @@ static void reload_reply(DBusPendingCall *pending, void *data) */ DEBUG(0, ("A reply callback was called but no reply was received" " and no timeout occurred\n")); - /* Destroy this connection */ sbus_disconnect(svc->conn); - goto done; + dbus_pending_call_unref(pending); + return; } /* TODO: Handle cases where the call has timed out or returned * with an error. */ -done: + dbus_pending_call_unref(pending); dbus_message_unref(reply); } @@ -687,9 +687,6 @@ static int monitor_update_resolv(struct config_file_ctx *file_ctx, static int service_signal(struct mt_svc *svc, const char *svc_signal) { DBusMessage *msg; - dbus_bool_t dbret; - DBusConnection *dbus_conn; - DBusPendingCall *pending_reply; if (svc->provider && strcasecmp(svc->provider, "local") == 0) { /* The local provider requires no signaling */ @@ -705,7 +702,6 @@ static int service_signal(struct mt_svc *svc, const char *svc_signal) return EIO; } - dbus_conn = sbus_get_connection(svc->conn); msg = dbus_message_new_method_call(NULL, MONITOR_PATH, MONITOR_INTERFACE, @@ -717,21 +713,9 @@ static int service_signal(struct mt_svc *svc, const char *svc_signal) return ENOMEM; } - dbret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply, - svc->mt_ctx->service_id_timeout); - if (!dbret || pending_reply == NULL) { - DEBUG(0, ("D-BUS send failed.\n")); - dbus_message_unref(msg); - monitor_kill_service(svc); - talloc_free(svc); - return EIO; - } - - /* Set up the reply handler */ - dbus_pending_call_set_notify(pending_reply, reload_reply, svc, NULL); - dbus_message_unref(msg); - - return EOK; + return sbus_conn_send(svc->conn, msg, + svc->mt_ctx->service_id_timeout, + reload_reply, svc, NULL); } static int service_signal_dns_reload(struct mt_svc *svc) @@ -1859,9 +1843,6 @@ static int monitor_service_init(struct sbus_connection *conn, void *data) static int service_send_ping(struct mt_svc *svc) { DBusMessage *msg; - DBusPendingCall *pending_reply; - DBusConnection *dbus_conn; - dbus_bool_t dbret; if (!svc->conn) { DEBUG(8, ("Service not yet initialized\n")); @@ -1870,8 +1851,6 @@ static int service_send_ping(struct mt_svc *svc) DEBUG(4,("Pinging %s\n", svc->name)); - dbus_conn = sbus_get_connection(svc->conn); - /* * Set up identity request * This should be a well-known path and method @@ -1887,24 +1866,9 @@ static int service_send_ping(struct mt_svc *svc) return ENOMEM; } - dbret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply, - svc->mt_ctx->service_id_timeout); - if (!dbret || pending_reply == NULL) { - /* - * Critical Failure - * We can't communicate on this connection - * We'll drop it using the default destructor. - */ - DEBUG(0, ("D-BUS send failed.\n")); - talloc_zfree(svc->conn); - return EIO; - } - - /* Set up the reply handler */ - dbus_pending_call_set_notify(pending_reply, ping_check, svc, NULL); - dbus_message_unref(msg); - - return EOK; + return sbus_conn_send(svc->conn, msg, + svc->mt_ctx->service_id_timeout, + ping_check, svc, NULL); } static void ping_check(DBusPendingCall *pending, void *data) diff --git a/src/monitor/monitor_sbus.c b/src/monitor/monitor_sbus.c index d60a087..eedb60b 100644 --- a/src/monitor/monitor_sbus.c +++ b/src/monitor/monitor_sbus.c @@ -109,13 +109,9 @@ done: int monitor_common_send_id(struct sbus_connection *conn, const char *name, uint16_t version) { - DBusPendingCall *pending_reply; - DBusConnection *dbus_conn; DBusMessage *msg; dbus_bool_t ret; - dbus_conn = sbus_get_connection(conn); - /* create the message */ msg = dbus_message_new_method_call(NULL, MON_SRV_PATH, @@ -137,24 +133,9 @@ int monitor_common_send_id(struct sbus_connection *conn, return EIO; } - ret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply, - 30000 /* TODO: set timeout */); - if (!ret || !pending_reply) { - /* - * Critical Failure - * We can't communicate on this connection - * We'll drop it using the default destructor. - */ - DEBUG(0, ("D-BUS send failed.\n")); - dbus_message_unref(msg); - return EIO; - } - - /* Set up the reply handler */ - dbus_pending_call_set_notify(pending_reply, id_callback, NULL, NULL); - dbus_message_unref(msg); - - return EOK; + return sbus_conn_send(conn, msg, 3000, + id_callback, + NULL, NULL); } int monitor_common_pong(DBusMessage *message, diff --git a/src/providers/dp_auth_util.c b/src/providers/dp_auth_util.c index 5291176..e78884a 100644 --- a/src/providers/dp_auth_util.c +++ b/src/providers/dp_auth_util.c @@ -336,13 +336,9 @@ done: int dp_common_send_id(struct sbus_connection *conn, uint16_t version, const char *name) { - DBusPendingCall *pending_reply; - DBusConnection *dbus_conn; DBusMessage *msg; dbus_bool_t ret; - dbus_conn = sbus_get_connection(conn); - /* create the message */ msg = dbus_message_new_method_call(NULL, DP_PATH, @@ -365,23 +361,6 @@ int dp_common_send_id(struct sbus_connection *conn, uint16_t version, return EIO; } - ret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply, - 30000 /* TODO: set timeout */); - if (!ret || !pending_reply) { - /* - * Critical Failure - * We can't communicate on this connection - * We'll drop it using the default destructor. - */ - DEBUG(0, ("D-BUS send failed.\n")); - dbus_message_unref(msg); - return EIO; - } - - /* Set up the reply handler */ - dbus_pending_call_set_notify(pending_reply, id_callback, NULL, NULL); - dbus_message_unref(msg); - - return EOK; + return sbus_conn_send(conn, msg, 30000, id_callback, NULL, NULL); } diff --git a/src/responder/common/responder_dp.c b/src/responder/common/responder_dp.c index d5a4ca7..001661c 100644 --- a/src/responder/common/responder_dp.c +++ b/src/responder/common/responder_dp.c @@ -412,7 +412,6 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx, void *callback_ctx, struct sss_dp_req **ndp) { - DBusConnection *dbus_conn; DBusMessage *msg; DBusPendingCall *pending_reply; dbus_bool_t dbret; @@ -432,7 +431,6 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx, " This maybe a bug, it shouldn't happen!\n", domain)); return EIO; } - dbus_conn = sbus_get_connection(be_conn->conn); /* create the message */ msg = dbus_message_new_method_call(NULL, @@ -457,9 +455,16 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx, return EIO; } - dbret = dbus_connection_send_with_reply(dbus_conn, msg, - &pending_reply, timeout); - if (!dbret || pending_reply == NULL) { + sdp_req = talloc_zero(rctx, struct sss_dp_req); + if (!sdp_req) { + dbus_message_unref(msg); + return ENOMEM; + } + + ret = sbus_conn_send(be_conn->conn, msg, timeout, + sss_dp_send_acct_callback, + sdp_req, &pending_reply); + if (ret != EOK) { /* * Critical Failure * We can't communicate on this connection @@ -470,11 +475,6 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx, return EIO; } - sdp_req = talloc_zero(rctx, struct sss_dp_req); - if (!sdp_req) { - dbus_message_unref(msg); - return ENOMEM; - } sdp_req->ev = rctx->ev; sdp_req->pending_reply = pending_reply; @@ -493,18 +493,6 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx, talloc_set_destructor((TALLOC_CTX *)cb, sss_dp_callback_destructor); } - /* Set up the reply handler */ - dbret = dbus_pending_call_set_notify(pending_reply, - sss_dp_send_acct_callback, - sdp_req, NULL); - if (!dbret) { - DEBUG(0, ("Could not queue up pending request!\n")); - talloc_zfree(sdp_req); - dbus_pending_call_cancel(pending_reply); - dbus_message_unref(msg); - return EIO; - } - dbus_message_unref(msg); *ndp = sdp_req; diff --git a/src/responder/pam/pamsrv_dp.c b/src/responder/pam/pamsrv_dp.c index 071d09b..d9431f2 100644 --- a/src/responder/pam/pamsrv_dp.c +++ b/src/responder/pam/pamsrv_dp.c @@ -43,10 +43,10 @@ static void pam_dp_process_reply(DBusPendingCall *pending, void *ptr) dbus_error_init(&dbus_error); - dbus_pending_call_block(pending); msg = dbus_pending_call_steal_reply(pending); if (msg == NULL) { - DEBUG(0, ("Severe error. A reply callback was called but no reply was received and no timeout occurred\n")); + DEBUG(0, ("Severe error. A reply callback was called but no reply was" + "received and no timeout occurred\n")); preq->pd->pam_status = PAM_SYSTEM_ERR; goto done; } @@ -84,8 +84,6 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout) struct pam_data *pd = preq->pd; struct be_conn *be_conn; DBusMessage *msg; - DBusPendingCall *pending_reply; - DBusConnection *dbus_conn; dbus_bool_t ret; int res; @@ -100,7 +98,6 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout) " This maybe a bug, it shouldn't happen!\n", preq->domain)); return EIO; } - dbus_conn = sbus_get_connection(be_conn->conn); msg = dbus_message_new_method_call(NULL, DP_PATH, @@ -121,22 +118,8 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout) return EIO; } - ret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply, timeout); - if (!ret || pending_reply == NULL) { - /* - * Critical Failure - * We can't communicate on this connection - * We'll drop it using the default destructor. - */ - DEBUG(0, ("D-BUS send failed.\n")); - dbus_message_unref(msg); - return EIO; - } - - dbus_pending_call_set_notify(pending_reply, - pam_dp_process_reply, preq, NULL); - dbus_message_unref(msg); - - return EOK; + return sbus_conn_send(be_conn->conn, msg, + timeout, pam_dp_process_reply, + preq, NULL); } diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h index ac02c44..2dbf4ab 100644 --- a/src/sbus/sssd_dbus.h +++ b/src/sbus/sssd_dbus.h @@ -144,6 +144,22 @@ DBusHandlerResult sbus_message_handler(DBusConnection *conn, DBusMessage *message, void *user_data); +/* + * Send a message across the SBUS + * If requested, the DBusPendingCall object will + * be returned to the caller. + * + * This function will return EAGAIN in the event + * that the connection is not open for + * communication. + */ +int sbus_conn_send(struct sbus_connection *conn, + DBusMessage *msg, + int timeout_ms, + DBusPendingCallNotifyFunction reply_handler, + void *pvt, + DBusPendingCall **pending); + void sbus_conn_send_reply(struct sbus_connection *conn, DBusMessage *reply); diff --git a/src/sbus/sssd_dbus_connection.c b/src/sbus/sssd_dbus_connection.c index 38ccc6a..d2918fb 100644 --- a/src/sbus/sssd_dbus_connection.c +++ b/src/sbus/sssd_dbus_connection.c @@ -685,6 +685,71 @@ bool sbus_conn_disconnecting(struct sbus_connection *conn) return false; } +/* + * Send a message across the SBUS + * If requested, the DBusPendingCall object will + * be returned to the caller. + * + * This function will return EAGAIN in the event + * that the connection is not open for + * communication. + */ +int sbus_conn_send(struct sbus_connection *conn, + DBusMessage *msg, + int timeout_ms, + DBusPendingCallNotifyFunction reply_handler, + void *pvt, + DBusPendingCall **pending) +{ + DBusPendingCall *pending_reply; + DBusConnection *dbus_conn; + dbus_bool_t dbret; + + dbus_conn = sbus_get_connection(conn); + + dbret = dbus_connection_send_with_reply(dbus_conn, msg, + &pending_reply, + timeout_ms); + if (!dbret) { + /* + * Critical Failure + * Insufficient memory to send message + */ + DEBUG(0, ("D-BUS send failed.\n")); + return ENOMEM; + } + + if (pending_reply) { + /* Set up the reply handler */ + dbret = dbus_pending_call_set_notify(pending_reply, reply_handler, + pvt, NULL); + if (!dbret) { + /* + * Critical Failure + * Insufficient memory to create pending call notify + */ + DEBUG(0, ("D-BUS send failed.\n")); + dbus_pending_call_cancel(pending_reply); + dbus_pending_call_unref(pending_reply); + return ENOMEM; + } + + if(pending) { + *pending = pending_reply; + } + return EOK; + } + + /* If pending_reply is NULL, the connection was not + * open for sending. + */ + + /* TODO: Create a callback into the reconnection logic so this + * request is invoked when the connection is re-established + */ + return EAGAIN; +} + void sbus_conn_send_reply(struct sbus_connection *conn, DBusMessage *reply) { dbus_connection_send(conn->dbus.conn, reply, NULL); -- 1.6.6.1
0001-Properly-handle-dbus-send-attempts-on-a-closed-conne.patch.sig
Description: PGP signature
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
