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 3aaf070d5adb7a7a99ec9811b8a52549d23c9fee..7ac7df78faf4a0e3c21235c63878608b2648b53d 100644 (file)
--- 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 a5a9a3aa42d8955c7feccb536d7482a2f07337da..d739de6637e40a0caa82d662e356726b49224b3f 100644 (file)
--- 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 03e33babcbf636e4b3000e0d35484fb7a9e4819f..eb584f3d1ffbdedd78ca1b2bca879a2abe127bd5 100644 (file)
--- 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);