From efc33d6b82a78a5289690bbd90dde9460ed6f78d Mon Sep 17 00:00:00 2001 From: Anoop C S Date: Mon, 24 Feb 2025 11:54:45 +0530 Subject: [PATCH 1/4] vfs_ceph_new: Remove redundant re-intialization to NULL TALLOC_FREE() by default re-initializes the pointer to NULL after corresponding memory is freed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15818 Signed-off-by: Anoop C S Reviewed-by: Guenther Deschner (cherry picked from commit c5ddd94a08503a52914ce351ebf1083178e8c8bc) --- source3/modules/vfs_ceph_new.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/source3/modules/vfs_ceph_new.c b/source3/modules/vfs_ceph_new.c index cae8317b1c8..e3853cdefc4 100644 --- a/source3/modules/vfs_ceph_new.c +++ b/source3/modules/vfs_ceph_new.c @@ -698,10 +698,7 @@ static struct dirent *vfs_ceph_get_fh_dirent(struct vfs_ceph_fh *cfh) static void vfs_ceph_put_fh_dirent(struct vfs_ceph_fh *cfh) { - if (cfh->de != NULL) { - TALLOC_FREE(cfh->de); - cfh->de = NULL; - } + TALLOC_FREE(cfh->de); } static int vfs_ceph_release_fh(struct vfs_ceph_fh *cfh) -- 2.48.1 From 44211f09a736233a868f280a9937df02bf6bd850 Mon Sep 17 00:00:00 2001 From: Anoop C S Date: Mon, 24 Feb 2025 12:09:06 +0530 Subject: [PATCH 2/4] vfs_ceph_new: Remove unused code in cephmount_mount_fs() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15818 Signed-off-by: Anoop C S Reviewed-by: Guenther Deschner (cherry picked from commit ee1c3e1db9a2d12ba6d9dd24faccf0020b1daf0d) --- source3/modules/vfs_ceph_new.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/source3/modules/vfs_ceph_new.c b/source3/modules/vfs_ceph_new.c index e3853cdefc4..ec39e1c8b91 100644 --- a/source3/modules/vfs_ceph_new.c +++ b/source3/modules/vfs_ceph_new.c @@ -274,7 +274,6 @@ static struct ceph_mount_info *cephmount_mount_fs( struct vfs_ceph_config *config) { int ret; - char buf[256]; struct ceph_mount_info *mnt = NULL; /* if config_file and/or user_id are NULL, ceph will use defaults */ @@ -294,12 +293,6 @@ static struct ceph_mount_info *cephmount_mount_fs( goto out; } - DBG_DEBUG("[CEPH] calling ceph_conf_get: option='%s'\n", "log_file"); - ret = config->ceph_conf_get_fn(mnt, "log_file", buf, sizeof(buf)); - if (ret < 0) { - goto out; - } - /* libcephfs disables POSIX ACL support by default, enable it... */ ret = cephmount_update_conf(config, mnt, -- 2.48.1 From 11ba515cbb019c1b91fbfcb537d82c66ce0e94c4 Mon Sep 17 00:00:00 2001 From: Anoop C S Date: Mon, 24 Feb 2025 14:00:56 +0530 Subject: [PATCH 3/4] vfs_ceph_new: Handle absolute path in vfs_ceph_ll_walk It can very well be the case that the incoming path is absolute in nature which breaks the assumption inside vfs_ceph_ll_walk that it is within the current working directory. Instead perform a check to see whether the path includes current working directory path in its components and accordingly trim it to make it relative in nature. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15818 Signed-off-by: Anoop C S Reviewed-by: Guenther Deschner (cherry picked from commit 9341d7fb466c95ea5aa0643049ce2a1f4183b9d0) --- source3/modules/vfs_ceph_new.c | 37 ++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/source3/modules/vfs_ceph_new.c b/source3/modules/vfs_ceph_new.c index ec39e1c8b91..0a8faf1d835 100644 --- a/source3/modules/vfs_ceph_new.c +++ b/source3/modules/vfs_ceph_new.c @@ -813,10 +813,31 @@ static int vfs_ceph_ll_walk(const struct vfs_handle_struct *handle, struct UserPerm *uperm = NULL; int ret = -1; struct vfs_ceph_config *config = NULL; + const char *cwd = NULL; + size_t cwdlen; SMB_VFS_HANDLE_GET_DATA(handle, config, struct vfs_ceph_config, return -ENOMEM); + cwd = config->ceph_getcwd_fn(config->mount); + cwdlen = strlen(cwd); + + /* + * ceph_ll_walk() always operate on "name" relative to current working + * directory even if it starts with a '/' i.e, absolute path is never + * honoured. But why?? For now stick to the current behaviour and ensure + * that the "name" is always relative when it contains current working + * directory path with an exception to "/". + */ + if ((strcmp(cwd, "/") != 0) && + (strncmp(name, cwd, cwdlen) == 0)) { + if (name[cwdlen] == '/') { + name += cwdlen + 1; + } else if (name[cwdlen] == '\0') { + name = "."; + } + } + DBG_DEBUG("[CEPH] ceph_ll_walk: name=%s\n", name); uperm = vfs_ceph_userperm_new(config, handle->conn); @@ -1827,21 +1848,7 @@ static int vfs_ceph_iget_by_fname(const struct vfs_handle_struct *handle, const struct smb_filename *smb_fname, struct vfs_ceph_iref *iref) { - const char *name = smb_fname->base_name; - const char *cwd = NULL; - int ret = -1; - struct vfs_ceph_config *config = NULL; - - SMB_VFS_HANDLE_GET_DATA(handle, config, struct vfs_ceph_config, - return -ENOMEM); - - cwd = config->ceph_getcwd_fn(config->mount); - if (!strcmp(name, cwd)) { - ret = vfs_ceph_iget(handle, 0, "./", 0, iref); - } else { - ret = vfs_ceph_iget(handle, 0, name, 0, iref); - } - return ret; + return vfs_ceph_iget(handle, 0, smb_fname->base_name, 0, iref); } static int vfs_ceph_igetl(const struct vfs_handle_struct *handle, -- 2.48.1 From 7a89b1d0d905e04813e93f7ed4ea2afa674a16aa Mon Sep 17 00:00:00 2001 From: Anoop C S Date: Tue, 25 Feb 2025 17:40:13 +0530 Subject: [PATCH 4/4] vfs_ceph_new: Do not resolve by inode number MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CephFS snapshots within snap directory shares the same inode number from its parent. Until unless we resolve by name we may incorrectly point at an inode which is not a snapshot directory. Therefore to be functionally correct we avoid resolving by inode number but proper name. For example: path (ino = 3) | --- dir (ino = 4) | --- .snap (ino = 3) | --- snap1 (ino = 3) | --- dir (ino = 4) In this case an attempt to resolve 'snap1' by inode number 3 results in pointing at 'path' which is not the desired outcome. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15818 Signed-off-by: Anoop C S Reviewed-by: Guenther Deschner Autobuild-User(master): Günther Deschner Autobuild-Date(master): Fri Mar 7 18:20:47 UTC 2025 on atb-devel-224 (cherry picked from commit a96f0542c8317a7dd0470b32350de6893fd98723) --- source3/modules/vfs_ceph_new.c | 100 +++++++++------------------------ 1 file changed, 25 insertions(+), 75 deletions(-) diff --git a/source3/modules/vfs_ceph_new.c b/source3/modules/vfs_ceph_new.c index 0a8faf1d835..eefe638a135 100644 --- a/source3/modules/vfs_ceph_new.c +++ b/source3/modules/vfs_ceph_new.c @@ -110,7 +110,6 @@ struct vfs_ceph_config { enum vfs_cephfs_proxy_mode proxy; void *libhandle; - CEPH_FN(ceph_ll_lookup_inode); CEPH_FN(ceph_ll_walk); CEPH_FN(ceph_ll_getattr); CEPH_FN(ceph_ll_setattr); @@ -389,7 +388,6 @@ static bool vfs_cephfs_load_lib(struct vfs_ceph_config *config) break; } - CHECK_CEPH_FN(libhandle, ceph_ll_lookup_inode); CHECK_CEPH_FN(libhandle, ceph_ll_walk); CHECK_CEPH_FN(libhandle, ceph_ll_getattr); CHECK_CEPH_FN(libhandle, ceph_ll_setattr); @@ -788,21 +786,6 @@ static void vfs_ceph_assign_fh_fd(struct vfs_ceph_fh *cfh) /* Ceph low-level wrappers */ -static int vfs_ceph_ll_lookup_inode(const struct vfs_handle_struct *handle, - uint64_t inoval, - Inode **pout) -{ - struct inodeno_t ino = {.val = inoval}; - struct vfs_ceph_config *config = NULL; - - SMB_VFS_HANDLE_GET_DATA(handle, config, struct vfs_ceph_config, - return -ENOMEM); - - DBG_DEBUG("[CEPH] ceph_ll_lookup_inode: ino=%" PRIu64 "\n", inoval); - - return config->ceph_ll_lookup_inode_fn(config->mount, ino, pout); -} - static int vfs_ceph_ll_walk(const struct vfs_handle_struct *handle, const char *name, struct Inode **pin, @@ -1808,60 +1791,30 @@ static int vfs_ceph_ll_fremovexattr(const struct vfs_handle_struct *handle, /* Ceph Inode-refernce get/put wrappers */ static int vfs_ceph_iget(const struct vfs_handle_struct *handle, - uint64_t ino, const char *name, unsigned int flags, struct vfs_ceph_iref *iref) { struct Inode *inode = NULL; int ret = -1; + struct ceph_statx stx = {.stx_ino = 0}; - if (ino > CEPH_INO_ROOT) { - /* get-by-ino */ - ret = vfs_ceph_ll_lookup_inode(handle, ino, &inode); - if (ret != 0) { - return ret; - } - } else { - /* get-by-path */ - struct ceph_statx stx = {.stx_ino = 0}; - - ret = vfs_ceph_ll_walk(handle, - name, - &inode, - &stx, - CEPH_STATX_INO, - flags); - if (ret != 0) { - return ret; - } - ino = stx.stx_ino; + ret = vfs_ceph_ll_walk(handle, + name, + &inode, + &stx, + CEPH_STATX_INO, + flags); + if (ret != 0) { + return ret; } iref->inode = inode; - iref->ino = ino; + iref->ino = stx.stx_ino; iref->owner = true; DBG_DEBUG("[CEPH] iget: %s ino=%" PRIu64 "\n", name, iref->ino); return 0; } -static int vfs_ceph_iget_by_fname(const struct vfs_handle_struct *handle, - const struct smb_filename *smb_fname, - struct vfs_ceph_iref *iref) -{ - return vfs_ceph_iget(handle, 0, smb_fname->base_name, 0, iref); -} - -static int vfs_ceph_igetl(const struct vfs_handle_struct *handle, - const struct smb_filename *smb_fname, - struct vfs_ceph_iref *iref) -{ - return vfs_ceph_iget(handle, - 0, - smb_fname->base_name, - AT_SYMLINK_NOFOLLOW, - iref); -} - static int vfs_ceph_igetd(struct vfs_handle_struct *handle, const struct files_struct *dirfsp, struct vfs_ceph_iref *iref) @@ -1880,25 +1833,16 @@ static int vfs_ceph_igetd(struct vfs_handle_struct *handle, /* case-2: resolve by current work-dir */ if (fsp_get_pathref_fd(dirfsp) == AT_FDCWD) { - return vfs_ceph_iget(handle, 0, ".", 0, iref); + return vfs_ceph_iget(handle, ".", 0, iref); } /* case-3: resolve by parent dir and name */ return vfs_ceph_iget(handle, - dirfsp->file_id.inode, dirfsp->fsp_name->base_name, AT_SYMLINK_NOFOLLOW, iref); } -static int vfs_ceph_igetf(struct vfs_handle_struct *handle, - const struct files_struct *fsp, - struct vfs_ceph_iref *iref) -{ - return vfs_ceph_iget( - handle, fsp->file_id.inode, fsp->fsp_name->base_name, 0, iref); -} - static void vfs_ceph_iput(const struct vfs_handle_struct *handle, struct vfs_ceph_iref *iref) { @@ -1963,7 +1907,7 @@ static int vfs_ceph_statvfs(struct vfs_handle_struct *handle, struct vfs_ceph_iref iref = {0}; int ret; - ret = vfs_ceph_iget_by_fname(handle, smb_fname, &iref); + ret = vfs_ceph_iget(handle, smb_fname->base_name, 0, &iref); if (ret != 0) { goto out; } @@ -2677,7 +2621,7 @@ static int vfs_ceph_stat(struct vfs_handle_struct *handle, goto out; } - result = vfs_ceph_iget_by_fname(handle, smb_fname, &iref); + result = vfs_ceph_iget(handle, smb_fname->base_name, 0, &iref); if (result != 0) { goto out; } @@ -2773,7 +2717,10 @@ static int vfs_ceph_lstat(struct vfs_handle_struct *handle, goto out; } - result = vfs_ceph_igetl(handle, smb_fname, &iref); + result = vfs_ceph_iget(handle, + smb_fname->base_name, + AT_SYMLINK_NOFOLLOW, + &iref); if (result != 0) { goto out; } @@ -2911,7 +2858,10 @@ static int vfs_ceph_lchown(struct vfs_handle_struct *handle, uid, gid); - result = vfs_ceph_igetl(handle, smb_fname, &iref); + result = vfs_ceph_iget(handle, + smb_fname->base_name, + AT_SYMLINK_NOFOLLOW, + &iref); if (result != 0) { goto out; } @@ -3424,7 +3374,7 @@ static ssize_t vfs_ceph_fgetxattr(struct vfs_handle_struct *handle, } else { struct vfs_ceph_iref iref = {0}; - ret = vfs_ceph_igetf(handle, fsp, &iref); + ret = vfs_ceph_iget(handle, fsp->fsp_name->base_name, 0, &iref); if (ret != 0) { goto out; } @@ -3467,7 +3417,7 @@ static ssize_t vfs_ceph_flistxattr(struct vfs_handle_struct *handle, } else { struct vfs_ceph_iref iref = {0}; - ret = vfs_ceph_igetf(handle, fsp, &iref); + ret = vfs_ceph_iget(handle, fsp->fsp_name->base_name, 0, &iref); if (ret != 0) { goto out; } @@ -3506,7 +3456,7 @@ static int vfs_ceph_fremovexattr(struct vfs_handle_struct *handle, } else { struct vfs_ceph_iref iref = {0}; - ret = vfs_ceph_igetf(handle, fsp, &iref); + ret = vfs_ceph_iget(handle, fsp->fsp_name->base_name, 0, &iref); if (ret != 0) { goto out; } @@ -3552,7 +3502,7 @@ static int vfs_ceph_fsetxattr(struct vfs_handle_struct *handle, } else { struct vfs_ceph_iref iref = {0}; - ret = vfs_ceph_igetf(handle, fsp, &iref); + ret = vfs_ceph_iget(handle, fsp->fsp_name->base_name, 0, &iref); if (ret != 0) { goto out; } -- 2.48.1