Attachment 18641 Details for Bug 15819 – patch from master for v4-21-test

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

v4.21.patch (text/plain), 5.08 KB, created by Anoop C S on 2025-04-30 14:36:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Anoop C S

Created: 2025-04-30 14:36:55 UTC

Size: 5.08 KB

patch

obsolete

>From aacc5f02fbb7bce2913800e874992dd883a92236 Mon Sep 17 00:00:00 2001
>From: Anoop C S <anoopcs@samba.org>
>Date: Tue, 4 Mar 2025 15:09:33 +0530
>Subject: [PATCH 1/2] vfs_ceph_snapshots: Use full path from dirfsp at
> smb_fname
>
>In ceph_snap_gmt_openat() we hand in the incoming smb_fname as it is
>to ceph_snap_gmt_strip_snapshot() which is then passed on to derive
>the actual snapshot path using ceph_snap_gmt_convert(). But this can
>go wrong in ceph_snap_gmt_convert_dir() while opening the snapdir.
>Unless we constitute the full path from dirfsp at the first place we
>always end up opening the snapdir from the parent directory with
>OpenDir().
>
>For example with dirfsp("foobar") and smb_fname("shift.txt"), we open
>snapdir from share root because parent is calculated as empty string
>via ceph_snap_get_parent_path(). Instead we could construct the full
>path from dirfsp using full_path_from_dirfsp_atname() to ensure we
>don't open the wrong snapdir.
>
>Since we have access to the twrp token at VFS layer it doesn't make
>much sense to make use of ceph_snap_gmt_strip_snapshot() in openat.
>We could instead directly act based on already available twrp token
>avoiding an extra copy of incoming smb_filename.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15819
>
>Signed-off-by: Anoop C S <anoopcs@samba.org>
>Reviewed-by: David Disseldorp ddiss@samba.org
>(cherry picked from commit ad8b2dbb67d87db22f2fa2df814dd91cbe071e60)
>---
> source3/modules/vfs_ceph_snapshots.c | 29 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
>
>diff --git a/source3/modules/vfs_ceph_snapshots.c b/source3/modules/vfs_ceph_snapshots.c
>index 98b8f5f6b5d..7400bef008e 100644
>--- a/source3/modules/vfs_ceph_snapshots.c
>+++ b/source3/modules/vfs_ceph_snapshots.c
>@@ -938,21 +938,11 @@ static int ceph_snap_gmt_openat(vfs_handle_struct *handle,
> {
> 	time_t timestamp = 0;
> 	struct smb_filename *smb_fname = NULL;
>-	char stripped[PATH_MAX + 1];
> 	char conv[PATH_MAX + 1];
> 	int ret;
> 	int saved_errno = 0;
> 
>-	ret = ceph_snap_gmt_strip_snapshot(handle,
>-					   smb_fname_in,
>-					   &timestamp,
>-					   stripped,
>-					   sizeof(stripped));
>-	if (ret < 0) {
>-		errno = -ret;
>-		return -1;
>-	}
>-	if (timestamp == 0) {
>+	if (smb_fname_in->twrp == 0) {
> 		return SMB_VFS_NEXT_OPENAT(handle,
> 					   dirfsp,
> 					   smb_fname_in,
>@@ -960,8 +950,18 @@ static int ceph_snap_gmt_openat(vfs_handle_struct *handle,
> 					   how);
> 	}
> 
>+	timestamp = nt_time_to_unix(smb_fname_in->twrp);
>+
>+	smb_fname = full_path_from_dirfsp_atname(talloc_tos(),
>+						 dirfsp,
>+						 smb_fname_in);
>+	if (smb_fname == NULL) {
>+		errno = ENOMEM;
>+		return -1;
>+	}
>+
> 	ret = ceph_snap_gmt_convert(handle,
>-				    stripped,
>+				    smb_fname->base_name,
> 				    timestamp,
> 				    conv,
> 				    sizeof(conv));
>@@ -969,10 +969,7 @@ static int ceph_snap_gmt_openat(vfs_handle_struct *handle,
> 		errno = -ret;
> 		return -1;
> 	}
>-	smb_fname = cp_smb_filename(talloc_tos(), smb_fname_in);
>-	if (smb_fname == NULL) {
>-		return -1;
>-	}
>+
> 	smb_fname->base_name = conv;
> 
> 	ret = SMB_VFS_NEXT_OPENAT(handle,
>-- 
>2.49.0
>
>
>From 8954b417d29fec8b186b6b367ec5f79233e76b69 Mon Sep 17 00:00:00 2001
>From: Anoop C S <anoopcs@samba.org>
>Date: Tue, 4 Mar 2025 16:15:05 +0530
>Subject: [PATCH 2/2] vfs_ceph_snapshots: Always calculate absolute snapshot
> path
>
>Use the same logic from shadow_copy2 module to always prepend the
>connectpath to the relative snapshot path so as to return converted
>path corresponding to the file's share root.
>
>Please note that with the current working directory staying at the
>connectpath level we are safe to prefix it to the smb_filename. In
>other words it seems we never get past the connectpath internally
>during normal file system operations via chdir(). Since all relative
>paths are now based on dirfsp we could constitute absolute path by
>prepending the connectpath to full_path_from_dirfsp_atname() output
>ignoring the current working directory.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15819
>
>Signed-off-by: Anoop C S <anoopcs@samba.org>
>Reviewed-by: David Disseldorp <ddiss@samba.org>
>
>Autobuild-User(master): Anoop C S <anoopcs@samba.org>
>Autobuild-Date(master): Wed Apr 30 11:32:59 UTC 2025 on atb-devel-224
>
>(cherry picked from commit 95a2b50b1983a6ba810a96f50b27db7c992c02c0)
>---
> source3/modules/vfs_ceph_snapshots.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/source3/modules/vfs_ceph_snapshots.c b/source3/modules/vfs_ceph_snapshots.c
>index 7400bef008e..f5ba0752614 100644
>--- a/source3/modules/vfs_ceph_snapshots.c
>+++ b/source3/modules/vfs_ceph_snapshots.c
>@@ -531,10 +531,13 @@ static int ceph_snap_gmt_convert_dir(struct vfs_handle_struct *handle,
> 	 * Temporally use the caller's return buffer for this.
> 	 */
> 	if (strlen(name) == 0) {
>-		ret = strlcpy(_converted_buf, snapdir, buflen);
>+		ret = snprintf(_converted_buf, buflen, "%s/%s",
>+			       handle->conn->connectpath, snapdir);
> 	} else {
>-		ret = snprintf(_converted_buf, buflen, "%s/%s", name, snapdir);
>+		ret = snprintf(_converted_buf, buflen, "%s/%s/%s",
>+			       handle->conn->connectpath, name, snapdir);
> 	}
>+
> 	if (ret >= buflen) {
> 		ret = -EINVAL;
> 		goto err_out;
>-- 
>2.49.0
>

Flags:

ddiss: review+

Actions: View

Attachments on bug 15819: 18640 | 18641