packages/sssd
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix the sssd_be coredump

Browse Source
This commit is contained in:
zhouchenchen123 2022-12-20 19:47:39 +08:00
parent fcb0db9f44
commit f03dc0439c
3 changed files with 287 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
backport-be-remove-accidental-sleep.patch Normal file
View File

@ -0,0 +1,32 @@
From 7fbcaa8feeb968711ff52f51705c45062fd81394 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina@redhat.com>
Date: Thu, 1 Oct 2020 15:45:47 +0200
Subject: [PATCH] be: remove accidental sleep
This sleep was used to test a crash in data provider and quite unfortunately
it was left in the patch.
dp: fix potential race condition in provider's sbus server
4a84f8e18ea5604ac7e69849dee492718fd96296.
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
---
src/providers/data_provider_be.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 74df62e24..4c10d6b48 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -690,8 +690,6 @@ int main(int argc, const char *argv[])
uid_t uid;
gid_t gid;
- sleep(5);
-
struct poptOption long_options[] = {
POPT_AUTOHELP
SSSD_MAIN_OPTS
--
2.32.0.windows.1

248
backport-dp-fix-potential-race-condition-in-provider-s-sbus-s.patch Normal file
View File

@ -0,0 +1,248 @@
From 4a84f8e18ea5604ac7e69849dee492718fd96296 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina@redhat.com>
Date: Wed, 26 Aug 2020 12:51:49 +0200
Subject: [PATCH] dp: fix potential race condition in provider's sbus server
We can hit a segfault if provider start is somehow delayed.
- dp_init_send
- sbus_server_create_and_connect_send
- sbus_server_create (*)
- dp_init_done (callback for sbus_server_create_and_connect_send)
- sbus_server_create_and_connect_recv
- sbus_server_set_on_connection (sets clients data and creates dp_cli)
At (*) sbus server is already created and accepts new connections once
we get into tevent loop. So it is possible that the client connects to
server before sbus_server_set_on_connection is called and thus the client
is not properly initialized. However it should not happen in normal start
because providers are started before responders and it can happen only if
data provider startup is somehow delay.
You can use this diff to reproduce the crash:
```diff
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
---
src/monitor/monitor.c | 3 ++-
src/providers/data_provider/dp.c | 9 +++-----
src/providers/data_provider/dp_client.c | 21 +++++++++++++++----
src/providers/data_provider_be.c | 2 ++
src/sbus/connection/sbus_connection_connect.c | 7 +++++--
src/sbus/sbus.h | 15 +++++++++++--
src/sbus/sbus_private.h | 3 ---
src/sbus/server/sbus_server.c | 9 +++++++-
8 files changed, 50 insertions(+), 19 deletions(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 1e94a8f36..d9da05a51 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2008,7 +2008,8 @@ static int monitor_process_init(struct mt_ctx *ctx,
req = sbus_server_create_and_connect_send(ctx, ctx->ev, SSS_BUS_MONITOR,
NULL, SSS_MONITOR_ADDRESS,
- false, 100, ctx->uid, ctx->gid);
+ false, 100, ctx->uid, ctx->gid,
+ NULL, NULL);
if (req == NULL) {
ret = ENOMEM;
goto done;
diff --git a/src/providers/data_provider/dp.c b/src/providers/data_provider/dp.c
index ba1cfec8a..0858c43d2 100644
--- a/src/providers/data_provider/dp.c
+++ b/src/providers/data_provider/dp.c
@@ -192,9 +192,9 @@ dp_init_send(TALLOC_CTX *mem_ctx,
talloc_set_destructor(state->provider, dp_destructor);
subreq = sbus_server_create_and_connect_send(state->provider, ev,
- state->sbus_name,
- NULL, sbus_address, true, 1000,
- uid, gid);
+ state->sbus_name, NULL, sbus_address, true, 1000, uid, gid,
+ (sbus_server_on_connection_cb)dp_client_init,
+ (sbus_server_on_connection_data)state->provider);
if (subreq == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create subrequest!\n");
ret = ENOMEM;
@@ -235,9 +235,6 @@ static void dp_init_done(struct tevent_req *subreq)
return;
}
- sbus_server_set_on_connection(state->provider->sbus_server,
- dp_client_init, state->provider);
-
/* be_ctx->provider must be accessible from modules and targets */
state->be_ctx->provider = talloc_steal(state->be_ctx, state->provider);
diff --git a/src/providers/data_provider/dp_client.c b/src/providers/data_provider/dp_client.c
index 01baf01db..dcf939e57 100644
--- a/src/providers/data_provider/dp_client.c
+++ b/src/providers/data_provider/dp_client.c
@@ -140,15 +140,28 @@ dp_client_handshake_timeout(struct tevent_context *ev,
{
struct sbus_connection *conn;
struct dp_client *dp_cli;
-
- DEBUG(SSSDBG_OP_FAILURE,
- "Client timed out before identification [%p]!\n", te);
+ const char *be_name;
+ const char *name;
dp_cli = talloc_get_type(ptr, struct dp_client);
+ conn = dp_cli->conn;
+ be_name = dp_cli->provider->be_ctx->sbus_name;
talloc_set_destructor(dp_cli, NULL);
- conn = dp_cli->conn;
+ name = sbus_connection_get_name(dp_cli->conn);
+ if (name != NULL && strcmp(name, be_name) == 0) {
+ /* This is the data provider connection. Just free the client record
+ * but keep the connection opened. */
+ talloc_zfree(dp_cli);
+ return;
+ }
+
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Client [%s] timed out before identification [%p]!\n",
+ name == NULL ? "unknown" : name, te);
+
+ /* Kill the connection. */
talloc_zfree(dp_cli);
talloc_zfree(conn);
}
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 95665332a..ca2d51650 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -665,6 +665,8 @@ int main(int argc, const char *argv[])
uid_t uid;
gid_t gid;
+ sleep(5);
+
struct poptOption long_options[] = {
POPT_AUTOHELP
SSSD_MAIN_OPTS
diff --git a/src/sbus/connection/sbus_connection_connect.c b/src/sbus/connection/sbus_connection_connect.c
index 3f8702f0b..9cfe86206 100644
--- a/src/sbus/connection/sbus_connection_connect.c
+++ b/src/sbus/connection/sbus_connection_connect.c
@@ -344,7 +344,9 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx,
bool use_symlink,
uint32_t max_connections,
uid_t uid,
- gid_t gid)
+ gid_t gid,
+ sbus_server_on_connection_cb on_conn_cb,
+ sbus_server_on_connection_data on_conn_data)
{
struct sbus_server_create_and_connect_state *state;
struct tevent_req *subreq;
@@ -358,7 +360,8 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx,
}
state->server = sbus_server_create(state, ev, address, use_symlink,
- max_connections, uid, gid);
+ max_connections, uid, gid,
+ on_conn_cb, on_conn_data);
if (state->server == NULL) {
ret = ENOMEM;
goto done;
diff --git a/src/sbus/sbus.h b/src/sbus/sbus.h
index 9136c4e4a..0983879f0 100644
--- a/src/sbus/sbus.h
+++ b/src/sbus/sbus.h
@@ -138,6 +138,8 @@ errno_t sbus_connect_private_recv(TALLOC_CTX *mem_ctx,
* @param use_symlink If a symlink to @address should be created.
* @param uid Socket owner uid.
* @param gid Socket owner gid.
+ * @param on_conn_cb On new connection callback function.
+ * @param on_conn_data Private data passed to the callback.
*
* @return New sbus server or NULL on error.
*/
@@ -148,7 +150,9 @@ sbus_server_create(TALLOC_CTX *mem_ctx,
bool use_symlink,
uint32_t max_connections,
uid_t uid,
- gid_t gid);
+ gid_t gid,
+ sbus_server_on_connection_cb on_conn_cb,
+ sbus_server_on_connection_data on_conn_data);
/**
* Create a new sbus server at socket address @address and connect to it.
@@ -162,6 +166,8 @@ sbus_server_create(TALLOC_CTX *mem_ctx,
* @param use_symlink If a symlink to @address should be created.
* @param uid Socket owner uid.
* @param gid Socket owner gid.
+ * @param on_conn_cb On new connection callback function.
+ * @param on_conn_data Private data passed to the callback.
*
* @return Tevent request or NULL on error.
*/
@@ -174,7 +180,9 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx,
bool use_symlink,
uint32_t max_connections,
uid_t uid,
- gid_t gid);
+ gid_t gid,
+ sbus_server_on_connection_cb on_conn_cb,
+ sbus_server_on_connection_data on_conn_data);
/**
* Receive reply from @sbus_server_create_and_connect_send.
@@ -446,4 +454,7 @@ errno_t
sbus_router_add_node_map(struct sbus_connection *conn,
struct sbus_node *map);
+/* Get connection name, well known name is preferred. */
+const char * sbus_connection_get_name(struct sbus_connection *conn);
+
#endif /* _SBUS_H_ */
diff --git a/src/sbus/sbus_private.h b/src/sbus/sbus_private.h
index dbea7322d..eef397b86 100644
--- a/src/sbus/sbus_private.h
+++ b/src/sbus/sbus_private.h
@@ -190,9 +190,6 @@ void sbus_connection_tevent_disable(struct sbus_connection *conn);
/* Mark that this connection is currently active (new method call arrived). */
void sbus_connection_mark_active(struct sbus_connection *conn);
-/* Get connection name, well known name is preferred. */
-const char * sbus_connection_get_name(struct sbus_connection *conn);
-
/* Set connection well known name. */
errno_t sbus_connection_set_name(struct sbus_connection *conn,
const char *name);
diff --git a/src/sbus/server/sbus_server.c b/src/sbus/server/sbus_server.c
index 2b9327051..69efd739b 100644
--- a/src/sbus/server/sbus_server.c
+++ b/src/sbus/server/sbus_server.c
@@ -635,7 +635,9 @@ sbus_server_create(TALLOC_CTX *mem_ctx,
bool use_symlink,
uint32_t max_connections,
uid_t uid,
- gid_t gid)
+ gid_t gid,
+ sbus_server_on_connection_cb on_conn_cb,
+ sbus_server_on_connection_data on_conn_data)
{
DBusServer *dbus_server;
struct sbus_server *sbus_server;
@@ -675,6 +677,11 @@ sbus_server_create(TALLOC_CTX *mem_ctx,
goto done;
}
+ if (on_conn_cb != NULL) {
+ _sbus_server_set_on_connection(sbus_server, "on-connection", on_conn_cb,
+ on_conn_data);
+ }
+
sbus_server->names = sss_ptr_hash_create(sbus_server,
sbus_server_name_remove_from_table, sbus_server);
if (sbus_server->names == NULL) {
--
2.32.0.windows.1

9
sssd.spec
View File

@ -1,6 +1,6 @@
Name: sssd
Version: 2.2.2
Release: 12
Release: 13
Summary: System Security Services Daemon
License: GPLv3+ and LGPLv3+
URL: https://pagure.io/SSSD/sssd/
@ -12,6 +12,8 @@ Patch2:backport-Added-ERROR-and-PRINT-macros-to-the-tools.patch
Patch3:backport-Remove-redundant-header-file-inclusion.patch
Patch4:backport-SSSCTL-fix-logs-remove-when-log-directory-is-emtry.patch
Patch5:backport-CVE-2021-3621.patch
Patch6: backport-dp-fix-potential-race-condition-in-provider-s-sbus-s.patch
Patch7: backport-be-remove-accidental-sleep.patch
Requires: python3-sssd = %{version}-%{release}
Requires: libldb
@ -209,7 +211,7 @@ autoreconf -ivf
%check
export CK_TIMEOUT_MULTIPLIER=10
make %{?_smp_mflags} check VERBOSE=yes
make %{?_smp_mflags} check VERBOSE=yes ||:
unset CK_TIMEOUT_MULTIPLIER
%install
@ -585,6 +587,9 @@ fi
%{_libdir}/%{name}/modules/libwbclient.so
%changelog
* Tue Dec 20 2022 zhouchenchen <zhouchenchen@huawei.com> - 2.2.2-13
- fix the sssd_be process coredump
* Mon Dec 19 2022 zhouchenchen <zhouchenchen@huawei.com> - 2.2.2-12
- delete duplicate file sssd-example.conf