From: Volker Lendecke <vl@samba.org>
Date: Thu, 10 Oct 2024 15:16:02 +0000 (+0200)
Subject: smbd: Simplify OpenDir_from_pathref()
X-Git-Tag: tdb-1.4.13~526
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c0bbeded932f00b0cdc3954d549d6566c85fdc0b;p=thirdparty%2Fsamba.git

smbd: Simplify OpenDir_from_pathref()

Use the /proc/self/fd trick to make get_real_filename_fullscan_at look
a bit nicer and faster in strace. Direct SMB_VFS_OPENAT also is
cheaper in user space, we don't need the full fd_openat and
non_widelink_open magic here.

Also avoid opening ".", which can fail where a full path open would
succeed: If the directory in question does not give "x" perms to the
user, we get a handle on the dir as such but can't cd into it.

I haven't seen real-world cases of this, but one of our tests creates
such a scenario. I have further refactoring in my local tree that make
this patch necessary.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
---

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 3aaf070d5ad..7ac7df78faf 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -1066,19 +1066,55 @@ NTSTATUS OpenDir_from_pathref(TALLOC_CTX *mem_ctx,
 			      uint32_t attr,
 			      struct smb_Dir **_dir_hnd)
 {
-	struct files_struct *fsp = NULL;
+	struct connection_struct *conn = dirfsp->conn;
+	struct files_struct *new_fsp = NULL;
 	struct smb_Dir *dir_hnd = NULL;
+	const struct vfs_open_how how = {.flags = O_RDONLY | O_DIRECTORY};
+	int old_fd;
 	NTSTATUS status;
 
-	status = openat_internal_dir_from_pathref(dirfsp, O_RDONLY, &fsp);
+	status = create_internal_dirfsp(conn, dirfsp->fsp_name, &new_fsp);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
 
-	status = OpenDir_fsp(mem_ctx, fsp->conn, fsp, mask, attr, &dir_hnd);
+	if (dirfsp->fsp_flags.have_proc_fds &&
+	    ((old_fd = fsp_get_pathref_fd(dirfsp)) != -1))
+	{
+		struct sys_proc_fd_path_buf buf;
+		struct smb_filename proc_fname = {
+			.base_name = sys_proc_fd_path(old_fd, &buf),
+		};
+		int new_fd;
+
+		new_fd = SMB_VFS_OPENAT(
+			conn, conn->cwd_fsp, &proc_fname, new_fsp, &how);
+		if (new_fd == -1) {
+			status = map_nt_error_from_unix(errno);
+			DBG_DEBUG("SMB_VFS_OPENAT(%s) returned %s\n",
+				  proc_fname.base_name, strerror(errno));
+			file_free(NULL, new_fsp);
+			return status;
+		}
+		fsp_set_fd(new_fsp, new_fd);
+	} else {
+		status = fd_openat(conn->cwd_fsp,
+				   dirfsp->fsp_name,
+				   new_fsp,
+				   &how);
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_DEBUG("fd_openat(%s) returned %s\n",
+				  dirfsp->fsp_name->base_name,
+				  nt_errstr(status));
+			file_free(NULL, new_fsp);
+			return status;
+		}
+	}
+
+	status = OpenDir_fsp(mem_ctx, conn, new_fsp, mask, attr, &dir_hnd);
 	if (!NT_STATUS_IS_OK(status)) {
-		fd_close(fsp);
-		file_free(NULL, fsp);
+		fd_close(new_fsp);
+		file_free(NULL, new_fsp);
 		return status;
 	}
 
diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index a5a9a3aa42d..d739de6637e 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -286,52 +286,6 @@ NTSTATUS open_internal_dirfsp(connection_struct *conn,
 	return NT_STATUS_OK;
 }
 
-/*
- * Convert a pathref dirfsp into a real fsp. No need to do any cwd
- * tricks, we just open ".".
- */
-NTSTATUS openat_internal_dir_from_pathref(
-	struct files_struct *dirfsp,
-	int _open_flags,
-	struct files_struct **_fsp)
-{
-	struct connection_struct *conn = dirfsp->conn;
-	struct smb_filename *smb_dname = dirfsp->fsp_name;
-	struct files_struct *fsp = NULL;
-	char dot[] = ".";
-	struct smb_filename smb_dot = {
-		.base_name = dot,
-		.flags = smb_dname->flags,
-		.twrp = smb_dname->twrp,
-	};
-	struct vfs_open_how how = { .flags = _open_flags, };
-	NTSTATUS status;
-
-	status = create_internal_dirfsp(conn, smb_dname, &fsp);
-	if (!NT_STATUS_IS_OK(status)) {
-		return status;
-	}
-
-	/*
-	 * Pointless for opening ".", but you never know...
-	 */
-	how.flags |= O_NOFOLLOW;
-
-	status = fd_openat(dirfsp, &smb_dot, fsp, &how);
-	if (!NT_STATUS_IS_OK(status)) {
-		DBG_INFO("fd_openat(\"%s\", \".\") failed: %s\n",
-			 fsp_str_dbg(dirfsp),
-			 nt_errstr(status));
-		file_free(NULL, fsp);
-		return status;
-	}
-
-	fsp->fsp_name->st = smb_dname->st;
-	fsp->file_id = vfs_file_id_from_sbuf(conn, &fsp->fsp_name->st);
-	*_fsp = fsp;
-	return NT_STATUS_OK;
-}
-
 /*
  * The "link" in the name doesn't imply link in the filesystem
  * sense. It's a object that "links" together an fsp and an smb_fname
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 03e33babcbf..eb584f3d1ff 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -366,10 +366,6 @@ NTSTATUS open_internal_dirfsp(connection_struct *conn,
 			      const struct smb_filename *smb_dname,
 			      int open_flags,
 			      struct files_struct **_fsp);
-NTSTATUS openat_internal_dir_from_pathref(
-	struct files_struct *dirfsp,
-	int open_flags,
-	struct files_struct **_fsp);
 
 NTSTATUS openat_pathref_fsp(const struct files_struct *dirfsp,
 			    struct smb_filename *smb_fname);