Attachment 18727 Details for Bug 15897 – patch from master for v4-23-test

[patch] patch from master for v4-23-test

v4.23.patch (text/plain), 7.78 KB, created by Anoop C S on 2025-09-15 16:30:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Anoop C S

Created: 2025-09-15 16:30:33 UTC

Size: 7.78 KB

patch

obsolete

>From 49cf50ed298f123c250936e05c953d43d9e601a1 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Sun, 7 Sep 2025 21:56:30 +0200
>Subject: [PATCH 1/3] smbd: Add openat_pathref_fsp_dot()
>
>Very simple reopen of a directory as pathref. Too much magic in
>openat_pathref_fsp_lcomp() leads to Bug 15897:
>openat_pathref_fsp_lcomp() can return NT_STATUS_OK but still leave the
>file descriptor at -1 for msdfs and smb1 posix reasons. When using it
>in filename_convert_dirfsp_nosymlink() this bites us, the -1 can leak
>into vfswrap_openat(). Avoid any magic by directly calling
>SMB_VFS_OPENAT() with maximum NOFOLLOW/etc safety for this use case
>and fail when this does not work.
>
>This adds another flavor of openat_pathref_fsp, and at some point we
>need to consolidate them again.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=15897
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Anoop C S <anoopcs@samba.org>
>(cherry picked from commit 83ece80ecc2baa52a3caa0ee3b0f954b005b2268)
>---
> source3/smbd/files.c | 108 +++++++++++++++++++++++++++++++++++++++++++
> source3/smbd/proto.h |   4 ++
> 2 files changed, 112 insertions(+)
>
>diff --git a/source3/smbd/files.c b/source3/smbd/files.c
>index 4cc203d8a1a..3cf1e78a704 100644
>--- a/source3/smbd/files.c
>+++ b/source3/smbd/files.c
>@@ -1664,6 +1664,114 @@ NTSTATUS openat_pathref_fsp_lcomp(struct files_struct *dirfsp,
> 	return NT_STATUS_OK;
> }
> 
>+NTSTATUS openat_pathref_fsp_dot(TALLOC_CTX *mem_ctx,
>+				struct files_struct *dirfsp,
>+				uint32_t flags,
>+				struct smb_filename **_dot)
>+{
>+	struct connection_struct *conn = dirfsp->conn;
>+	struct files_struct *fsp = NULL;
>+	struct smb_filename *full_fname = NULL;
>+        struct vfs_open_how how = {
>+                .flags = O_RDONLY | O_NONBLOCK | O_NOFOLLOW,
>+        };
>+        struct smb_filename *dot = NULL;
>+        NTSTATUS status;
>+        int fd;
>+
>+#ifdef O_DIRECTORY
>+        how.flags |= O_DIRECTORY;
>+#endif
>+
>+#ifdef O_PATH
>+        how.flags = O_PATH;
>+#endif
>+
>+	dot = synthetic_smb_fname(mem_ctx, ".", NULL, NULL, 0, flags);
>+	if (dot == NULL) {
>+		return NT_STATUS_NO_MEMORY;
>+	}
>+
>+	status = fsp_new(conn, conn, &fsp);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		DBG_DEBUG("fsp_new() failed: %s\n", nt_errstr(status));
>+		return status;
>+	}
>+
>+	GetTimeOfDay(&fsp->open_time);
>+	fsp_set_gen_id(fsp);
>+	ZERO_STRUCT(conn->sconn->fsp_fi_cache);
>+
>+	fsp->fsp_flags.is_pathref = true;
>+
>+	full_fname = full_path_from_dirfsp_atname(conn, dirfsp, dot);
>+	if (full_fname == NULL) {
>+		DBG_DEBUG("full_path_from_dirfsp_atname(%s/%s) failed\n",
>+			  dirfsp->fsp_name->base_name,
>+			  dot->base_name);
>+		file_free(NULL, fsp);
>+		return NT_STATUS_NO_MEMORY;
>+	}
>+
>+	status = fsp_attach_smb_fname(fsp, &full_fname);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		DBG_DEBUG("fsp_attach_smb_fname(fsp, %s) failed: %s\n",
>+			  smb_fname_str_dbg(full_fname),
>+			  nt_errstr(status));
>+		file_free(NULL, fsp);
>+		return status;
>+	}
>+
>+	fd = SMB_VFS_OPENAT(conn, dirfsp, dot, fsp, &how);
>+	if (fd == -1) {
>+		status = map_nt_error_from_unix(errno);
>+		DBG_DEBUG("smb_vfs_openat(%s/%s) failed: %s\n",
>+			  dirfsp->fsp_name->base_name,
>+			  dot->base_name,
>+			  strerror(errno));
>+		file_free(NULL, fsp);
>+		return status;
>+	}
>+
>+	fsp_set_fd(fsp, fd);
>+
>+	status = vfs_stat_fsp(fsp);
>+
>+	if (!NT_STATUS_IS_OK(status)) {
>+		DBG_DEBUG("vfs_stat_fsp(\"/\") failed: %s\n",
>+			  nt_errstr(status));
>+		fd_close(fsp);
>+		file_free(NULL, fsp);
>+		return status;
>+	}
>+
>+	fsp->fsp_flags.is_directory = S_ISDIR(fsp->fsp_name->st.st_ex_mode);
>+	fsp->fsp_flags.posix_open =
>+		((dot->flags & SMB_FILENAME_POSIX_PATH) != 0);
>+	fsp->file_id = vfs_file_id_from_sbuf(conn, &fsp->fsp_name->st);
>+
>+	dot->st = fsp->fsp_name->st;
>+
>+	status = fsp_smb_fname_link(fsp,
>+				    &dot->fsp_link,
>+				    &dot->fsp);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		DBG_DEBUG("fsp_smb_fname_link() failed: %s\n",
>+			  nt_errstr(status));
>+		fd_close(fsp);
>+		file_free(NULL, fsp);
>+		return status;
>+	}
>+
>+	DBG_DEBUG("fsp [%s]: OK, fd=%d\n", fsp_str_dbg(fsp), fd);
>+
>+	talloc_set_destructor(dot, smb_fname_fsp_destructor);
>+
>+	*_dot = dot;
>+
>+	return NT_STATUS_OK;
>+}
>+
> void smb_fname_fsp_unlink(struct smb_filename *smb_fname)
> {
> 	talloc_set_destructor(smb_fname, NULL);
>diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
>index 08506dec743..3b67efcbe23 100644
>--- a/source3/smbd/proto.h
>+++ b/source3/smbd/proto.h
>@@ -401,6 +401,10 @@ NTSTATUS openat_pathref_fsp_nosymlink(
> NTSTATUS openat_pathref_fsp_lcomp(struct files_struct *dirfsp,
> 				  struct smb_filename *smb_fname_rel,
> 				  uint32_t ucf_flags);
>+NTSTATUS openat_pathref_fsp_dot(TALLOC_CTX *mem_ctx,
>+				struct files_struct *dirfsp,
>+				uint32_t flags,
>+				struct smb_filename **_dot);
> NTSTATUS readlink_talloc(
> 	TALLOC_CTX *mem_ctx,
> 	struct files_struct *dirfsp,
>-- 
>2.51.0
>
>
>From fe8492b8e83e78b105ce9564c1ae8be234d70da3 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Sun, 7 Sep 2025 21:57:27 +0200
>Subject: [PATCH 2/3] smbd: Fix Bug 15897
>
>Don't leak smb_dirname->fsp->fh->fd == -1 coming from
>openat_pathref_fsp_lcomp().
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=15897
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Anoop C S <anoopcs@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>Autobuild-Date(master): Wed Sep 10 09:37:33 UTC 2025 on atb-devel-224
>
>(cherry picked from commit 7143caeecc856d3326fdc3eb466ef1f37bc564b5)
>---
> source3/smbd/filename.c | 16 +++-------------
> 1 file changed, 3 insertions(+), 13 deletions(-)
>
>diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
>index 6a9d5f99d2a..ec2f65553b6 100644
>--- a/source3/smbd/filename.c
>+++ b/source3/smbd/filename.c
>@@ -767,19 +767,9 @@ filename_convert_dirfsp_nosymlink(TALLOC_CTX *mem_ctx,
> 	}
> 
> 	if (dirname[0] == '\0') {
>-		smb_dirname = synthetic_smb_fname(
>-			mem_ctx,
>-			".",
>-			NULL,
>-			NULL,
>-			0,
>-			posix ? SMB_FILENAME_POSIX_PATH : 0);
>-		if (smb_dirname == NULL) {
>-			return NT_STATUS_NO_MEMORY;
>-		}
>-		status = openat_pathref_fsp_lcomp(basedir,
>-						  smb_dirname,
>-						  UCF_POSIX_PATHNAMES);
>+		status = openat_pathref_fsp_dot(
>+			mem_ctx, basedir,
>+			posix ? SMB_FILENAME_POSIX_PATH : 0, &smb_dirname);
> 	} else {
> 		status = normalize_filename_case(conn, dirname, ucf_flags);
> 		if (!NT_STATUS_IS_OK(status)) {
>-- 
>2.51.0
>
>
>From edeb8179e12ec0cd2efeea8e34e2f41d1f294ab7 Mon Sep 17 00:00:00 2001
>From: Anoop C S <anoopcs@samba.org>
>Date: Thu, 11 Sep 2025 11:41:25 +0530
>Subject: [PATCH 3/3] smbd: Fix CID 1665417, UNUSED_VALUE in
> openat_pathref_fsp_dot()
>
>Rearrange how the open flags are defined so as to make Coverity happy
>about it.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15897
>
>Signed-off-by: Anoop C S <anoopcs@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>
>Autobuild-User(master): Anoop C S <anoopcs@samba.org>
>Autobuild-Date(master): Mon Sep 15 15:44:38 UTC 2025 on atb-devel-224
>
>(cherry picked from commit 1ec54347acac241f900b52c16c2dcba04164e898)
>---
> source3/smbd/files.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/source3/smbd/files.c b/source3/smbd/files.c
>index 3cf1e78a704..334810e45f9 100644
>--- a/source3/smbd/files.c
>+++ b/source3/smbd/files.c
>@@ -1672,9 +1672,7 @@ NTSTATUS openat_pathref_fsp_dot(TALLOC_CTX *mem_ctx,
> 	struct connection_struct *conn = dirfsp->conn;
> 	struct files_struct *fsp = NULL;
> 	struct smb_filename *full_fname = NULL;
>-        struct vfs_open_how how = {
>-                .flags = O_RDONLY | O_NONBLOCK | O_NOFOLLOW,
>-        };
>+	struct vfs_open_how how = { .flags = O_NOFOLLOW, };
>         struct smb_filename *dot = NULL;
>         NTSTATUS status;
>         int fd;
>@@ -1684,7 +1682,9 @@ NTSTATUS openat_pathref_fsp_dot(TALLOC_CTX *mem_ctx,
> #endif
> 
> #ifdef O_PATH
>-        how.flags = O_PATH;
>+	how.flags |= O_PATH;
>+#else
>+	how.flags |= (O_RDONLY | O_NONBLOCK);
> #endif
> 
> 	dot = synthetic_smb_fname(mem_ctx, ".", NULL, NULL, 0, flags);
>-- 
>2.51.0
>

Flags:

anoopcs: review? (slow)

Actions: View

Attachments on bug 15897: 18708 | 18714 | 18727 | 18758 | 18759