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

Jule Anger janger at samba.org
Tue Aug 23 08:54:02 UTC 2022


The branch, v4-16-test has been updated
       via  df7d6f0c486 lib:replace: Only include <sys/mount.h> on non-Linux systems
       via  ce464a83c76 s3: smbd: Plumb close_type parameter through close_file_in_loop(), file_close_conn()
       via  a5cf33d4041 s3: smbd: Add "enum file_close_type close_type" parameter to file_close_conn().
       via  706c64c6f0e s3: smbd: Add "enum file_close_type close_type" parameter to close_cnum().
       via  d1bc0d0b51b s3/smbd: Use after free when iterating smbd_server_connection->connections
       via  56e1a9fc623 s3/smbd: Use after free when iterating smbd_server_connection->connections
       via  9cb40437278 s3:smbd: only clear LEASE_READ if there's no read lease is left
       via  b910d9f6e00 s4:torture/smb2: add smb2.lease.v[1,2]_bug_15148
       via  19f285e0809 s3:smbd: share_mode_flags_set() takes SMB2_LEASE_* values
       via  f6afc5b35e7 libcli/smb: Set error status if 'iov' pointer is NULL
       via  f33ad1c1725 libcli/smb: Ensure we call tevent_req_nterror() on failure
      from  b75b5f60ba3 s3/util/py_net.c: fix samba-tool domain join&leave segfault

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


- Log -----------------------------------------------------------------
commit df7d6f0c48612feea428643006d32c2292c662e2
Author: Andreas Schneider <asn at samba.org>
Date:   Tue Aug 2 07:55:46 2022 +0200

    lib:replace: Only include <sys/mount.h> on non-Linux systems
    
    Details at:
    https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15132
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit 766151bf5b7ef95ae4c8c98b8994e5c21c5bbec0)
    
    Autobuild-User(v4-16-test): Jule Anger <janger at samba.org>
    Autobuild-Date(v4-16-test): Tue Aug 23 08:53:41 UTC 2022 on sn-devel-184

commit ce464a83c76ce612171f3df4933058695210915e
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Aug 17 11:43:47 2022 -0700

    s3: smbd: Plumb close_type parameter through close_file_in_loop(), file_close_conn()
    
    Allows close_file_in_loop() to differentiate between SHUTDOWN_CLOSE
    (previously it only used this close type) and ERROR_CLOSE - called
    on error from smbXsrv_tcon_disconnect() in the error path. In that
    case we want to close the fd, but not run any delete-on-close actions.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15128
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reivewed-by: Noel Power <npower at samba.org>
    
    Autobuild-User(master): Noel Power <npower at samba.org>
    Autobuild-Date(master): Thu Aug 18 14:10:18 UTC 2022 on sn-devel-184
    
    (cherry picked from commit cf5f7b1489930f6d64c3e3512f116ccf286d4605)

commit a5cf33d4041d44f1f8a80563b81f3bc6893bc7ce
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Aug 17 11:39:36 2022 -0700

    s3: smbd: Add "enum file_close_type close_type" parameter to file_close_conn().
    
    Not yet used.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15128
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>
    (cherry picked from commit 7005a6354df5522d9f665fb30052c458dfc93124)
    [npower at samba.org Adjusted for 4.15 filename change
             smb2-service.c -> service.c]

commit 706c64c6f0ee8cca24715cf4d591ed504432ce0f
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Aug 17 11:35:29 2022 -0700

    s3: smbd: Add "enum file_close_type close_type" parameter to close_cnum().
    
    Not yet used, but needed so we can differentiate between
    SHUTDOWN_CLOSE and ERROR_CLOSE in smbXsrv_tcon_disconnect()
    if we fail to chdir. In that case we want to close the fd,
    but not run any delete-on-close actions.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15128
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>
    (cherry picked from commit 9203d17106c0e55a30813ff1ed76869c7581a343)
    [npower at samba.org Adjusted for 4.15 filename change
             smb2-service.c -> service.c]

commit d1bc0d0b51bb8145c4d1597a39f72d85b28f8b35
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Jul 22 16:28:03 2022 +0100

    s3/smbd: Use after free when iterating smbd_server_connection->connections
    
    Change conn_free() to just use a destructor. We now
    catch any other places where we may have forgetten to
    call conn_free() - it's implicit on talloc_free(conn).
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15128
    
    Based on code from Noel Power <noel.power at suse.com>.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>
    
    Autobuild-User(master): Noel Power <npower at samba.org>
    Autobuild-Date(master): Wed Aug 17 09:54:06 UTC 2022 on sn-devel-184
    
    (cherry picked from commit f92bacbe216d2d74ea3ccf3fe0df5c1cc9860996)

commit 56e1a9fc623ae184fefcf3214a6b1801b37e5fff
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Aug 16 13:51:27 2022 -0700

    s3/smbd: Use after free when iterating smbd_server_connection->connections
    
    In SMB2 smbd_smb2_tree_connect() we create a new conn struct
    inside make_connection_smb2() then move the ownership to tcon using:
    
            tcon->compat = talloc_move(tcon, &compat_conn);
    
    so the lifetime of tcon->compat is tied directly to tcon.
    
    Inside smbXsrv_tcon_disconnect() we have:
    
     908                 ok = chdir_current_service(tcon->compat);
     909                 if (!ok) {
     910                         status = NT_STATUS_INTERNAL_ERROR;
     911                         DEBUG(0, ("smbXsrv_tcon_disconnect(0x%08x, '%s'): "
     912                                   "chdir_current_service() failed: %s\n",
     913                                   tcon->global->tcon_global_id,
     914                                   tcon->global->share_name,
     915                                   nt_errstr(status)));
     916                         tcon->compat = NULL;
     917                         return status;
     918                 }
     919
     920                 close_cnum(tcon->compat, vuid);
     921                 tcon->compat = NULL;
    
    If chdir_current_service(tcon->compat) fails, we return status without ever having
    called close_cnum(tcon->compat, vuid), leaving the conn pointer left in the linked
    list sconn->connections.
    
    The caller frees tcon and (by ownership) tcon->compat, still leaving the
    freed tcon->compat pointer on the sconn->connections linked list.
    
    When deadtime_fn() fires and walks the sconn->connections list it
    indirects this freed pointer. We must call close_cnum() on error also.
    
    Valgrind trace from Noel Power <noel.power at suse.com> is:
    
    ==6432== Invalid read of size 8
    ==6432==    at 0x52CED3A: conn_lastused_update (conn_idle.c:38)
    ==6432==    by 0x52CEDB1: conn_idle_all (conn_idle.c:54)
    ==6432==    by 0x5329971: deadtime_fn (smb2_process.c:1566)
    ==6432==    by 0x5DA2339: smbd_idle_event_handler (util_event.c:45)
    ==6432==    by 0x685F2F8: tevent_common_invoke_timer_handler (tevent_timed.c:376)
    
    ==6432==  Address 0x19074b88 is 232 bytes inside a block of size 328 free'd
    ==6432==    at 0x4C3451B: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==6432==    by 0x5B38521: _tc_free_internal (talloc.c:1222)
    ==6432==    by 0x5B39463: _tc_free_children_internal (talloc.c:1669)
    ==6432==    by 0x5B38404: _tc_free_internal (talloc.c:1184)
    ==6432==    by 0x5B39463: _tc_free_children_internal (talloc.c:1669)
    ==6432==    by 0x5B38404: _tc_free_internal (talloc.c:1184)
    ==6432==    by 0x5B39463: _tc_free_children_internal (talloc.c:1669)
    ==6432==    by 0x5B38404: _tc_free_internal (talloc.c:1184)
    ==6432==    by 0x5B39463: _tc_free_children_internal (talloc.c:1669)
    ==6432==    by 0x5B38404: _tc_free_internal (talloc.c:1184)
    ==6432==    by 0x5B385C5: _talloc_free_internal (talloc.c:1248)
    ==6432==    by 0x5B3988D: _talloc_free (talloc.c:1792)
    ==6432==    by 0x5349B22: smbd_smb2_flush_send_queue (smb2_server.c:4828)
    
    ==6432==  Block was alloc'd at
    ==6432==    at 0x4C332EF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==6432==    by 0x5B378D9: __talloc_with_prefix (talloc.c:783)
    ==6432==    by 0x5B37A73: __talloc (talloc.c:825)
    ==6432==    by 0x5B37E0C: _talloc_named_const (talloc.c:982)
    ==6432==    by 0x5B3A8ED: _talloc_zero (talloc.c:2421)
    ==6432==    by 0x539873A: conn_new (conn.c:70)
    ==6432==    by 0x532D692: make_connection_smb2 (smb2_service.c:909)
    ==6432==    by 0x5352B5E: smbd_smb2_tree_connect (smb2_tcon.c:344)
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15128
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>
    (cherry picked from commit 0bdfb5a5e60df214c088df0782c4a1bcc2a4944a)

commit 9cb40437278fb7963f42efe69ce0227aa21303bc
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Aug 15 22:45:17 2022 +0200

    s3:smbd: only clear LEASE_READ if there's no read lease is left
    
    If contend_level2_oplocks_begin_default() skips break it's
    own lease, we should not clear SHARE_MODE_LEASE_READ
    in share_mode_data->flags.
    
    Otherwise that lease won't see any lease break notifications
    for writes from other clients (file handles not using the same lease
    key).
    
    So we need to count the number existing read leases (including
    the one with the same lease key) in order to know it's
    safe to clear SMB2_LEASE_READ/SHARE_MODE_LEASE_READ.
    
    Otherwise the next run (likely from another client)
    will get the wrong result from file_has_read_lease().
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15148
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Thu Aug 18 19:41:33 UTC 2022 on sn-devel-184
    
    (cherry picked from commit 96e2a82760ea06a89b7387b5cd3e864732afded3)

commit b910d9f6e0077159f44a12437402811337c51533
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Aug 17 17:07:08 2022 +0200

    s4:torture/smb2: add smb2.lease.v[1,2]_bug_15148
    
    This demonstrates the bug that happens with a
    write to a file handle holding an R lease,
    while there are other openers without any lease.
    
    When one of the other openers writes to the file,
    the R lease of the only lease holder isn't broken to NONE.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15148
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit 9e5ff607eb1b9c45c8836d3cff9d51b418740b87)

commit 19f285e080980b0fbac125d3e0877bfe8424ff25
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon Aug 15 10:49:13 2022 +0200

    s3:smbd: share_mode_flags_set() takes SMB2_LEASE_* values
    
    We currently only ever pass SMB2_LEASE_READ and both
    have the same value of 0x1, so for now it's only cosmetic,
    but that will change soon.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15148
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit 7592aad4d7a84d0ac66a156a22af3ad77803e55c)

commit f6afc5b35e733b757be8b3c16cff1c872014c8c2
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Mon Aug 22 16:56:46 2022 +1200

    libcli/smb: Set error status if 'iov' pointer is NULL
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15152
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    
    Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
    Autobuild-Date(master): Mon Aug 22 09:03:29 UTC 2022 on sn-devel-184
    
    (cherry picked from commit 75e03ea021afa66842b6e0dea21072b1b8026d58)

commit f33ad1c1725946c3485ab5023e0391a38ebe5fc6
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Mon Aug 22 15:50:02 2022 +1200

    libcli/smb: Ensure we call tevent_req_nterror() on failure
    
    Commit 3594c3ae202688fd8aae5f7f5e20464cb23feea9 added a NULL check for
    'inhdr', but it meant we didn't always call tevent_req_nterror() when we
    should.
    
    Now we handle connection errors. We now also set an error status if the
    NULL check fails.
    
    I noticed this when an ECONNRESET error from a server refusing SMB1
    wasn't handled, and the client subsequently hung in epoll_wait().
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15152
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    (cherry picked from commit 40d4912d841e6bcd7cd37810ef101d5f89268ee7)

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

Summary of changes:
 lib/replace/system/filesys.h |   4 +-
 lib/replace/wscript          |   3 +
 libcli/smb/smbXcli_base.c    |  12 ++-
 source3/smbd/conn.c          |  36 +++++---
 source3/smbd/files.c         |  17 ++--
 source3/smbd/open.c          |   2 +-
 source3/smbd/oplock.c        |  13 +--
 source3/smbd/proto.h         |   6 +-
 source3/smbd/service.c       |   6 +-
 source3/smbd/smbXsrv_tcon.c  |  11 ++-
 source4/torture/smb2/lease.c | 208 +++++++++++++++++++++++++++++++++++++++++++
 11 files changed, 285 insertions(+), 33 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/replace/system/filesys.h b/lib/replace/system/filesys.h
index 034e5d5886c..bb9482c69af 100644
--- a/lib/replace/system/filesys.h
+++ b/lib/replace/system/filesys.h
@@ -36,7 +36,8 @@
 #include <sys/param.h>
 #endif
 
-#ifdef HAVE_SYS_MOUNT_H
+/* This include is required on UNIX (*BSD, AIX, ...) for statfs() */
+#if !defined(LINUX) && defined(HAVE_SYS_MOUNT_H)
 #include <sys/mount.h>
 #endif
 
@@ -44,6 +45,7 @@
 #include <mntent.h>
 #endif
 
+/* This include is required on Linux for statfs() */
 #ifdef HAVE_SYS_VFS_H
 #include <sys/vfs.h>
 #endif
diff --git a/lib/replace/wscript b/lib/replace/wscript
index e4c2d513076..0db93d8caf1 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -31,6 +31,9 @@ def configure(conf):
 
     conf.env.standalone_replace = conf.IN_LAUNCH_DIR()
 
+    if sys.platform.rfind('linux') > -1:
+        conf.DEFINE('LINUX', '1')
+
     conf.DEFINE('BOOL_DEFINED', 1)
     conf.DEFINE('HAVE_LIBREPLACE', 1)
     conf.DEFINE('LIBREPLACE_NETWORK_CHECKS', 1)
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index 7579fa1c378..5d5b5ac45fd 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -4469,7 +4469,11 @@ static void smbXcli_negprot_smb1_done(struct tevent_req *subreq)
 				  NULL, /* pinbuf */
 				  expected, ARRAY_SIZE(expected));
 	TALLOC_FREE(subreq);
-	if (inhdr == NULL || tevent_req_nterror(req, status)) {
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+	if (inhdr == NULL) {
+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
 		return;
 	}
 
@@ -5009,7 +5013,11 @@ static void smbXcli_negprot_smb2_done(struct tevent_req *subreq)
 
 	status = smb2cli_req_recv(subreq, state, &iov,
 				  expected, ARRAY_SIZE(expected));
-	if (tevent_req_nterror(req, status) || iov == NULL) {
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+	if (iov == NULL) {
+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
 		return;
 	}
 
diff --git a/source3/smbd/conn.c b/source3/smbd/conn.c
index 044242d5697..776d7af4c12 100644
--- a/source3/smbd/conn.c
+++ b/source3/smbd/conn.c
@@ -24,6 +24,25 @@
 #include "smbd/globals.h"
 #include "lib/util/bitmap.h"
 
+static void conn_free_internal(connection_struct *conn);
+
+/****************************************************************************
+ * Remove a conn struct from conn->sconn->connections
+ * if not already done.
+****************************************************************************/
+
+static int conn_struct_destructor(connection_struct *conn)
+{
+        if (conn->sconn != NULL) {
+		DLIST_REMOVE(conn->sconn->connections, conn);
+		SMB_ASSERT(conn->sconn->num_connections > 0);
+		conn->sconn->num_connections--;
+		conn->sconn = NULL;
+	}
+	conn_free_internal(conn);
+	return 0;
+}
+
 /****************************************************************************
  Return the number of open connections.
 ****************************************************************************/
@@ -115,6 +134,11 @@ connection_struct *conn_new(struct smbd_server_connection *sconn)
 	DLIST_ADD(sconn->connections, conn);
 	sconn->num_connections++;
 
+	/*
+	 * Catches the case where someone forgets to call
+	 * conn_free().
+	 */
+	talloc_set_destructor(conn, conn_struct_destructor);
 	return conn;
 }
 
@@ -212,7 +236,6 @@ static void conn_free_internal(connection_struct *conn)
 	free_namearray(conn->aio_write_behind_list);
 
 	ZERO_STRUCTP(conn);
-	talloc_destroy(conn);
 }
 
 /****************************************************************************
@@ -221,16 +244,7 @@ static void conn_free_internal(connection_struct *conn)
 
 void conn_free(connection_struct *conn)
 {
-	if (conn->sconn == NULL) {
-		conn_free_internal(conn);
-		return;
-	}
-
-	DLIST_REMOVE(conn->sconn->connections, conn);
-	SMB_ASSERT(conn->sconn->num_connections > 0);
-	conn->sconn->num_connections--;
-
-	conn_free_internal(conn);
+	TALLOC_FREE(conn);
 }
 
 /*
diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index 704d3d84316..e2591b4d77d 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -752,7 +752,8 @@ NTSTATUS parent_pathref(TALLOC_CTX *mem_ctx,
 	return NT_STATUS_OK;
 }
 
-static bool close_file_in_loop(struct files_struct *fsp)
+static bool close_file_in_loop(struct files_struct *fsp,
+			       enum file_close_type close_type)
 {
 	if (fsp->base_fsp != NULL) {
 		/*
@@ -770,7 +771,7 @@ static bool close_file_in_loop(struct files_struct *fsp)
 		fsp->base_fsp->stream_fsp = NULL;
 		fsp->base_fsp = NULL;
 
-		close_file_free(NULL, &fsp, SHUTDOWN_CLOSE);
+		close_file_free(NULL, &fsp, close_type);
 		return NULL;
 	}
 
@@ -794,7 +795,7 @@ static bool close_file_in_loop(struct files_struct *fsp)
 		return false;
 	}
 
-	close_file_free(NULL, &fsp, SHUTDOWN_CLOSE);
+	close_file_free(NULL, &fsp, close_type);
 	return true;
 }
 
@@ -804,6 +805,7 @@ static bool close_file_in_loop(struct files_struct *fsp)
 
 struct file_close_conn_state {
 	struct connection_struct *conn;
+	enum file_close_type close_type;
 	bool fsp_left_behind;
 };
 
@@ -825,7 +827,7 @@ static struct files_struct *file_close_conn_fn(
 		fsp->op->global->durable = false;
 	}
 
-	did_close = close_file_in_loop(fsp);
+	did_close = close_file_in_loop(fsp, state->close_type);
 	if (!did_close) {
 		state->fsp_left_behind = true;
 	}
@@ -833,9 +835,10 @@ static struct files_struct *file_close_conn_fn(
 	return NULL;
 }
 
-void file_close_conn(connection_struct *conn)
+void file_close_conn(connection_struct *conn, enum file_close_type close_type)
 {
-	struct file_close_conn_state state = { .conn = conn };
+	struct file_close_conn_state state = { .conn = conn,
+					       .close_type = close_type };
 
 	files_forall(conn->sconn, file_close_conn_fn, &state);
 
@@ -921,7 +924,7 @@ static struct files_struct *file_close_user_fn(
 		return NULL;
 	}
 
-	did_close = close_file_in_loop(fsp);
+	did_close = close_file_in_loop(fsp, SHUTDOWN_CLOSE);
 	if (!did_close) {
 		state->fsp_left_behind = true;
 	}
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 0fdbb1a9fa3..5d21f1ab0e7 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2726,7 +2726,7 @@ grant:
 	if (granted & SMB2_LEASE_READ) {
 		uint32_t acc, sh, ls;
 		share_mode_flags_get(lck, &acc, &sh, &ls);
-		ls |= SHARE_MODE_LEASE_READ;
+		ls |= SMB2_LEASE_READ;
 		share_mode_flags_set(lck, acc, sh, ls, NULL);
 	}
 
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index d02872a610f..7dc944c10c1 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -1175,7 +1175,7 @@ struct break_to_none_state {
 	struct file_id id;
 	struct smb2_lease_key lease_key;
 	struct GUID client_guid;
-	size_t num_broken;
+	size_t num_read_leases;
 };
 
 static bool do_break_lease_to_none(struct share_mode_entry *e,
@@ -1209,6 +1209,8 @@ static bool do_break_lease_to_none(struct share_mode_entry *e,
 		return false;
 	}
 
+	state->num_read_leases += 1;
+
 	our_own = smb2_lease_equal(&state->client_guid,
 				   &state->lease_key,
 				   &e->client_guid,
@@ -1224,8 +1226,6 @@ static bool do_break_lease_to_none(struct share_mode_entry *e,
 
 	send_break_to_none(state->sconn->msg_ctx, &state->id, e);
 
-	state->num_broken += 1;
-
 	return false;
 }
 
@@ -1259,11 +1259,12 @@ static bool do_break_oplock_to_none(struct share_mode_entry *e,
 		return false;
 	}
 
+	state->num_read_leases += 1;
+
 	/* Paranoia .... */
 	SMB_ASSERT(!EXCLUSIVE_OPLOCK_TYPE(e->op_type));
 
 	send_break_to_none(state->sconn->msg_ctx, &state->id, e);
-	state->num_broken += 1;
 
 	return false;
 }
@@ -1337,14 +1338,14 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
 		DBG_WARNING("share_mode_forall_entries failed\n");
 	}
 
-	if (state.num_broken == 0) {
+	if (state.num_read_leases == 0) {
 		/*
 		 * Lazy update here. It might be that the read lease
 		 * has gone in the meantime.
 		 */
 		uint32_t acc, sh, ls;
 		share_mode_flags_get(lck, &acc, &sh, &ls);
-		ls &= ~SHARE_MODE_LEASE_READ;
+		ls &= ~SMB2_LEASE_READ;
 		share_mode_flags_set(lck, acc, sh, ls, NULL);
 	}
 
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 598ca1de2e2..d7020147861 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -394,7 +394,7 @@ void fsp_set_gen_id(files_struct *fsp);
 NTSTATUS file_new(struct smb_request *req, connection_struct *conn,
 		  files_struct **result);
 NTSTATUS fsp_bind_smb(struct files_struct *fsp, struct smb_request *req);
-void file_close_conn(connection_struct *conn);
+void file_close_conn(connection_struct *conn, enum file_close_type close_type);
 bool file_init_global(void);
 bool file_init(struct smbd_server_connection *sconn);
 void file_close_user(struct smbd_server_connection *sconn, uint64_t vuid);
@@ -1124,7 +1124,9 @@ connection_struct *make_connection(struct smb_request *req,
 				   const char *service_in,
 				   const char *pdev, uint64_t vuid,
 				   NTSTATUS *status);
-void close_cnum(connection_struct *conn, uint64_t vuid);
+void close_cnum(connection_struct *conn,
+		uint64_t vuid,
+		enum file_close_type close_type);
 
 /* The following definitions come from smbd/session.c  */
 struct sessionid;
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index ef7c14d92d0..d4dc81bf986 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -1111,14 +1111,16 @@ connection_struct *make_connection(struct smb_request *req,
  Close a cnum.
 ****************************************************************************/
 
-void close_cnum(connection_struct *conn, uint64_t vuid)
+void close_cnum(connection_struct *conn,
+		uint64_t vuid,
+		enum file_close_type close_type)
 {
 	char rootpath[2] = { '/', '\0'};
 	struct smb_filename root_fname = { .base_name = rootpath };
 	const struct loadparm_substitution *lp_sub =
 		loadparm_s3_global_substitution();
 
-	file_close_conn(conn);
+	file_close_conn(conn, close_type);
 
 	change_to_root_user();
 
diff --git a/source3/smbd/smbXsrv_tcon.c b/source3/smbd/smbXsrv_tcon.c
index 6b105522855..8707082edd6 100644
--- a/source3/smbd/smbXsrv_tcon.c
+++ b/source3/smbd/smbXsrv_tcon.c
@@ -913,11 +913,20 @@ NTSTATUS smbXsrv_tcon_disconnect(struct smbXsrv_tcon *tcon, uint64_t vuid)
 				  tcon->global->tcon_global_id,
 				  tcon->global->share_name,
 				  nt_errstr(status)));
+			/*
+			 * We must call close_cnum() on
+			 * error, as the caller is going
+			 * to free tcon and tcon->compat
+			 * so we must ensure tcon->compat is
+			 * removed from the linked list
+			 * conn->sconn->connections.
+			 */
+			close_cnum(tcon->compat, vuid, ERROR_CLOSE);
 			tcon->compat = NULL;
 			return status;
 		}
 
-		close_cnum(tcon->compat, vuid);
+		close_cnum(tcon->compat, vuid, SHUTDOWN_CLOSE);
 		tcon->compat = NULL;
 	}
 
diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
index 43b418c5acf..a2c354dc02a 100644
--- a/source4/torture/smb2/lease.c
+++ b/source4/torture/smb2/lease.c
@@ -4556,6 +4556,210 @@ done:
 	return ret;
 }
 
+static bool test_lease_v1_bug_15148(struct torture_context *tctx,
+				    struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	struct smb2_create io1;
+	struct smb2_create io2;
+	struct smb2_lease ls1;
+	struct smb2_lease ls2;
+	struct smb2_handle h1 = {{0}};
+	struct smb2_handle h2 = {{0}};
+	struct smb2_write w;
+	NTSTATUS status;
+	const char *fname = "lease_v1_bug_15148.dat";
+	bool ret = true;
+	uint32_t caps;
+
+	caps = smb2cli_conn_server_capabilities(tree->session->transport->conn);
+	if (!(caps & SMB2_CAP_LEASING)) {
+		torture_skip(tctx, "leases are not supported");
+	}
+
+	tree->session->transport->lease.handler = torture_lease_handler;
+	tree->session->transport->lease.private_data = tree;
+	tree->session->transport->oplock.handler = torture_oplock_handler;
+	tree->session->transport->oplock.private_data = tree;
+
+	smb2_util_unlink(tree, fname);
+
+	torture_reset_lease_break_info(tctx, &lease_break_info);
+
+	/* Grab R lease over connection 1a */
+	smb2_lease_create(&io1, &ls1, false, fname, LEASE1, smb2_util_lease_state("R"));
+	status = smb2_create(tree, mem_ctx, &io1);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h1 = io1.out.file.handle;
+	CHECK_CREATED(&io1, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io1, "R", true, LEASE1, 0);
+
+	CHECK_NO_BREAK(tctx);
+
+	/* Contend with LEASE2. */
+	smb2_lease_create(&io2, &ls2, false, fname, LEASE2, smb2_util_lease_state("R"));
+	status = smb2_create(tree, mem_ctx, &io2);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h2 = io2.out.file.handle;
+	CHECK_CREATED(&io2, EXISTED, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io2, "R", true, LEASE2, 0);
+
+	CHECK_NO_BREAK(tctx);
+
+	ZERO_STRUCT(w);
+	w.in.file.handle = h1;
+	w.in.offset      = 0;
+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
+	memset(w.in.data.data, 'o', w.in.data.length);
+	status = smb2_write(tree, &w);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	ls2.lease_epoch += 1;
+	CHECK_BREAK_INFO("R", "", LEASE2);
+
+	torture_reset_lease_break_info(tctx, &lease_break_info);
+
+	ZERO_STRUCT(w);
+	w.in.file.handle = h1;
+	w.in.offset      = 0;
+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
+	memset(w.in.data.data, 'O', w.in.data.length);
+	status = smb2_write(tree, &w);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	CHECK_NO_BREAK(tctx);
+
+	ZERO_STRUCT(w);
+	w.in.file.handle = h2;
+	w.in.offset      = 0;
+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
+	memset(w.in.data.data, 'o', w.in.data.length);
+	status = smb2_write(tree, &w);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	ls1.lease_epoch += 1;
+	CHECK_BREAK_INFO("R", "", LEASE1);
+
+ done:
+	smb2_util_close(tree, h1);
+	smb2_util_close(tree, h2);
+
+	smb2_util_unlink(tree, fname);
+
+	talloc_free(mem_ctx);
+
+	return ret;
+}
+
+static bool test_lease_v2_bug_15148(struct torture_context *tctx,
+				    struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	struct smb2_create io1;
+	struct smb2_create io2;
+	struct smb2_lease ls1;
+	struct smb2_lease ls2;
+	struct smb2_handle h1 = {{0}};
+	struct smb2_handle h2 = {{0}};
+	struct smb2_write w;
+	NTSTATUS status;
+	const char *fname = "lease_v2_bug_15148.dat";
+	bool ret = true;
+	uint32_t caps;
+	enum protocol_types protocol;
+
+	caps = smb2cli_conn_server_capabilities(tree->session->transport->conn);
+	if (!(caps & SMB2_CAP_LEASING)) {
+		torture_skip(tctx, "leases are not supported");
+	}
+
+	protocol = smbXcli_conn_protocol(tree->session->transport->conn);
+	if (protocol < PROTOCOL_SMB3_00) {
+		torture_skip(tctx, "v2 leases are not supported");
+	}
+
+	tree->session->transport->lease.handler = torture_lease_handler;
+	tree->session->transport->lease.private_data = tree;
+	tree->session->transport->oplock.handler = torture_oplock_handler;
+	tree->session->transport->oplock.private_data = tree;
+
+	smb2_util_unlink(tree, fname);
+
+	torture_reset_lease_break_info(tctx, &lease_break_info);
+
+	/* Grab R lease over connection 1a */
+	smb2_lease_v2_create(&io1, &ls1, false, fname, LEASE1, NULL,
+			     smb2_util_lease_state("R"), 0x4711);
+	status = smb2_create(tree, mem_ctx, &io1);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h1 = io1.out.file.handle;
+	CHECK_CREATED(&io1, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+	ls1.lease_epoch += 1;


-- 
Samba Shared Repository



More information about the samba-cvs mailing list