From a7cc0ce7dd55d267282d4615cd5288fbf55f9c3b Mon Sep 17 00:00:00 2001 From: zhouchenchen123 Date: Fri, 1 Jul 2022 11:18:08 +0800 Subject: [PATCH] fix sssd be core dump --- backport-be-remove-accidental-sleep.patch | 32 +++ ...-race-condition-in-provider-s-sbus-s.patch | 248 ++++++++++++++++++ sssd.spec | 8 +- 3 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 backport-be-remove-accidental-sleep.patch create mode 100644 backport-dp-fix-potential-race-condition-in-provider-s-sbus-s.patch diff --git a/backport-be-remove-accidental-sleep.patch b/backport-be-remove-accidental-sleep.patch new file mode 100644 index 0000000..b44d1a4 --- /dev/null +++ b/backport-be-remove-accidental-sleep.patch @@ -0,0 +1,32 @@ +From 7fbcaa8feeb968711ff52f51705c45062fd81394 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Pavel=20B=C5=99ezina?= +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 +--- + 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 74df62e..4c10d6b 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 +-- +1.8.3.1 + diff --git a/backport-dp-fix-potential-race-condition-in-provider-s-sbus-s.patch b/backport-dp-fix-potential-race-condition-in-provider-s-sbus-s.patch new file mode 100644 index 0000000..19d8cb3 --- /dev/null +++ b/backport-dp-fix-potential-race-condition-in-provider-s-sbus-s.patch @@ -0,0 +1,248 @@ +From 4a84f8e18ea5604ac7e69849dee492718fd96296 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Pavel=20B=C5=99ezina?= +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 +--- + 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 1e94a8f..d9da05a 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 ba1cfec..0858c43 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 01baf01..dcf939e 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 9566533..ca2d516 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 3f8702f..9cfe862 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 9136c4e..0983879 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 dbea732..eef397b 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 2b93270..69efd73 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_cb, sbus_server); + if (sbus_server->names == NULL) { +-- +1.8.3.1 + diff --git a/sssd.spec b/sssd.spec index 6a307bf..c683ea3 100644 --- a/sssd.spec +++ b/sssd.spec @@ -1,11 +1,14 @@ Name: sssd Version: 2.6.1 -Release: 1 +Release: 2 Summary: System Security Services Daemon License: GPLv3+ and LGPLv3+ URL: https://pagure.io/SSSD/sssd/ Source0: https://github.com/SSSD/sssd/releases/download/%{version}/%{name}-%{version}.tar.gz +Patch1: backport-dp-fix-potential-race-condition-in-provider-s-sbus-s.patch +Patch2: backport-be-remove-accidental-sleep.patch + Requires: python3-sssd = %{version}-%{release} Requires: libldb Requires: cyrus-sasl-gssapi%{?_isa} @@ -538,6 +541,9 @@ fi %{_libdir}/%{name}/modules/libwbclient.so %changelog +* Tue Jun 30 2022 zhouchenchen - 2.6.1-2 +- fix the sssd_be process coredump + * Wed Dec 29 2021 panxiaohe - 2.6.1-1 - update version to 2.6.1 -- Gitee