- Resolves: upstream#3821 - crash related to sbus_router_destructor() - Resolves: upstream#3810 - sbus2: fix memory leak in sbus_message_bound_ref - Resolves: upstream#3819 - sssd only sets the SELinux login context if it differs from the default - Resolves: upstream#3807 - The sbus codegen script relies on "python" which might not be available on all distributions - Resolves: upstream#3820 - sudo: search with lower cased name for case insensitive domains - Resolves: upstream#3701 - [RFE] Allow changing default behavior of SSSD from an allow-any default to a deny-any default when it can't find any GPOs to apply to a user login. - Resolves: upstream#3828 - Invalid domain provider causes SSSD to abort startup - Resolves: upstream#3500 - Make sure sssd is a replacement for pam_pkcs11 also for local account authentication - Resolves: upstream#3812 - sssd 2.0.0 segfaults on startup - Resolves: upstream#3826 - Remove references of sss_user/group/add/del commands in man pages since local provider is deprecated - Resolves: upstream#3827 - SSSD should log to syslog if a domain is not started due to a misconfiguration - Resolves: upstream#3830 - Printing incorrect information about domain with sssctl utility - Resolves: upstream#3489 - p11_child should work wit openssl1.0+ - Resolves: upstream#3750 - [RFE] man 5 sssd-files should mention necessary changes in nsswitch.conf - Resovles: upstream#3650 - RFE: Require smartcard authentication - Resolves: upstream#3334 - sssctl config-check does not check any special characters in domain name of domain section - Resolves: upstream#3849 - Files: The files provider always enumerates which causes duplicate when running getent passwd - Related: upstream#3855 - session not recording for local user when groups defined - Resolves: upstream#3802 - Reuse sysdb_error_to_errno() outside sysdb - Related: upstream#3493 - Remove the pysss.local interface
337 lines
13 KiB
Diff
337 lines
13 KiB
Diff
From ca50c40511f08c0f7c786598e5793a06789c6cce Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina@redhat.com>
|
|
Date: Thu, 16 Aug 2018 13:17:13 +0200
|
|
Subject: [PATCH 11/83] sbus: replace sbus_message_bound_ref with
|
|
sbus_message_bound_steal
|
|
|
|
The memory context used to new message reference accidentally overwrote
|
|
the one use by the initial sbus_message_bound call. This caused a memory
|
|
leak of message as its reference counter got increased but number of
|
|
talloc contexts bound this this message decreased at the same time.
|
|
|
|
Fixing this is non-trival and it would require separate data slot for
|
|
each reference. Because we do not have any existing use case for this
|
|
and we use it only as an equivalent of talloc_steal it is better to
|
|
provide a real equivalent for this talloc function.
|
|
|
|
Resolves:
|
|
https://pagure.io/SSSD/sssd/issue/3810
|
|
|
|
Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>
|
|
---
|
|
src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c | 4 +-
|
|
src/sbus/codegen/templates/client_async.c.tpl | 4 +-
|
|
src/sbus/codegen/templates/client_sync.c.tpl | 4 +-
|
|
src/sbus/interface_dbus/sbus_dbus_client_async.c | 8 ++--
|
|
src/sbus/interface_dbus/sbus_dbus_client_sync.c | 8 ++--
|
|
src/sbus/request/sbus_message.c | 51 +++++++++++++++++-----
|
|
src/sbus/request/sbus_request.c | 10 ++---
|
|
src/sbus/request/sbus_request_call.c | 5 +--
|
|
src/sbus/sbus_message.h | 8 +---
|
|
src/sbus/sync/sbus_sync_call.c | 5 +--
|
|
10 files changed, 65 insertions(+), 42 deletions(-)
|
|
|
|
diff --git a/src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c b/src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c
|
|
index 4859b93..1f0a8e3 100644
|
|
--- a/src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c
|
|
+++ b/src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c
|
|
@@ -526,9 +526,9 @@ sbus_method_in_sas_out_raw
|
|
goto done;
|
|
}
|
|
|
|
- ret = sbus_message_bound_ref(mem_ctx, reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
goto done;
|
|
}
|
|
diff --git a/src/sbus/codegen/templates/client_async.c.tpl b/src/sbus/codegen/templates/client_async.c.tpl
|
|
index 6ffb4f8..e16ce42 100644
|
|
--- a/src/sbus/codegen/templates/client_async.c.tpl
|
|
+++ b/src/sbus/codegen/templates/client_async.c.tpl
|
|
@@ -193,9 +193,9 @@
|
|
return EINVAL;
|
|
}
|
|
|
|
- ret = sbus_message_bound_ref(mem_ctx, state->reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, state->reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
return ret;
|
|
}
|
|
diff --git a/src/sbus/codegen/templates/client_sync.c.tpl b/src/sbus/codegen/templates/client_sync.c.tpl
|
|
index 30fa009..fe9a3a4 100644
|
|
--- a/src/sbus/codegen/templates/client_sync.c.tpl
|
|
+++ b/src/sbus/codegen/templates/client_sync.c.tpl
|
|
@@ -110,9 +110,9 @@
|
|
goto done;
|
|
}
|
|
|
|
- ret = sbus_message_bound_ref(mem_ctx, reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
goto done;
|
|
}
|
|
diff --git a/src/sbus/interface_dbus/sbus_dbus_client_async.c b/src/sbus/interface_dbus/sbus_dbus_client_async.c
|
|
index 9dbd72c..0060e8b 100644
|
|
--- a/src/sbus/interface_dbus/sbus_dbus_client_async.c
|
|
+++ b/src/sbus/interface_dbus/sbus_dbus_client_async.c
|
|
@@ -301,9 +301,9 @@ sbus_method_in_s_out_raw_recv
|
|
return EINVAL;
|
|
}
|
|
|
|
- ret = sbus_message_bound_ref(mem_ctx, state->reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, state->reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
return ret;
|
|
}
|
|
@@ -513,9 +513,9 @@ sbus_method_in_ss_out_raw_recv
|
|
return EINVAL;
|
|
}
|
|
|
|
- ret = sbus_message_bound_ref(mem_ctx, state->reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, state->reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
return ret;
|
|
}
|
|
diff --git a/src/sbus/interface_dbus/sbus_dbus_client_sync.c b/src/sbus/interface_dbus/sbus_dbus_client_sync.c
|
|
index a0473cd..3ab0aab 100644
|
|
--- a/src/sbus/interface_dbus/sbus_dbus_client_sync.c
|
|
+++ b/src/sbus/interface_dbus/sbus_dbus_client_sync.c
|
|
@@ -101,9 +101,9 @@ sbus_method_in_s_out_raw
|
|
goto done;
|
|
}
|
|
|
|
- ret = sbus_message_bound_ref(mem_ctx, reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
goto done;
|
|
}
|
|
@@ -159,9 +159,9 @@ sbus_method_in_ss_out_raw
|
|
goto done;
|
|
}
|
|
|
|
- ret = sbus_message_bound_ref(mem_ctx, reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
goto done;
|
|
}
|
|
diff --git a/src/sbus/request/sbus_message.c b/src/sbus/request/sbus_message.c
|
|
index 7314fd7..90c6df4 100644
|
|
--- a/src/sbus/request/sbus_message.c
|
|
+++ b/src/sbus/request/sbus_message.c
|
|
@@ -29,8 +29,9 @@
|
|
#include "sbus/interface/sbus_iterator_writers.h"
|
|
|
|
/* Data slot that is used for message data. The slot is shared for all
|
|
- * messages. */
|
|
-dbus_int32_t data_slot = -1;
|
|
+ * messages, i.e. when a data slot is allocated all messages have the
|
|
+ * slot available. */
|
|
+dbus_int32_t global_data_slot = -1;
|
|
|
|
struct sbus_talloc_msg {
|
|
DBusMessage *msg;
|
|
@@ -48,7 +49,7 @@ static int sbus_talloc_msg_destructor(struct sbus_talloc_msg *talloc_msg)
|
|
/* There may exist more references to this message but this talloc
|
|
* context is no longer valid. We remove dbus message data to invoke
|
|
* dbus destructor now. */
|
|
- dbus_message_set_data(talloc_msg->msg, data_slot, NULL, NULL);
|
|
+ dbus_message_set_data(talloc_msg->msg, global_data_slot, NULL, NULL);
|
|
dbus_message_unref(talloc_msg->msg);
|
|
return 0;
|
|
}
|
|
@@ -60,7 +61,7 @@ static void sbus_msg_data_destructor(void *ctx)
|
|
talloc_msg = talloc_get_type(ctx, struct sbus_talloc_msg);
|
|
|
|
/* Decrement ref counter on data slot. */
|
|
- dbus_message_free_data_slot(&data_slot);
|
|
+ dbus_message_free_data_slot(&global_data_slot);
|
|
|
|
if (!talloc_msg->in_talloc_destructor) {
|
|
/* References to this message dropped to zero but through
|
|
@@ -100,7 +101,8 @@ sbus_message_bound(TALLOC_CTX *mem_ctx, DBusMessage *msg)
|
|
/* Allocate a dbus message data slot that will contain pointer to the
|
|
* talloc context so we can pick up cases when the dbus message is
|
|
* freed through dbus api. */
|
|
- bret = dbus_message_allocate_data_slot(&data_slot);
|
|
+
|
|
+ bret = dbus_message_allocate_data_slot(&global_data_slot);
|
|
if (!bret) {
|
|
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to allocate data slot!\n");
|
|
talloc_free(talloc_msg);
|
|
@@ -108,11 +110,11 @@ sbus_message_bound(TALLOC_CTX *mem_ctx, DBusMessage *msg)
|
|
}
|
|
|
|
free_fn = sbus_msg_data_destructor;
|
|
- bret = dbus_message_set_data(msg, data_slot, talloc_msg, free_fn);
|
|
+ bret = dbus_message_set_data(msg, global_data_slot, talloc_msg, free_fn);
|
|
if (!bret) {
|
|
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to set message data!\n");
|
|
talloc_free(talloc_msg);
|
|
- dbus_message_free_data_slot(&data_slot);
|
|
+ dbus_message_free_data_slot(&global_data_slot);
|
|
return ENOMEM;
|
|
}
|
|
|
|
@@ -125,15 +127,44 @@ sbus_message_bound(TALLOC_CTX *mem_ctx, DBusMessage *msg)
|
|
}
|
|
|
|
errno_t
|
|
-sbus_message_bound_ref(TALLOC_CTX *mem_ctx, DBusMessage *msg)
|
|
+sbus_message_bound_steal(TALLOC_CTX *mem_ctx, DBusMessage *msg)
|
|
{
|
|
+ struct sbus_talloc_msg *talloc_msg;
|
|
+ void *data;
|
|
+
|
|
+ if (mem_ctx == NULL) {
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Warning: bounding to NULL context!\n");
|
|
+ return EINVAL;
|
|
+ }
|
|
+
|
|
if (msg == NULL) {
|
|
DEBUG(SSSDBG_CRIT_FAILURE, "Message can not be NULL!\n");
|
|
return EINVAL;
|
|
}
|
|
|
|
- dbus_message_ref(msg);
|
|
- return sbus_message_bound(mem_ctx, msg);
|
|
+ if (global_data_slot < 0) {
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "This message is not talloc-bound! "
|
|
+ "(data slot < 0)\n");
|
|
+ return ERR_INTERNAL;
|
|
+ }
|
|
+
|
|
+ data = dbus_message_get_data(msg, global_data_slot);
|
|
+ if (data == NULL) {
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "This message is not talloc-bound! "
|
|
+ "(returned data is NULL)\n");
|
|
+ return ERR_INTERNAL;
|
|
+ }
|
|
+
|
|
+ talloc_msg = talloc_get_type(data, struct sbus_talloc_msg);
|
|
+ if (talloc_msg == NULL) {
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "This message is not talloc-bound! "
|
|
+ "(invalid data)\n");
|
|
+ return ERR_INTERNAL;
|
|
+ }
|
|
+
|
|
+ talloc_steal(mem_ctx, talloc_msg);
|
|
+
|
|
+ return EOK;
|
|
}
|
|
|
|
DBusMessage *
|
|
diff --git a/src/sbus/request/sbus_request.c b/src/sbus/request/sbus_request.c
|
|
index 3d0e2f9..1ccd01e 100644
|
|
--- a/src/sbus/request/sbus_request.c
|
|
+++ b/src/sbus/request/sbus_request.c
|
|
@@ -564,10 +564,9 @@ sbus_incoming_request_recv(TALLOC_CTX *mem_ctx,
|
|
return EOK;
|
|
}
|
|
|
|
- /* Create new reference to the reply and bound it with caller mem_ctx. */
|
|
- ret = sbus_message_bound_ref(mem_ctx, state->reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, state->reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
return ret;
|
|
}
|
|
@@ -709,10 +708,9 @@ sbus_outgoing_request_recv(TALLOC_CTX *mem_ctx,
|
|
|
|
TEVENT_REQ_RETURN_ON_ERROR(req);
|
|
|
|
- /* Create new reference to the reply and bound it with caller mem_ctx. */
|
|
- ret = sbus_message_bound_ref(mem_ctx, state->reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, state->reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
return ret;
|
|
}
|
|
diff --git a/src/sbus/request/sbus_request_call.c b/src/sbus/request/sbus_request_call.c
|
|
index 1cf58bd..cf2a6e5 100644
|
|
--- a/src/sbus/request/sbus_request_call.c
|
|
+++ b/src/sbus/request/sbus_request_call.c
|
|
@@ -126,10 +126,9 @@ sbus_call_method_recv(TALLOC_CTX *mem_ctx,
|
|
|
|
TEVENT_REQ_RETURN_ON_ERROR(req);
|
|
|
|
- /* Create new reference to the reply and bound it with caller mem_ctx. */
|
|
- ret = sbus_message_bound_ref(mem_ctx, state->reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, state->reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
return ret;
|
|
}
|
|
diff --git a/src/sbus/sbus_message.h b/src/sbus/sbus_message.h
|
|
index 92d5cea..e7b8fe5 100644
|
|
--- a/src/sbus/sbus_message.h
|
|
+++ b/src/sbus/sbus_message.h
|
|
@@ -45,11 +45,7 @@ errno_t
|
|
sbus_message_bound(TALLOC_CTX *mem_ctx, DBusMessage *msg);
|
|
|
|
/**
|
|
- * Reference the message and bound it with talloc context.
|
|
- *
|
|
- * DO NOT USE dbus_message_unref() on such message anymore since it would not
|
|
- * release internal data about the bound. The message will be automatically
|
|
- * unreferenced when the talloc context is freed.
|
|
+ * Steal previously bound D-Bus message to a new talloc parent.
|
|
*
|
|
* @param mem_ctx Memory context to bound the message with. It can not be NULL.
|
|
* @param msg Message to be bound with memory context.
|
|
@@ -57,7 +53,7 @@ sbus_message_bound(TALLOC_CTX *mem_ctx, DBusMessage *msg);
|
|
* @return EOK on success, other errno code on error.
|
|
*/
|
|
errno_t
|
|
-sbus_message_bound_ref(TALLOC_CTX *mem_ctx, DBusMessage *msg);
|
|
+sbus_message_bound_steal(TALLOC_CTX *mem_ctx, DBusMessage *msg);
|
|
|
|
/**
|
|
* Create an empty D-Bus method call.
|
|
diff --git a/src/sbus/sync/sbus_sync_call.c b/src/sbus/sync/sbus_sync_call.c
|
|
index 8549e58..a4f8a5c 100644
|
|
--- a/src/sbus/sync/sbus_sync_call.c
|
|
+++ b/src/sbus/sync/sbus_sync_call.c
|
|
@@ -63,10 +63,9 @@ sbus_sync_call_method(TALLOC_CTX *mem_ctx,
|
|
goto done;
|
|
}
|
|
|
|
- /* Create new reference to the reply and bound it with caller mem_ctx. */
|
|
- ret = sbus_message_bound_ref(mem_ctx, reply);
|
|
+ ret = sbus_message_bound_steal(mem_ctx, reply);
|
|
if (ret != EOK) {
|
|
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to bound message [%d]: %s\n",
|
|
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to steal message [%d]: %s\n",
|
|
ret, sss_strerror(ret));
|
|
goto done;
|
|
}
|
|
--
|
|
2.9.5
|
|
|