[SCM] Samba Shared Repository - branch v4-14-test updated

Karolin Seeger kseeger at samba.org
Thu Apr 1 11:27:01 UTC 2021


The branch, v4-14-test has been updated
       via  0662726974b s3: smbd: fix deferred renames
       via  f5bb7a55018 s4: torture. Add smb2.lease.rename_wait test to reproduce regression in delay rename for lease break code.
       via  e85d111f54f rpc_server3: Fix a memleak for internal pipes
       via  ed30ce7aa0c spools: avoid leaking memory into the callers mem_ctx
       via  55c76604ca2 pidl: set the per-request memory context in the pidl generator
      from  051585ef361 smbd: free open_rec state in remove_deferred_open_message_smb2_internal()

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-14-test


- Log -----------------------------------------------------------------
commit 0662726974b43c2caa9d4a143c98d6935ca28eb7
Author: Ralph Boehme <slow at samba.org>
Date:   Mon Mar 29 12:24:39 2021 +0200

    s3: smbd: fix deferred renames
    
    This was broken by c7a9e0e4cdfb22e66533b5c8e20af3cfdb8ae78c.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14679
    CI: https://gitlab.com/samba-team/samba/-/merge_requests/1875
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at amba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Wed Mar 31 06:13:39 UTC 2021 on sn-devel-184
    
    (cherry picked from commit 10d753868e810604d8f60673bbd48f55aaff0797)
    
    Autobuild-User(v4-14-test): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(v4-14-test): Thu Apr  1 11:26:31 UTC 2021 on sn-devel-184

commit f5bb7a550180a60d929d854e01aeb84ddb00791f
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Mar 30 15:05:47 2021 -0700

    s4: torture. Add smb2.lease.rename_wait test to reproduce regression in delay rename for lease break code.
    
    Passes against Windows 10. Add to knownfail, the
    next commit will fix this.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14679
    CI: https://gitlab.com/samba-team/samba/-/merge_requests/1875
    
    Back-ported from 8d9a0b8d57713781c72440c7e91746b5d89e6f6a.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit e85d111f54f7aa77803f1e9fef92d5dd97968fd9
Author: Volker Lendecke <vl at samba.org>
Date:   Tue Mar 23 17:06:15 2021 +0100

    rpc_server3: Fix a memleak for internal pipes
    
    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 <vl at samba.org>
    Reviewed-by: Samuel Cabrero <scabrero at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Wed Mar 31 12:14:01 UTC 2021 on sn-devel-184
    
    (cherry picked from commit 12f516e4680753460e7fe8811e6c6ff70057580c)

commit ed30ce7aa0cce39bf0e0a6a97afc8716873692fc
Author: Ralph Boehme <slow at samba.org>
Date:   Mon Mar 22 12:06:39 2021 +0100

    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 <slow at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    (cherry picked from commit 481176ec745c14b78fca68e01a61c83405a4b97b)

commit 55c76604ca2fac7348a6bddba1dfdc128c728f30
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Mar 23 11:40:21 2021 +0100

    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 <slow at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    (cherry picked from commit 4c3fb2a5912966a61e7ebdb05eb3231a0e1d6033)

-----------------------------------------------------------------------

Summary of changes:
 pidl/lib/Parse/Pidl/Samba4/NDR/ServerCompat.pm |   2 +
 source3/rpc_server/rpc_handles.c               |   6 -
 source3/rpc_server/rpc_ncacn_np.c              |   2 +-
 source3/rpc_server/spoolss/srv_spoolss_nt.c    |   6 +-
 source3/smbd/smb2_setinfo.c                    |   1 +
 source4/torture/smb2/lease.c                   | 145 +++++++++++++++++++++++++
 6 files changed, 153 insertions(+), 9 deletions(-)


Changeset truncated at 500 lines:

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;
 
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);
 	}
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() */
diff --git a/source3/smbd/smb2_setinfo.c b/source3/smbd/smb2_setinfo.c
index 646e009a746..e490596a2e0 100644
--- a/source3/smbd/smb2_setinfo.c
+++ b/source3/smbd/smb2_setinfo.c
@@ -214,6 +214,7 @@ static bool delay_rename_lease_break_fn(
 		return false;
 	}
 
+	state->delay = true;
 	break_to = (e_lease_type & ~SMB2_LEASE_HANDLE);
 
 	send_break_message(
diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
index d3b8daea310..824db95a4b5 100644
--- a/source4/torture/smb2/lease.c
+++ b/source4/torture/smb2/lease.c
@@ -3722,6 +3722,148 @@ static bool test_lease_timeout(struct torture_context *tctx,
 	return ret;
 }
 
+static bool test_lease_rename_wait(struct torture_context *tctx,
+                               struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	struct smb2_create io;
+	struct smb2_lease ls1;
+	struct smb2_lease ls2;
+	struct smb2_lease ls3;
+	struct smb2_handle h1 = {{0}};
+	struct smb2_handle h2 = {{0}};
+	struct smb2_handle h3 = {{0}};
+	union smb_setfileinfo sinfo;
+	NTSTATUS status;
+	const char *fname_src = "lease_rename_src.dat";
+	const char *fname_dst = "lease_rename_dst.dat";
+	bool ret = true;
+	struct smb2_lease_break_ack ack = {};
+	struct smb2_request *rename_req = NULL;
+	uint32_t caps;
+	unsigned int i;
+
+	caps = smb2cli_conn_server_capabilities(tree->session->transport->conn);
+	if (!(caps & SMB2_CAP_LEASING)) {
+		torture_skip(tctx, "leases are not supported");
+	}
+
+	smb2_util_unlink(tree, fname_src);
+	smb2_util_unlink(tree, fname_dst);
+
+	/* Short timeout for fails. */
+	tree->session->transport->options.request_timeout = 15;
+
+	/* Grab a RH lease. */
+	smb2_lease_create(&io,
+			&ls1,
+			false,
+			fname_src,
+			LEASE1,
+			smb2_util_lease_state("RH"));
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io, "RH", true, LEASE1, 0);
+	h1 = io.out.file.handle;
+
+	/* Second open with a RH lease. */
+	smb2_lease_create(&io,
+			&ls2,
+			false,
+			fname_src,
+			LEASE2,
+			smb2_util_lease_state("RH"));
+	io.in.create_disposition = NTCREATEX_DISP_OPEN;
+	io.in.desired_access = GENERIC_READ_ACCESS;
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_CREATED(&io, EXISTED, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io, "RH", true, LEASE2, 0);
+	h2 = io.out.file.handle;
+
+	/*
+	 * Don't ack a lease break.
+	 */
+	tree->session->transport->lease.handler	= torture_lease_handler;
+	tree->session->transport->lease.private_data = tree;
+	torture_reset_lease_break_info(tctx, &lease_break_info);
+	lease_break_info.lease_skip_ack = true;
+
+	/* Break with a rename. */
+	ZERO_STRUCT(sinfo);
+	sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION;
+	sinfo.rename_information.in.file.handle = h1;
+	sinfo.rename_information.in.overwrite = true;
+	sinfo.rename_information.in.new_name = fname_dst;
+	rename_req = smb2_setinfo_file_send(tree, &sinfo);
+
+	torture_assert(tctx,
+			rename_req != NULL,
+			"smb2_setinfo_file_send");
+	torture_assert(tctx,
+			rename_req->state == SMB2_REQUEST_RECV,
+			"rename pending");
+
+	/* Try and open the destination with a RH lease. */
+	smb2_lease_create(&io,
+			&ls3,
+			false,
+			fname_dst,
+			LEASE3,
+			smb2_util_lease_state("RH"));
+	/* We want to open, not create. */
+	io.in.create_disposition = NTCREATEX_DISP_OPEN;
+	io.in.desired_access = GENERIC_READ_ACCESS;
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_NOT_FOUND);
+
+	/*
+	 * The smb2_create() I/O should have picked up the break request
+	 * caused by the pending rename.
+	 */
+
+	/* Copy the break request. */
+	ack.in.lease.lease_key =
+		lease_break_info.lease_break.current_lease.lease_key;
+	ack.in.lease.lease_state =
+		lease_break_info.lease_break.new_lease_state;
+
+	/*
+	 * Give the server 3 more chances to have renamed
+	 * the file. Better than doing a sleep.
+	 */
+	for (i = 0; i < 3; i++) {
+		status = smb2_create(tree, mem_ctx, &io);
+		CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_NOT_FOUND);
+	}
+
+	/* Ack the break. The server is now free to rename. */
+	status = smb2_lease_break_ack(tree, &ack);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/* Get the rename reply. */
+	status = smb2_setinfo_recv(rename_req);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/* The target should now exist. */
+	status = smb2_create(tree, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h3 = io.out.file.handle;
+
+ done:
+	smb2_util_close(tree, h1);
+	smb2_util_close(tree, h2);
+	smb2_util_close(tree, h3);
+
+	smb2_util_unlink(tree, fname_src);
+	smb2_util_unlink(tree, fname_dst);
+
+	talloc_free(mem_ctx);
+
+	return ret;
+}
+
 static bool test_lease_v2_rename(struct torture_context *tctx,
 				 struct smb2_tree *tree)
 {
@@ -4192,6 +4334,9 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx)
 	torture_suite_add_1smb2_test(suite, "dynamic_share", test_lease_dynamic_share);
 	torture_suite_add_1smb2_test(suite, "timeout", test_lease_timeout);
 	torture_suite_add_1smb2_test(suite, "unlink", test_lease_unlink);
+	torture_suite_add_1smb2_test(suite, "rename_wait",
+				test_lease_rename_wait);
+
 
 	suite->description = talloc_strdup(suite, "SMB2-LEASE tests");
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list