Attachment 18708 Details for Bug 15897 – Patch

Patch

bug-15897.txt (text/plain), 5.86 KB, created by Volker Lendecke on 2025-09-08 08:05:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Volker Lendecke

Created: 2025-09-08 08:05:45 UTC

Size: 5.86 KB

patch

obsolete

>From 378df7412fa12bbe6a3da86d4e1d049a16e7ada2 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/2] smbd: Add openat_pathref_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>
>---
> 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 4cc203d8a1..d1deda8c71 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_dot(TALLOC_CTX *mem_ctx,
>+			    struct files_struct *dirfsp,
>+			    uint32_t ucf_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 ret, 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, ucf_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 08506dec74..832b1d2c2f 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_dot(TALLOC_CTX *mem_ctx,
>+			    struct files_struct *dirfsp,
>+			    uint32_t ucf_flags,
>+			    struct smb_filename **_dot);
> NTSTATUS readlink_talloc(
> 	TALLOC_CTX *mem_ctx,
> 	struct files_struct *dirfsp,
>-- 
>2.50.1
>
>
>From fff80ba75dbffa23177383ff7aa6a9134787c120 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/2] 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>
>---
> 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 6a9d5f99d2..61ec44ddc7 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_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.50.1
>

Actions: View

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