From 75a66ce1da41c5b081771b2db55c2994d378d882 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 23 Mar 2021 11:40:21 +0100 Subject: [PATCH 1/3] pidl: set the per-request memory context in the pidl generator The talloc memory context referenced by the pipe_struct mem_ctx member is used as talloc parent for RPC response data by the RPC service implementations. In Samba versions up to 4.10 all talloc children of p->mem_ctx were freed after a RPC response was delivered by calling talloc_free_children(p->mem_ctx). Commit 60fa8e255254d38e9443bf96f2c0f31430be6ab8 removed this call which resulted in all memory allocations on this context not getting released, which can consume significant memory in long running RPC connections. Instead of putting the talloc_free_children(p->mem_ctx) back, just use the mem_ctx argument of the ${pipename}_op_dispatch_internal() function which is a dcesrv_call_state object created by dcesrv_process_ncacn_packet() and released by the RPC server when the RPC request processing is finished. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14675 CI: https://gitlab.com/samba-team/samba/-/merge_requests/1861 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 4c3fb2a5912966a61e7ebdb05eb3231a0e1d6033) --- pidl/lib/Parse/Pidl/Samba4/NDR/ServerCompat.pm | 2 ++ source3/rpc_server/rpc_handles.c | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/ServerCompat.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/ServerCompat.pm index 54feea0a9ef..d1368c3dbca 100644 --- a/pidl/lib/Parse/Pidl/Samba4/NDR/ServerCompat.pm +++ b/pidl/lib/Parse/Pidl/Samba4/NDR/ServerCompat.pm @@ -299,6 +299,7 @@ sub boilerplate_iface($) $self->pidl("/* Update pipes struct opnum */"); $self->pidl("p->opnum = opnum;"); $self->pidl("p->dce_call = dce_call;"); + $self->pidl("p->mem_ctx = mem_ctx;"); $self->pidl("/* Update pipes struct session info */"); $self->pidl("pipe_session_info = p->session_info;"); $self->pidl("p->session_info = dce_call->auth_state->session_info;"); @@ -344,6 +345,7 @@ sub boilerplate_iface($) $self->pidl(""); $self->pidl("p->dce_call = NULL;"); + $self->pidl("p->mem_ctx = NULL;"); $self->pidl("/* Restore session info */"); $self->pidl("p->session_info = pipe_session_info;"); $self->pidl("p->auth.auth_type = 0;"); diff --git a/source3/rpc_server/rpc_handles.c b/source3/rpc_server/rpc_handles.c index 45968746440..9ef93231466 100644 --- a/source3/rpc_server/rpc_handles.c +++ b/source3/rpc_server/rpc_handles.c @@ -60,12 +60,6 @@ int make_base_pipes_struct(TALLOC_CTX *mem_ctx, return ENOMEM; } - p->mem_ctx = talloc_named(p, 0, "pipe %s %p", pipe_name, p); - if (!p->mem_ctx) { - talloc_free(p); - return ENOMEM; - } - p->msg_ctx = msg_ctx; p->transport = transport; -- 2.30.2 From 33a7749f7fc5c58752815789d086d696a20878e7 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 22 Mar 2021 12:06:39 +0100 Subject: [PATCH 2/3] spools: avoid leaking memory into the callers mem_ctx BUG: https://bugzilla.samba.org/show_bug.cgi?id=14675 CI: https://gitlab.com/samba-team/samba/-/merge_requests/1861 Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke (cherry picked from commit 481176ec745c14b78fca68e01a61c83405a4b97b) --- source3/rpc_server/spoolss/srv_spoolss_nt.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c index d20c19d5271..24ea7367ec8 100644 --- a/source3/rpc_server/spoolss/srv_spoolss_nt.c +++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c @@ -5731,7 +5731,8 @@ static WERROR construct_printer_driver_info_level(TALLOC_CTX *mem_ctx, } if (pinfo2->drivername == NULL || pinfo2->drivername[0] == '\0') { - return WERR_UNKNOWN_PRINTER_DRIVER; + result = WERR_UNKNOWN_PRINTER_DRIVER; + goto done; } DBG_INFO("Construct printer driver [%s] for [%s]\n", @@ -7023,7 +7024,8 @@ static WERROR update_printer(struct pipes_struct *p, raddr = tsocket_address_inet_addr_string(p->remote_address, p->mem_ctx); if (raddr == NULL) { - return WERR_NOT_ENOUGH_MEMORY; + result = WERR_NOT_ENOUGH_MEMORY; + goto done; } /* add_printer_hook() will call reload_services() */ -- 2.30.2 From 602290d48d3bc49acca64a089822f26da293ee1e Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 23 Mar 2021 17:06:15 +0100 Subject: [PATCH 3/3] rpc_server3: Fix a memleak for internal pipes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit state->call should not be talloc'ed off a long-lived context BUG: https://bugzilla.samba.org/show_bug.cgi?id=14675 CI: https://gitlab.com/samba-team/samba/-/merge_requests/1861 RN: Memory leak in the RPC server Signed-off-by: Volker Lendecke Reviewed-by: Samuel Cabrero Reviewed-by: Ralph Boehme Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Wed Mar 31 12:14:01 UTC 2021 on sn-devel-184 (cherry picked from commit 12f516e4680753460e7fe8811e6c6ff70057580c) --- source3/rpc_server/rpc_ncacn_np.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/rpc_server/rpc_ncacn_np.c b/source3/rpc_server/rpc_ncacn_np.c index 9ba271c2479..494b002e714 100644 --- a/source3/rpc_server/rpc_ncacn_np.c +++ b/source3/rpc_server/rpc_ncacn_np.c @@ -476,7 +476,7 @@ static struct tevent_req *rpcint_bh_raw_call_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } - state->call = talloc_zero(hs->conn, struct dcesrv_call_state); + state->call = talloc_zero(state, struct dcesrv_call_state); if (tevent_req_nomem(state->call, req)) { return tevent_req_post(req, ev); } -- 2.30.2