Bug 14232 - vfs_ceph.renameat_fn ignores srcfsp and dstfsp
Summary: vfs_ceph.renameat_fn ignores srcfsp and dstfsp
Status: RESOLVED INVALID
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: David Disseldorp
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-09 12:23 UTC by David Disseldorp
Modified: 2020-01-09 17:40 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2020-01-09 12:23:05 UTC
ab74d8d5bd4636405731c9d2aa2d32880844b349 added a renameat_fn hook to vfs_ceph, but  the implementation completely ignores srcfsp and dstfsp. As a result, relative smb_fname_src/smb_fname_dest paths will be processed as relative to cwd, instead of the corresponding fsp.

I'm in the process of rewriting vfs_ceph to use the cephfs low-level (ceph_ll_X) API for bug 14053, and plan on addressing this bug with the same patchset.
Comment 1 Jeremy Allison 2020-01-09 16:56:37 UTC
Yep, ignoring srcfsp and dstfsp is by design at present.

The smb_fname_src/smb_fname_dest paths *should* be processed as relative to cwd rather than the corresponding fsp until the rest of the infrastructure is in place in smbd to correctly modify the paths to be fsp-relative.

This wasn't a bug, it's part of a staged approach for this.

The only fix required is to make sure we do:

SMB_ASSERT(srcfsp == srcfsp->conn->cwd_fsp)
SMB_ASSERT(dstfsp == dstfsp->conn->cwd_fsp)

in the calls currently modified to use the AT() signature in ceph.

We'll get to these as the rest of the code is fixed.
Comment 2 David Disseldorp 2020-01-09 17:13:04 UTC
(In reply to Jeremy Allison from comment #1)
> Yep, ignoring srcfsp and dstfsp is by design at present.
> 
> The smb_fname_src/smb_fname_dest paths *should* be processed as relative to
> cwd rather than the corresponding fsp until the rest of the infrastructure
> is in place in smbd to correctly modify the paths to be fsp-relative.
> 
> This wasn't a bug, it's part of a staged approach for this.
> 
> The only fix required is to make sure we do:
> 
> SMB_ASSERT(srcfsp == srcfsp->conn->cwd_fsp)
> SMB_ASSERT(dstfsp == dstfsp->conn->cwd_fsp)
> 
> in the calls currently modified to use the AT() signature in ceph.
> 
> We'll get to these as the rest of the code is fixed.

Got it, thanks Jeremy. I'll submit a change to add the asserts.
Comment 3 David Disseldorp 2020-01-09 17:40:30 UTC
(In reply to David Disseldorp from comment #2)
...
> Got it, thanks Jeremy. I'll submit a change to add the asserts.

Submitted via https://gitlab.com/samba-team/samba/merge_requests/1043 .

FWIW, my WIP vfs_ceph rewrite tracks CephFS inode information alongside fsp/DIR for use as a "parent" parameter in cephwrap_*at()->ceph_ll_X() calls. I plan on removing all of the cwd assumptions once I have it cleaned up and working.