Bug 12911 - vfs_ceph provides inconsistent directory listings
Summary: vfs_ceph provides inconsistent directory listings
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: 4.6.6
Hardware: All All
: P5 major (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-15 00:28 UTC by David Disseldorp
Modified: 2017-07-31 09:28 UTC (History)
3 users (show)

See Also:


Attachments
Proposed fix (1.21 KB, patch)
2017-07-15 00:33 UTC, David Disseldorp
no flags Details
git-am fix for 4.7.0 (1.47 KB, patch)
2017-07-21 23:19 UTC, Jeremy Allison
ddiss: review+
Details
fix for 4.5.next (1.41 KB, patch)
2017-07-24 09:47 UTC, David Disseldorp
jra: review+
Details
fix for 4.6.next (1.40 KB, patch)
2017-07-24 09:48 UTC, David Disseldorp
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2017-07-15 00:28:50 UTC
I have the following Ceph FS (shown from kernel CephFS mount):
client:~/:[0]# ls -Rla /mnt/
/mnt/:
total 10240
drwxrwxrwx 1 ddiss suse        2 Jul 14 16:05 .
drwxr-xr-x 1 root  root      214 Jul 10 20:29 ..
-rwxr--r-- 1 ddiss suse 10485760 Jul 13 16:01 smb_upload
drwxr-xr-x 1 root  root        1 Jul 14 16:12 stuff

/mnt/stuff:
total 0
drwxr-xr-x 1 root  root 1 Jul 14 16:12 .
drwxrwxrwx 1 ddiss suse 2 Jul 14 16:05 ..
-rw-r--r-- 1 root  root 0 Jul 14 16:12 nested_file


My Samba share of the same filesystem is configured via:
[rootshare]
        path = /
        ceph: config_file = /etc/ceph/ceph.conf
        ceph: user_id = samba.server
        vfs objects = ceph
        read only = no

When using smbclient to dump the directory listing of share I see:
client:~/:[0]# smbclient -U ddiss //smbserver/rootshare
Domain=[CTDB-SERVER] OS=[] Server=[]
smb: \> ls
  .                                   D        0  Fri Jul 14 16:05:26 2017
  ..                                  D        0  Fri Jul 14 16:05:26 2017
  smb_upload                          A 10485760  Thu Jul 13 16:01:37 2017
  stuff                               D        0  Fri Jul 14 16:12:13 2017

                131381297152 blocks of size 1024. 131372830720 blocks available
smb: \> cd stuff\
smb: \stuff\> ls
NT_STATUS_NO_SUCH_FILE listing \stuff\*
smb: \stuff\> ls
NT_STATUS_OBJECT_PATH_NOT_FOUND listing \stuff\*
smb: \stuff\> cd ../
smb: \> ls
  .                                   D        0  Fri Jul 14 16:12:13 2017
  ..                                  D        0  Fri Jul 14 16:05:26 2017
  nested_file                         N        0  Fri Jul 14 16:12:13 2017

                131381297152 blocks of size 1024. 131372830720 blocks available


This broken behaviour is caused by the following logic:

 934 static int cephwrap_chdir(struct vfs_handle_struct *handle,
 935                         const struct smb_filename *smb_fname)
 936 {
 937         int result = -1;
 938         DBG_DEBUG("[CEPH] chdir(%p, %s)\n", handle, smb_fname->base_name);
 939         /*
 940          * If the path is just / use chdir because Ceph is below / and
 941          * cannot deal with changing directory above its mount point
 942          */
 943         if (!strcmp(smb_fname->base_name, "/")) {
 944                 return chdir(smb_fname->base_name);

                            ^^^^^---- This is changing the *local* FS working directory!!

 945         }
 946 
 947         result = ceph_chdir(handle->data, smb_fname->base_name);
 948         DBG_DEBUG("[CEPH] chdir(...) = %d\n", result);
 949         WRAP_RETURN(result);
 950 }

I don't know why the chdir() logic was added in the first place (even with the comment), but the test case above passes once cephwrap_chdir() is converted to a simple ceph_chdir() call.
Comment 1 David Disseldorp 2017-07-15 00:33:22 UTC
Created attachment 13390 [details]
Proposed fix
Comment 2 Jeremy Allison 2017-07-21 23:19:51 UTC
Created attachment 13409 [details]
git-am fix for 4.7.0

Cherry-pick from master to 4.7.0
Comment 3 David Disseldorp 2017-07-24 09:24:35 UTC
Comment on attachment 13409 [details]
git-am fix for 4.7.0

Thanks Jeremy.
I'd like to also get this into the 4.5 and 4.6 maint releases - patches to follow.
Comment 4 David Disseldorp 2017-07-24 09:47:52 UTC
Created attachment 13422 [details]
fix for 4.5.next
Comment 5 David Disseldorp 2017-07-24 09:48:34 UTC
Created attachment 13423 [details]
fix for 4.6.next
Comment 6 Jeremy Allison 2017-07-24 23:34:04 UTC
Re-assigning to Karolin for inclusion in 4.7.0, 4.6.next, 4.5.next.
Comment 7 Karolin Seeger 2017-07-25 09:40:22 UTC
(In reply to Jeremy Allison from comment #6)
Pushed to autobuild-v4-{7,6,5}-test.
Comment 8 Jeff Layton 2017-07-25 15:42:39 UTC
Sorry for late reply here. Patch looks good to me.
Comment 9 Karolin Seeger 2017-07-31 09:28:10 UTC
(In reply to Karolin Seeger from comment #7)
Pushed to all branches.
Closing out bug report.

Thanks!