[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Thu Jan 21 14:48:01 UTC 2021


The branch, master has been updated
       via  480516e3b85 vfs_fruit: make use of adouble_open_from_base_fsp(ADOUBLE_RSRC) in fruit_open_rsrc_adouble()
       via  0b8c6e736af vfs_fruit: add fruit_get_complete_fio() helper
       via  94799dc8e61 vfs_fruit: let fruit_open_rsrc_adouble() return errno = EISDIR
       via  d62c670c3dd s3:adouble: add adouble_open_from_base_fsp()
       via  c45a8d753d3 s3:adouble: allow ad_fget/ad_get_internal to be used with a backend fsp
       via  db743ab005b share_mode_lock: DEBUG/ASSERT recursion deadlock detection
       via  1052314dcd0 s3:adouble: rewrite ad_open_rsrc() as adouble_open_rsrc_fsp() using create_internal_fsp()
      from  ff16c74ee2b WHATSNEW: Start release notes for Samba 4.15.0pre1.

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 480516e3b850035d188b8dab8490c63e0a7afe80
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Dec 8 16:29:10 2020 +0100

    vfs_fruit: make use of adouble_open_from_base_fsp(ADOUBLE_RSRC) in fruit_open_rsrc_adouble()
    
    The key is that we return a fake_fd to the caller and only open
    the '._' file in the background.
    
    The next vfs backend should only see the fsp from
    adouble_open_from_base_fsp, while the vfs backends above
    should only see the fake_fd.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Thu Jan 21 14:47:53 UTC 2021 on sn-devel-184

commit 0b8c6e736afbec7ea1f69219c54a33031db3b8d9
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Dec 10 13:11:06 2020 +0100

    vfs_fruit: add fruit_get_complete_fio() helper
    
    This will make it easier to hide some fsp extension later.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 94799dc8e61126f0bdd566e0d054424aebd9d477
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Dec 30 13:49:37 2020 +0100

    vfs_fruit: let fruit_open_rsrc_adouble() return errno = EISDIR
    
    That hopefully makes the check that ':AFP_Resource' can't
    be created on directories.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit d62c670c3dd33eafc515ee92be177d1ddcb7b0a9
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Dec 23 11:58:08 2020 +0100

    s3:adouble: add adouble_open_from_base_fsp()
    
    For now we only support ADOUBLE_RSRC, but that might change in future.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit c45a8d753d3a415d2a8038d1327276ecb972b036
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Dec 23 11:58:08 2020 +0100

    s3:adouble: allow ad_fget/ad_get_internal to be used with a backend fsp
    
    Up to now we only passed in stream fsp, but that will change shortly.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit db743ab005bc7671d4fb0f5bea895c1097b707c5
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Dec 23 11:59:05 2020 +0100

    share_mode_lock: DEBUG/ASSERT recursion deadlock detection
    
    This situation should never happen!
    
    The known trigger is fixed with the change to adouble_open_rsrc_fsp()
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 1052314dcd05738f29d1ae85a5a4b8eaa4babe3d
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Dec 23 11:58:08 2020 +0100

    s3:adouble: rewrite ad_open_rsrc() as adouble_open_rsrc_fsp() using create_internal_fsp()
    
    "._" AppleDouble files are hidden by vfs_fruit by default, so there's no
    need to go through a full SMB_VFS_CREATE_FILE() for them.
    
    They don't need an smbXsrv_open_global.tdb entry nor a locking.tdb
    entry, so we just open them with fd_openat().
    
    This avoids a recursion deadlock in get_share_mode_lock() when closing
    the ':AFP_Resource' stream.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

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

Summary of changes:
 source3/lib/adouble.c             | 146 ++++++++++++--------
 source3/lib/adouble.h             |   6 +
 source3/locking/share_mode_lock.c |  21 ++-
 source3/modules/vfs_fruit.c       | 284 ++++++++++++++++++++++++++------------
 4 files changed, 314 insertions(+), 143 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c
index dffc0d5b7be..e869965876d 100644
--- a/source3/lib/adouble.c
+++ b/source3/lib/adouble.c
@@ -2091,71 +2091,95 @@ exit:
 	return ealen;
 }
 
-static int ad_open_rsrc(vfs_handle_struct *handle,
-			const struct smb_filename *smb_fname,
-			int flags,
-			mode_t mode,
-			files_struct **_fsp)
+static NTSTATUS adouble_open_rsrc_fsp(const struct files_struct *dirfsp,
+				      const struct smb_filename *smb_base_fname,
+				      int in_flags,
+				      mode_t mode,
+				      struct files_struct **_ad_fsp)
 {
-	int ret;
+	int rc = 0;
+	struct adouble *ad = NULL;
 	struct smb_filename *adp_smb_fname = NULL;
-	files_struct *fsp = NULL;
-	uint32_t access_mask;
-	uint32_t share_access;
-	uint32_t create_disposition;
+	struct files_struct *ad_fsp = NULL;
 	NTSTATUS status;
+	int flags = in_flags;
 
-	ret = adouble_path(talloc_tos(), smb_fname, &adp_smb_fname);
-	if (ret != 0) {
-		return -1;
+	rc = adouble_path(talloc_tos(),
+			  smb_base_fname,
+			  &adp_smb_fname);
+	if (rc != 0) {
+		return NT_STATUS_NO_MEMORY;
 	}
 
-	ret = SMB_VFS_STAT(handle->conn, adp_smb_fname);
-	if (ret != 0) {
-		TALLOC_FREE(adp_smb_fname);
-		return -1;
+	status = create_internal_fsp(dirfsp->conn,
+				     adp_smb_fname,
+				     &ad_fsp);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
 	}
 
-	access_mask = FILE_GENERIC_READ;
-	share_access = FILE_SHARE_READ | FILE_SHARE_WRITE;
-	create_disposition = FILE_OPEN;
-
-	if (flags & O_RDWR) {
-		access_mask |= FILE_GENERIC_WRITE;
-		share_access &= ~FILE_SHARE_WRITE;
+#ifdef O_PATH
+	flags &= ~(O_PATH);
+#endif
+	if (flags & (O_CREAT | O_TRUNC | O_WRONLY)) {
+		/* We always need read/write access for the metadata header too */
+		flags &= ~(O_WRONLY);
+		flags |= O_RDWR;
 	}
 
-	status = openat_pathref_fsp(handle->conn->cwd_fsp, adp_smb_fname);
+	status = fd_openat(dirfsp,
+			   adp_smb_fname,
+			   ad_fsp,
+			   flags,
+			   mode);
 	if (!NT_STATUS_IS_OK(status)) {
-		return -1;
+		file_free(NULL, ad_fsp);
+		return status;
 	}
 
-	status = SMB_VFS_CREATE_FILE(
-		handle->conn,			/* conn */
-		NULL,				/* req */
-		adp_smb_fname,
-		access_mask,
-		share_access,
-		create_disposition,
-		0,				/* create_options */
-		0,				/* file_attributes */
-		INTERNAL_OPEN_ONLY,		/* oplock_request */
-		NULL,				/* lease */
-		0,				/* allocation_size */
-		0,				/* private_flags */
-		NULL,				/* sd */
-		NULL,				/* ea_list */
-		&fsp,
-		NULL,				/* psbuf */
-		NULL, NULL);			/* create context */
-	TALLOC_FREE(adp_smb_fname);
-	if (!NT_STATUS_IS_OK(status)) {
-		DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-		return -1;
+	if (flags & (O_CREAT | O_TRUNC)) {
+		ad = ad_init(talloc_tos(), ADOUBLE_RSRC);
+		if (ad == NULL) {
+			file_free(NULL, ad_fsp);
+			return NT_STATUS_NO_MEMORY;
+		}
+
+		rc = ad_fset(ad_fsp->conn->vfs_handles, ad, ad_fsp);
+		if (rc != 0) {
+			file_free(NULL, ad_fsp);
+			return NT_STATUS_IO_DEVICE_ERROR;
+		}
+		TALLOC_FREE(ad);
 	}
 
-	*_fsp = fsp;
-	return 0;
+	*_ad_fsp = ad_fsp;
+	return NT_STATUS_OK;
+}
+
+NTSTATUS adouble_open_from_base_fsp(const struct files_struct *dirfsp,
+				    struct files_struct *base_fsp,
+				    adouble_type_t type,
+				    int flags,
+				    mode_t mode,
+				    struct files_struct **_ad_fsp)
+{
+	*_ad_fsp = NULL;
+
+	SMB_ASSERT(base_fsp != NULL);
+	SMB_ASSERT(base_fsp->base_fsp == NULL);
+
+	switch (type) {
+	case ADOUBLE_META:
+		return NT_STATUS_INTERNAL_ERROR;
+	case ADOUBLE_RSRC:
+		return adouble_open_rsrc_fsp(dirfsp,
+					     base_fsp->fsp_name,
+					     flags,
+					     mode,
+					     _ad_fsp);
+	}
+
+	return NT_STATUS_INTERNAL_ERROR;
 }
 
 /*
@@ -2170,7 +2194,7 @@ static int ad_open(vfs_handle_struct *handle,
 		   int flags,
 		   mode_t mode)
 {
-	int ret;
+	NTSTATUS status;
 
 	DBG_DEBUG("Path [%s] type [%s]\n", smb_fname->base_name,
 		  ad->ad_type == ADOUBLE_META ? "meta" : "rsrc");
@@ -2185,8 +2209,13 @@ static int ad_open(vfs_handle_struct *handle,
 		return 0;
 	}
 
-	ret = ad_open_rsrc(handle, smb_fname, flags, mode, &ad->ad_fsp);
-	if (ret != 0) {
+	status = adouble_open_rsrc_fsp(handle->conn->cwd_fsp,
+				       smb_fname,
+				       flags,
+				       mode,
+				       &ad->ad_fsp);
+	if (!NT_STATUS_IS_OK(status)) {
+		errno = map_errno_from_nt_status(status);
 		return -1;
 	}
 	ad->ad_opened = true;
@@ -2291,11 +2320,14 @@ static int adouble_destructor(struct adouble *ad)
 
 	SMB_ASSERT(ad->ad_fsp != NULL);
 
-	status = close_file(NULL, ad->ad_fsp, NORMAL_CLOSE);
+	status = fd_close(ad->ad_fsp);
 	if (!NT_STATUS_IS_OK(status)) {
 		DBG_ERR("Closing [%s] failed: %s\n",
 			fsp_str_dbg(ad->ad_fsp), nt_errstr(status));
 	}
+	file_free(NULL, ad->ad_fsp);
+	ad->ad_fsp = NULL;
+	ad->ad_opened = false;
 
 	return 0;
 }
@@ -2434,7 +2466,11 @@ static struct adouble *ad_get_internal(TALLOC_CTX *ctx,
 	int mode;
 
 	if (fsp != NULL) {
-		smb_fname = fsp->base_fsp->fsp_name;
+		if (fsp->base_fsp != NULL) {
+			smb_fname = fsp->base_fsp->fsp_name;
+		} else {
+			smb_fname = fsp->fsp_name;
+		}
 	}
 
 	DEBUG(10, ("ad_get(%s) called for %s\n",
diff --git a/source3/lib/adouble.h b/source3/lib/adouble.h
index 90a825c502e..a5761281670 100644
--- a/source3/lib/adouble.h
+++ b/source3/lib/adouble.h
@@ -164,6 +164,12 @@ bool ad_unconvert(TALLOC_CTX *mem_ctx,
 		  struct smb_filename *smb_fname,
 		  bool *converted);
 struct adouble *ad_init(TALLOC_CTX *ctx, adouble_type_t type);
+NTSTATUS adouble_open_from_base_fsp(const struct files_struct *dirfsp,
+				    struct files_struct *base_fsp,
+				    adouble_type_t type,
+				    int flags,
+				    mode_t mode,
+				    struct files_struct **_ad_fsp);
 struct adouble *ad_get(TALLOC_CTX *ctx,
 		       vfs_handle_struct *handle,
 		       const struct smb_filename *smb_fname,
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 80c04fdeda0..e8bb3e58e1f 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -880,8 +880,15 @@ struct share_mode_lock *get_share_mode_lock(
 
 	if (static_share_mode_data != NULL) {
 		if (!file_id_equal(&static_share_mode_data->id, &id)) {
-			DEBUG(1, ("Can not lock two share modes "
-				  "simultaneously\n"));
+			struct file_id_buf existing;
+			struct file_id_buf requested;
+
+			DBG_ERR("Can not lock two share modes "
+				"simultaneously: existing %s requested %s\n",
+				file_id_str_buf(static_share_mode_data->id, &existing),
+				file_id_str_buf(id, &requested));
+
+			smb_panic(__location__);
 			goto fail;
 		}
 		goto done;
@@ -904,6 +911,7 @@ struct share_mode_lock *get_share_mode_lock(
 	cmp = tdb_data_cmp(share_mode_lock_key, key);
 	if (cmp != 0) {
 		DBG_WARNING("Can not lock two share modes simultaneously\n");
+		smb_panic(__location__);
 		goto fail;
 	}
 
@@ -929,6 +937,15 @@ done:
 
 	talloc_set_destructor(lck, share_mode_lock_destructor);
 
+	if (CHECK_DEBUGLVL(DBGLVL_DEBUG)) {
+		struct file_id_buf returned;
+
+		DBG_DEBUG("Returning %s (data_refcount=%zu key_refcount=%zu)\n",
+			  file_id_str_buf(id, &returned),
+			  static_share_mode_data_refcount,
+			  share_mode_lock_key_refcount);
+	}
+
 	return lck;
 fail:
 	TALLOC_FREE(lck);
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index b25aebfa9ac..f5b5f91a012 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -172,9 +172,17 @@ static const struct enum_list fruit_encoding[] = {
 };
 
 struct fio {
+	vfs_handle_struct *handle;
+	files_struct *fsp; /* backlink to itself */
+
 	/* tcon config handle */
 	struct fruit_config_data *config;
 
+	/* Backend fsp for AppleDouble file, can be NULL */
+	files_struct *ad_fsp;
+	/* link from adouble_open_from_base_fsp() to fio */
+	struct fio *real_fio;
+
 	/* Denote stream type, meta or rsrc */
 	adouble_type_t type;
 
@@ -194,6 +202,27 @@ struct fio {
  * Helper functions
  *****************************************************************************/
 
+static struct fio *fruit_get_complete_fio(vfs_handle_struct *handle,
+					  files_struct *fsp)
+{
+	struct fio *fio = (struct fio *)VFS_FETCH_FSP_EXTENSION(handle, fsp);
+
+	if (fio == NULL) {
+		return NULL;
+	}
+
+	if (fio->real_fio != NULL) {
+		/*
+		 * This is an fsp from adouble_open_from_base_fsp()
+		 * we should just pass this to the next
+		 * module.
+		 */
+		return NULL;
+	}
+
+	return fio;
+}
+
 /**
  * Initialize config struct from our smb.conf config parameters
  **/
@@ -1313,6 +1342,32 @@ static int fruit_connect(vfs_handle_struct *handle,
 	return rc;
 }
 
+static void fio_ref_destroy_fn(void *p_data)
+{
+	struct fio *ref_fio = (struct fio *)p_data;
+	if (ref_fio->real_fio != NULL) {
+		SMB_ASSERT(ref_fio->real_fio->ad_fsp == ref_fio->fsp);
+		ref_fio->real_fio->ad_fsp = NULL;
+		ref_fio->real_fio = NULL;
+	}
+}
+
+static void fio_close_ad_fsp(struct fio *fio)
+{
+	if (fio->ad_fsp != NULL) {
+		fd_close(fio->ad_fsp);
+		file_free(NULL, fio->ad_fsp);
+		/* fio_ref_destroy_fn() should have cleared this */
+		SMB_ASSERT(fio->ad_fsp == NULL);
+	}
+}
+
+static void fio_destroy_fn(void *p_data)
+{
+	struct fio *fio = (struct fio *)p_data;
+	fio_close_ad_fsp(fio);
+}
+
 static int fruit_open_meta_stream(vfs_handle_struct *handle,
 				  const struct files_struct *dirfsp,
 				  const struct smb_filename *smb_fname,
@@ -1330,7 +1385,9 @@ static int fruit_open_meta_stream(vfs_handle_struct *handle,
 	SMB_VFS_HANDLE_GET_DATA(handle, config,
 				struct fruit_config_data, return -1);
 
-	fio = VFS_ADD_FSP_EXTENSION(handle, fsp, struct fio, NULL);
+	fio = VFS_ADD_FSP_EXTENSION(handle, fsp, struct fio, fio_destroy_fn);
+	fio->handle = handle;
+	fio->fsp = fsp;
 	fio->type = ADOUBLE_META;
 	fio->config = config;
 
@@ -1397,7 +1454,9 @@ static int fruit_open_meta_netatalk(vfs_handle_struct *handle,
 	SMB_VFS_HANDLE_GET_DATA(handle, config,
 				struct fruit_config_data, return -1);
 
-	fio = VFS_ADD_FSP_EXTENSION(handle, fsp, struct fio, NULL);
+	fio = VFS_ADD_FSP_EXTENSION(handle, fsp, struct fio, fio_destroy_fn);
+	fio->handle = handle;
+	fio->fsp = fsp;
 	fio->type = ADOUBLE_META;
 	fio->config = config;
 	fio->fake_fd = true;
@@ -1449,10 +1508,12 @@ static int fruit_open_rsrc_adouble(vfs_handle_struct *handle,
 				   mode_t mode)
 {
 	int rc = 0;
-	struct adouble *ad = NULL;
-	struct smb_filename *smb_fname_base = NULL;
 	struct fruit_config_data *config = NULL;
-	int hostfd = -1;
+	struct files_struct *ad_fsp = NULL;
+	struct fio *fio = NULL;
+	struct fio *ref_fio = NULL;
+	NTSTATUS status;
+	int fd = -1;
 
 	SMB_VFS_HANDLE_GET_DATA(handle, config,
 				struct fruit_config_data, return -1);
@@ -1461,68 +1522,82 @@ static int fruit_open_rsrc_adouble(vfs_handle_struct *handle,
 	    S_ISDIR(fsp->base_fsp->fsp_name->st.st_ex_mode))
 	{
 		/* sorry, but directories don't habe a resource fork */
+		errno = EISDIR;
 		rc = -1;
 		goto exit;
 	}
 
-	rc = adouble_path(talloc_tos(), smb_fname, &smb_fname_base);
-	if (rc != 0) {
+	/*
+	 * We return a fake_fd to the vfs modules above,
+	 * while we open an internal backend fsp for the
+	 * '._' file for the next vfs modules.
+	 *
+	 * Note that adouble_open_from_base_fsp() recurses
+	 * into fruit_openat(), but it'll just pass to
+	 * the next module as just opens a flat file on
+	 * disk.
+	 */
+
+	fd = vfs_fake_fd();
+	if (fd == -1) {
+		rc = fd;
 		goto exit;
 	}
 
-	/* We always need read/write access for the metadata header too */
-	flags &= ~(O_RDONLY | O_WRONLY);
-	flags |= O_RDWR;
-
-	hostfd = SMB_VFS_NEXT_OPENAT(handle,
-				     dirfsp,
-				     smb_fname_base,
-				     fsp,
-				     flags,
-				     mode);
-	if (hostfd == -1) {
+	status = adouble_open_from_base_fsp(dirfsp,
+					    fsp->base_fsp,
+					    ADOUBLE_RSRC,
+					    flags,
+					    mode,
+					    &ad_fsp);
+	if (!NT_STATUS_IS_OK(status)) {
+		errno = map_errno_from_nt_status(status);
 		rc = -1;
 		goto exit;
 	}
 
-	if (flags & (O_CREAT | O_TRUNC)) {
-		ad = ad_init(fsp, ADOUBLE_RSRC);
-		if (ad == NULL) {
-			rc = -1;
-			goto exit;
-		}
-
-		fsp_set_fd(fsp, hostfd);
+	/*
+	 * Now we need to glue both handles together,
+	 * so that they automatically detach each other
+	 * on close.
+	 */
+	fio = fruit_get_complete_fio(handle, fsp);
 
-		rc = ad_fset(handle, ad, fsp);
-		fsp_set_fd(fsp, -1);
-		if (rc != 0) {
-			rc = -1;
-			goto exit;
-		}
-		TALLOC_FREE(ad);
+	ref_fio = VFS_ADD_FSP_EXTENSION(handle, ad_fsp,
+					struct fio,
+					fio_ref_destroy_fn);
+	if (ref_fio == NULL) {
+		int saved_errno = errno;
+		fd_close(ad_fsp);
+		file_free(NULL, ad_fsp);
+		ad_fsp = NULL;
+		errno = saved_errno;
+		rc = -1;
+		goto exit;
 	}
 
-exit:
+	SMB_ASSERT(ref_fio->fsp == NULL);
+	ref_fio->handle = handle;
+	ref_fio->fsp = ad_fsp;
+	ref_fio->type = ADOUBLE_RSRC;
+	ref_fio->config = config;
+	ref_fio->real_fio = fio;
+	SMB_ASSERT(fio->ad_fsp == NULL);
+	fio->ad_fsp = ad_fsp;
+	fio->fake_fd = true;
 
-	TALLOC_FREE(smb_fname_base);
+exit:
 
-	DEBUG(10, ("fruit_open resource fork: rc=%d, fd=%d\n", rc, hostfd));
+	DEBUG(10, ("fruit_open resource fork: rc=%d\n", rc));
 	if (rc != 0) {
 		int saved_errno = errno;
-		if (hostfd >= 0) {
-			/*
-			 * BUGBUGBUG -- we would need to call
-			 * fd_close_posix here, but we don't have a
-			 * full fsp yet
-			 */
-			fsp_set_fd(fsp, hostfd);
-			SMB_VFS_CLOSE(fsp);


-- 
Samba Shared Repository



More information about the samba-cvs mailing list