ceph: fix ceph_mdsc_build_path to not stop on first component
diff mbox series

Message ID 20190509140147.20755-1-jlayton@kernel.org
State New
Headers show
Series
  • ceph: fix ceph_mdsc_build_path to not stop on first component
Related show

Commit Message

Jeff Layton May 9, 2019, 2:01 p.m. UTC
When ceph_mdsc_build_path is handed a positive dentry, it will return a
zero-length path string with the base set to that dentry. This is not
what we want. Always include at least one path component in the string.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/mds_client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

I found this while testing my asynchronous unlink patches. We have
to do the unlink in this case without the parent being locked, and
that showed that we got a bogus path built in this case.

This seems to work correctly, but it's a little unclear whether the
existing behavior is desirable in some cases. Is this the right thing
to do here?

Comments

Yan, Zheng May 13, 2019, 1:10 p.m. UTC | #1
On 5/9/19 10:01 PM, Jeff Layton wrote:
> When ceph_mdsc_build_path is handed a positive dentry, it will return a
> zero-length path string with the base set to that dentry. This is not
> what we want. Always include at least one path component in the string.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>   fs/ceph/mds_client.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> I found this while testing my asynchronous unlink patches. We have
> to do the unlink in this case without the parent being locked, and
> that showed that we got a bogus path built in this case.
> 
> This seems to work correctly, but it's a little unclear whether the
> existing behavior is desirable in some cases. Is this the right thing
> to do here?
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 66eae336a68a..39f5bd2eafda 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -2114,9 +2114,10 @@ char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase,
>   		if (inode && ceph_snap(inode) == CEPH_SNAPDIR) {
>   			dout("build_path path+%d: %p SNAPDIR\n",
>   			     pos, temp);
> -		} else if (stop_on_nosnap && inode &&
> +		} else if (stop_on_nosnap && inode && dentry != temp &&
>   			   ceph_snap(inode) == CEPH_NOSNAP) {
>   			spin_unlock(&temp->d_lock);
> +			pos++; /* get rid of any prepended '/' */
>   			break;
>   		} else {
>   			pos -= temp->d_name.len;
> 

Reviewed-by: "Yan, Zheng" <zyan@redhat.com>

Patch
diff mbox series

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 66eae336a68a..39f5bd2eafda 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2114,9 +2114,10 @@  char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase,
 		if (inode && ceph_snap(inode) == CEPH_SNAPDIR) {
 			dout("build_path path+%d: %p SNAPDIR\n",
 			     pos, temp);
-		} else if (stop_on_nosnap && inode &&
+		} else if (stop_on_nosnap && inode && dentry != temp &&
 			   ceph_snap(inode) == CEPH_NOSNAP) {
 			spin_unlock(&temp->d_lock);
+			pos++; /* get rid of any prepended '/' */
 			break;
 		} else {
 			pos -= temp->d_name.len;