I need to access a share where I have restricted access at the root level, but r/w access to a subdirectory of the root. mount.cifs and mount.smbfs recently lost the ability to access such shares (I was able to do so with the versions installed on Ubuntu 11.04 and am unable to do so with with the versions in Ubuntu 12.04). smbclient, by contrast, is able to mount these shares. A more full account of this issue can be found in the Samba mailing list beginning on 5/16/2012. Network traffic captures were provided in off-list email exchanges with Steve French, Shirish Pargaonkar, and Gunter Kukkukk. Jeff Layton described the problem as follows: Known problem since the superblock sharing patches went in. cifs.ko needs to establish a dentry and inode for the root of the share and then walks down to the "prefixpath" for the mount. Unfortunately if you don't have access to any point along that path, the mount will fail. There have been a couple of proposals to fix it, but they've had their own problems. What probably needs to happen is to do something like what NFS does in its superblock sharing model. Allow several trees of dentries within a superblock and only connect them later if we happen to stumble across the right entry. See commit 54ceac45159 for an explanation of the model NFS uses for this. -- Jeff Layton <jlayton@samba.org> Opening this bug report at the suggestion of Steve French so that it does not get overlooked.
Reference: http://thread.gmane.org/gmane.network.samba.general/124822/focus=124872
This is still biting me in Linux 3.4.9, with exactly the same problem: * I don't have access to the share root * I do have access to a lower directory. * smbclient can access the lower levels fine, but I can't mount them with mount.cifs. * Getting the server administrator to fix this will be a long, tedious, process because it works just fine for all the Windows clients, and the administrators don't touch anything else.
I found at least some degree of work-around (although the performance is pretty poor): usmb can connect to and mount these shares through FUSE: http://ametros.net/code.html#usmb all the other FUSE/libsmbclient tools I found insisted upon doing the same directory walk (for different reasons) as the cifs kernel module, and thus failing in the same way.
This sounds like a regression to me, isn't? May be we rushed through the current shared superblock patches..?
I do not see this happening with 2.0 version of cifs vfs client. For example, I have a Windows 2003 server which a share exported that is owned by Administrator with a security descriptor with only Administrator access. As an another user (not Administrator), I can authenticate and mount a directory, which is owned by this user, under this share. There is no access denied error while browsing the mounted directory.
I really do not see the problem. I would like to work on the fix but having hard time recreating the problem. Here are the steps I do and do not see the problem in accessing files/directories that an authenticated user has permissions even if he/she does not have access permissions on prior directories. I am using sudo modinfo cifs filename: /lib/modules/3.10.0+/kernel/fs/cifs/cifs.ko version: 2.01 and a Windows 8 Pro server with sharename public2 and a directory underneath named sspdir. --------------- I think this is somewhat consistent with what has been repoted. $ sudo smbclient //192.168.1.67/public2 -U ssp%<password> Domain=[SSPWIN8] OS=[Windows 8 Pro with Media Center 9200] Server=[Windows 8 Pro with Media Center 6.2] smb: \> ls NT_STATUS_ACCESS_DENIED listing \* smb: \> ls sspdir NT_STATUS_ACCESS_DENIED listing \sspdir smb: \> cd sspdir smb: \sspdir\> ls . D 0 Sun Sep 1 09:36:14 2013 .. D 0 Sun Sep 1 09:36:14 2013 sspfile1 A 25 Sun Sep 1 09:36:14 2013 57624 blocks of size 8388608. 51639 blocks available smb: \sspdir\> --------------- But I do not see the same problem with cifs client (with vers=3.0 and vers=1.0) $ sudo mount -t cifs //192.168.1.67/public2 /mnt/smb_a -o sec=ntlmsspi,vers=3.0,user=ssp,pass=<password> $ ls /mnt/smb_a ls: reading directory /mnt/smb_a: Permission denied $ ls /mnt/smb_a/sspdir sspfile1 $ cd /mnt/smb_a/sspdir $ ls sspfile1 -------------------- I can also do this: $ sudo mount -t cifs //192.168.1.67/public2/sspdir /mnt/smb_a -o sec=ntlmsspi,vers=3.0,user=ssp,pass=<password> $ ls /mnt/smb_a sspfile1 -------------- These are the permissions at the server: $ smbcacls //192.168.1.67/public2 sspdir -U ssp%<password> REVISION:1 CONTROL:0x9404 OWNER:sspwin8\ssp GROUP:sspwin8\None ACL:sspwin8\ssp:ALLOWED/OI|CI/FULL I can not list (smb)cacls of the share, public2, but if I could, they are something like this: REVISION:1 CONTROL:0x9404 OWNER:sspwin8\Shirish GROUP:sspwin8\None ACL:nt authority\system:ALLOWED/OI|CI/FULL ACL:builtin\Administrators:ALLOWED/OI|CI/FULL ACL:sspwin8\Shirish:ALLOWED/OI|CI/FULL
(In reply to comment #6) > I really do not see the problem. I would like to work on the fix > but having hard time recreating the problem. [...] > > $ sudo mount -t cifs //192.168.1.67/public2 /mnt/smb_a -o > sec=ntlmsspi,vers=3.0,user=ssp,pass=<password> > > $ ls /mnt/smb_a > ls: reading directory /mnt/smb_a: Permission denied > > $ ls /mnt/smb_a/sspdir > sspfile1 > Assuming that you have the same problem with SMB1, if you're able to mount the root of the share, then you're almost certainly able to issue a QUERY_PATH_INFO against the root as well (a network capture would confirm this), though you may not be able to do a FIND_FIRST/NEXT in that directory. The problem in this bug crops up when you're not allowed to even do a QPathInfo against an intermediate directory between the root of the share and the directory within that share that you want to serve as the root of the mount. To reproduce this, you may need to tweak the ACLs a bit more, or perhaps have another level of directory in there that has very restrictive permissions.
ok, I tweaked this acl REVISION:1 CONTROL:0x9404 OWNER:sspwin8\Shirish GROUP:sspwin8\None ACL:nt authority\system:ALLOWED/OI|CI/FULL ACL:builtin\Administrators:ALLOWED/OI|CI/FULL ACL:sspwin8\Shirish:ALLOWED/OI|CI/FULL to REVISION:1 CONTROL:0x9404 OWNER:sspwin8\Shirish GROUP:sspwin8\None ACL:sspwin8\Shirish:ALLOWED/OI|CI/FULL and now user ssp can't mount the share (wither public2 or public2/sspdir) since it is denied access to the root of the share itself when doing create request file. That is what happens to a Windows 7 client when it tries to mount and access the share or a subdirectory under the share. smbclient can mount the share and can cd to the sspdir but that does not mean much since as it tries to access files under sspdir, it is denied access. So none of the clients, Windows 7, smbclient, and cifs vfs, as authenticated user ssp can go past the (root of the) share over to a directory which user ssp owns/has access to. This behaviour is against the Windows 8 Pro server. So if this is how a Windows server is responding to all kinds of client, I am still not sure what a cifs client is expected to do that a server does not permit.
I think I need to re-read and understand these lines that Jeff added in prior comments. "The problem in this bug crops up when you're not allowed to even do a QPathInfo against an intermediate directory between the root of the share and the directory within that share that you want to serve as the root of the mount." Will try to recreate this instead of what I was doing (denying a user access to the root of the share instead of denying access to an intermediate directory).
My use case (doesn't work with cifs): We have networked home directories (not profiles), which are on a shared server (call it server) and inside a share (call it share$). So, on Windows, H: -> \\server\share$\username We are not permitted to list the contents of the share$, but are free to read/write/delete inside username. This works with smbclient (smbclient -U username '\\server\share$', cd username, ls) and on Windows (obviously), but not with CIFS.
OK, I finally have the set up right, with correct ACLs aligned so that I can see the problem using cifs client but not with Windows client and smbclient. On to understanding various proposed solutions and implementing one.
"1) if the dentries to get down to the root of the mount don't already exist, then attach some sort of "placeholder" inode that can be fleshed out later if and when the dentry is accessed via other means." Once a dentry for intermediate inaccessible path(s) is/are created with "placeholder" inode, that dentry may/will need to be revalidated. For a shared superblock, not sure how a dentry with "placeholder" inode can be accessed to flesh out the inode. I think the challenge here is to create a "placeholder" inode (in cifs_get_root()) and once that is created to sometimes validate that dentry/inode.
I think it is possible to handle lookup/revalidate errors/failures on inaccessible intermediate paths but I am not sure how to handle revalidates on these intermediate paths in other places. For example, we could have paths b and d inaccessible. So we can hanele mount of /a/b/c/d/e on mountpoint /mnt/m and mount of /a/b/c on mount point /mnt/n. But a stat of /mnt/n/d could end up purging dentry for d and then we have disconnected paths to e.
Addng the same note with some corrections.. ---- I think it is possible to handle lookup/revalidate errors/failures on inaccessible intermediate paths in cifs_get_root() but I am not sure how to handle revalidates on these intermediate paths in other places. For example, we could have paths b and d inaccessible. So we can handle mount of /a/b/c/d/e on mountpoint /mnt/m and mount of /a/b/c on mount point /mnt/n. But a stat of /mnt/n/d could end up purging dentry for d and then we have disconnected paths to e.
Created attachment 9369 [details] create placeholder inodes So I came up with something like this and it works. But I think basic problem/idea is: -- Once we create these dentries with "placeholder" inodes, we want them to remain in the dcache. Thus either the "placeholder" inodes for these dentries should not be revalidated and if they should be revalidatable, if they do not get validated (as they would not) and dentry turns negative, it should somehow remain in dcache. -- So perhaps either unset the bit DCACHE_OP_REVALIDATE of d_flags of the dentries with placeholder inode or set their d_op to NULL.
Created attachment 9370 [details] create placeholder inodes for intermediate dentries oops, wrong attachment in the previous note.... So I came up with something like this and it works. But I think basic problem/idea is: -- Once we create these dentries with "placeholder" inodes, we want them to remain in the dcache. Thus either the "placeholder" inodes for these dentries should not be revalidated and if they should be revalidatable, if they do not get validated (as they would not) and dentry turns negative, it should somehow remain in dcache. -- So perhaps either unset the bit DCACHE_OP_REVALIDATE of d_flags of the dentries with placeholder inode or set their d_op to NULL.
Thanks Jeff. As pointed out on the linux-fsdevel mailing list that should not manipulate DCACHE_OP_REVALIDATE since it does not apply here. So I think one solution is to have for the intermediate paths, dentry d_ops with a d_revalidate method such that if a dentry can't be validated, keep it instantiated with placeholder inode and if it can be, with the real inode (info).
Created attachment 9414 [details] allow to mount a share with intermediate inaccessible paths I was able to make mounts work with this patch. If anybody would care to test, it would be nice. Basically, I tested thus: mount a share /<sharename>/a/b on /mnt/m as user x on the client where user x can't access a on the server mount a share /<sharename> on /mnt/n as user x this sharing superblock. stat /mnt/n/a without permission on a stat /mnt/n/a after changing / with permissions on a stat /mnt/n/a without permission on a
I did further testing like this: For the same authenticated user: mount /<share>/a/b on /mnt/smb_m mount /<share> on /mnt/smb_n stat /mnt/smb_n/a - which can't validate but d_count of dentry for a is > 0 sleep 5 mount /<share>/a/c on /mnt/smb_o stat /mnt/smb_n/a umount /mnt/smb_m stat /mnt/smb_n/a sleep 5 mount /<share>/a/b on /mnt/smb_m ls -l /mnt/smb_o ls -l /mnt/smb_m So d_count greater than one keep the dentry even if can't be invalidated. I think I feel comfortable with this patch enough so will stop further testing and post this patch on the mailing list sometimes today or so and await for any comments.
I had to DENY Read Attribute in Special Permissions for a directory for an user to make smb2 create request with access mask 0x80 to fail. The set up I can use to generate EACCES, finally.
Need to mimic what nfs does in such cases. That is the only solution, that is what it looks like. Spent quite some time over the break, have a faint idea now how to implement it but not fully. Will spend time on this next week and thereafter along with rest of the work. Basically trying to figure out how to create multiple vfsmount and root dentry pairs (sub-trees) within a shared superblock and how a sub-tree can become/materialise/splice (after being identified with a certainty) as a branch in another (sub)tree when otherwise inacessbile parent of a sub-tree becomes accessible. Not sure whether we need to create a false root for root dentry of the superblock (what would be the inode for this fake dentry?) Something like that...
Still doesn't work with patch: # cd /tmp # mkdir dir # mount -t cifs -o user=[snip],domain=[snip] //[snip]/[snip]\ [snip]$/[snip] dir Password for [snip]@//[snip]/[snip] [snip]$/[snip] mount error(13): Permission denied Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) # dmesg | tail -n 5 address conversion returned 1 for [snip] Max buf = 16644 CIFS VFS: cifs_read_super: get root inode failed SendRcvNoRsp flags 64 rc 0 SendRcvNoRsp flags 64 rc 0
(In reply to comment #22) > Still doesn't work with patch: No it would not, the patch is incorrect. About to resume work on this in a day or two.
I think I understand creating several trees within a superblock. Working on figuring out how to connect/splice them together.
I think it is basically down to calling d_obtain_alias() in cifs_get_root() and calling d_materialise_unique() in cifs_lookup(). I do not think d_splice_alias() would be useful for this problem. Following the discussion on mailing lists related to these function calls.
Created attachment 9727 [details] allow to mount a share with intermediate inaccessible paths using d_obtain and d_splice alias Second attempt with this patch posted on the mailing list. Waiting for the feedback.
Somehow, for a (root) disconnected dentry obtained by d_obtain_alias, inspite of having DCACHE_OP_DELETE (d_flag value is 0x100002c and lockref.count is 1), d_delete method does not get invoked during unmount (as if dput does not get invoked). For noserverino case, I rely on d_delete method of such disconnected entries to clean-up list elements maintained in cifs_sb_info structure. Will resume debugging tomorrow, hopefully this is the last hurdle to overcome to make superblock sharing code to work for all the cases.
Still doesn't work with patch "try #2".
P.S. see comment 22. if cifsFYI etc is needed, feel free to ask.
(In reply to comment #29) > P.S. see comment 22. > > if cifsFYI etc is needed, feel free to ask. Hi, Thanks for testing. Although the patch needs more work and working on that, the basic mount should have worked. I did try against a Windows 8.1 server and it did mount shares for smb1 and 2 and 3. Would you be able to attach a wireshark trace and the command you attempt to mount the share?
(In reply to comment #30) > (In reply to comment #29) > > P.S. see comment 22. > > > > if cifsFYI etc is needed, feel free to ask. > > Hi, Thanks for testing. Although the patch needs more work and working > on that, the basic mount should have worked. I did try against a Windows > 8.1 server and it did mount shares for smb1 and 2 and 3. > > Would you be able to attach a wireshark trace and the command you attempt > to mount the share? The latter is available in comment 22 ([snip]ped of internal information). The former will have to wait until Monday, as I do not have access to the server at the moment. I do know that the server is likely Windows Server 2000 or Server 2003, but may be 2008 or R2. I tried both with and without vers=1.0, but I don't know what the server actually supports, so that may or may not have actually made a difference.
(In reply to comment #27) > Somehow, for a (root) disconnected dentry obtained by d_obtain_alias, > inspite of having DCACHE_OP_DELETE (d_flag value is 0x100002c and lockref.count > is 1), d_delete method does not get invoked during unmount (as if dput does not > get invoked). > Not a correct observation. does not matter if the root dentry is a regular or anonymous. Need to figure out a way to clean up entries added to cifs_sb_info during d_obtain_alias for anonymous root dentries for the case of noserverino kind of servers if d_splice_alias does not happen before umount so the respective/added entries to cifs_sb_info are cleaned up.
What I need to figure out is how does a filesystem (in this case, cifs) know when one of its mounts is being unmounted!
d_release of a dentry works. During umount, d_delete may not always be called on the root dentries because one may have (child) dentries referring to it (including negative dentries). d_release will always be called, so the bookkeeping will happen while getting rid of hte superblock. The entries will always be added to begining of the list in cifs_sb_info, so that will take care of finding the correct inode for the matched path for a disconnected/anonymous root dentry (mount). Coding all of this and testing. And (hopefully) waiting for the wireshark trace to debug the reported failure.
(In reply to comment #34) > d_release of a dentry works. > > During umount, d_delete may not always be called on the root dentries > because one may have (child) dentries referring to it (including negative > dentries). > > d_release will always be called, so the bookkeeping will happen while > getting rid of hte superblock. > > The entries will always be added to begining of the list in cifs_sb_info, > so that will take care of finding the correct inode for the matched > path for a disconnected/anonymous root dentry (mount). > > Coding all of this and testing. > > And (hopefully) waiting for the wireshark trace to debug the > reported failure. I lied, I'm actually on break this week. I'll get it next week; ping me via email around 9:00 EDT Monday if you want to remind me.
Created attachment 9787 [details] 408-thil-lss-noauth.pcap pcap file attached. I have filtered by smb and not smb.security_blob to prevent offline cracking of my password.
(In reply to comment #36) > Created attachment 9787 [details] > 408-thil-lss-noauth.pcap > > pcap file attached. I have filtered by smb and not smb.security_blob to prevent > offline cracking of my password. Looks like user has access to directory/path 0727417143 under the share (name) 408 students$ but has no/restricted access/permissions on the share itself. Can this user mount just the share i.e. instead of 408 students$/0727417143, mount just 408 students$?
Looked at the wireshark trace again. The access denied is returned while querrying the share (directory/folder) on the server during the process to create root dentry for the superblock. I am not sure yet how to have such a setup where a share allows an authenticated user to tree connect but returns access denied to query the share (directory/folder). But although similar, this is a different problem than the one where a user does not have access to an intermediate path to a directory/folder being mounted. So the patch I have posted will not work for this case. I am not sure how to handle a case where we can't obtain (by querying) information to create an inode for the root dentry for a superblock itself.
Asked this question on MSDN. Hope someone responds... If there is a way to do this on a Samba server, that would be useful too. " I have a share (for a folder named public) where Authenticated User (group) has Sharing Permission as Read and Security Permissions as Allow Traverse Folder / Execute File. There is a folder (named user1folder) under the folder public, to which an user name user1 has Full Access. So I expect user1 as an autheticated user over network trying to mount a folder user1folder to do a successful query path info but acces denied to query path info on share/folder public. But that does not happen. Query path info on share/folder public also succeeds. I also tried Everyone (group) and user1 instead of Authenticated User (group) but no luck. Basically, on this Windows 7 SMB server, I would like to have an authenticated user, denied access if it attempts to mount the shared folder but be able to access a folder under this shared folder to which this authenticated user has full access. "
How can query path info on shared folder C:\public2 succeed is really puzzling inspite of these Sharing and Security permissions. C:\>net share public2 Share name public2 Path C:\public2 Remark Maximum users No limit Users Caching Manual caching of documents Permission server-PC\owner1, FULL server-PC\user1, READ The command completed successfully. C:\>cacls public2 C:\public2 server-PC\user1:(DENY)(special access:) DELETE READ_CONTROL WRITE_DAC WRITE_OWNER STANDARD_RIGHTS_REQUIRED FILE_READ_DATA FILE_WRITE_DATA FILE_APPEND_DATA FILE_READ_EA FILE_WRITE_EA FILE_DELETE_CHILD FILE_READ_ATTRIBUTES FILE_WRITE_ATTRIBUTES server-PC\user1:(special access:) SYNCHRONIZE FILE_EXECUTE server-PC\owner1:(OI)(CI)F C:\>icacls public2 public2 server-PC\user1:(DENY)(R,W,D,WDAC,WO,DC) server-PC\user1:(S,X) server-PC\owner1:(OI)(CI)(F)
I think I have the setup. I had to change permissions (deny ReadData) on the parent directory of the leaf of the share path so that query path info fails. Looking at events in the log (event 5145 on Windows 2012 server) was helpful. Will work on a meaningful patch and try to post soon.
This is the setup that works, thought will just log it here for reference... PS C:\Users\Administrator> net share public3$ Share name public3$ Path C:\Shares\public3$ Remark Maximum users No limit Users Caching Manual caching of documents Permission Everyone, READ The command completed successfully. PS C:\> net user user1 User name user1 Full Name user1 Comment User's comment Country/region code 000 (System Default) Account active Yes Account expires Never Password last set 4/26/2014 11:01:45 PM Password expires Never Password changeable 4/26/2014 11:01:45 PM Password required Yes User may change password Yes Workstations allowed All Logon script User profile Home directory Last logon 5/12/2014 6:53:16 PM Logon hours allowed All Local Group Memberships *Users Global Group memberships *None The command completed successfully. PS C:\Users\Administrator> cacls c:\shares c:\Shares BUILTIN\Users:(DENY)(special access:) FILE_READ_DATA NT AUTHORITY\SYSTEM:(OI)(CI)(ID)F BUILTIN\Administrators:(OI)(CI)(ID)F BUILTIN\Users:(OI)(CI)(ID)R BUILTIN\Users:(CI)(ID)(special access FILE_APPEND_DAT BUILTIN\Users:(CI)(ID)(special access FILE_WRITE_DATA CREATOR OWNER:(OI)(CI)(IO)(ID)F PS C:\> cacls c:\shares\public3$ c:\shares\public3$ WIN-VA77GADM8UM\user1:(DENY)(special access:) DELETE READ_CONTROL WRITE_DAC WRITE_OWNER STANDARD_RIGHTS_REQUIRED FILE_READ_DATA FILE_WRITE_DATA FILE_APPEND_DATA FILE_READ_EA FILE_WRITE_EA FILE_DELETE_CHILD FILE_READ_ATTRIBUTES FILE_WRITE_ATTRIBUTES WIN-VA77GADM8UM\Administrator:(OI)(CI)F BUILTIN\Administrators:(OI)(CI)F WIN-VA77GADM8UM\user1:(special access:) SYNCHRONIZE FILE_EXECUTE PS C:\> cacls c:\shares\public3$\user1dir3 c:\shares\public3$\user1dir3 WIN-VA77GADM8UM\user1:F WIN-VA77GADM8UM\Administrator:F BUILTIN\Administrators:R
Created attachment 9976 [details] shared superblock mount changes for mounts without access to intermediate path So this should work for a mount where a SMB server furbishes inode numbers. Now working on the remaining patch where a SMB server does not provide inode numbers. Still some issues to hash out but hopefully getting there...
If anybody wants, the five patches are available at https://github.com/shirishpargaonkar/sspcifs/commits/for-next-sb1 Would be nice to receive any feedback.
(In reply to comment #44) > If anybody wants, the five patches are available at > https://github.com/shirishpargaonkar/sspcifs/commits/for-next-sb1 Should this also fix the situation, where the user is denied access on the share itself, or just the other situation where user is denied access on some intermediate directories? I ran into this problem today on CentOS 7 - the situation where both the share and all directories above the last one were denying access to the user. Interestingly, such setup works not only with Windows clients, but OS X 10.9 as well.
(In reply to comment #45) > (In reply to comment #44) > > If anybody wants, the five patches are available at > > https://github.com/shirishpargaonkar/sspcifs/commits/for-next-sb1 > > Should this also fix the situation, where the user is denied access on the > share itself, or just the other situation where user is denied access on some > intermediate directories? > > I ran into this problem today on CentOS 7 - the situation where both the share > and all directories above the last one were denying access to the user. > Interestingly, such setup works not only with Windows clients, but OS X 10.9 as > well. It should work for both the cases. The first patch takes care of when user is denied access on the share itself and the rest four, when user is denied access on the intermediate paths.
Hm, the patch won't apply to sources for CentOS 3.10 kernel - problem in cifs_fs_sb.h: struct cifs_sb_info is missing rcu_head. What do you suggest - modify the patch, or apply to newer (vanilla?) kernel?
no, you can modify the patch, just the struct rcu_head rcu; does not exist in 3.10. Rest of the patches should apply.
(In reply to Michal Zatloukal from comment #47) I've rebased Shirish's five comment#44 patches atop current mainline, and v3.10.10 (if needed for CentOS). The bso8950_mount_restricted_root_subdir[_v3.10.10] branches can be found in my repo at git://git.samba.org/ddiss/linux.git: - https://git.samba.org/?p=ddiss/linux.git;a=shortlog;h=refs/heads/bso8950_mount_restricted_root_subdir - https://git.samba.org/?p=ddiss/linux.git;a=shortlog;h=refs/heads/bso8950_mount_restricted_root_subdir @Michal and Alex: It'd be great if you could both test the changes in your local environment and report back.
@Shirish, I've been taking a look at your comment#44 patch-set, and have a few minor (mostly cosmetic) comments so far: 708 static struct cifs_rdelem * 709 find_rdelem_by_inode(struct inode *rdinode, struct cifs_sb_info * cifs_sb) The non-static->static change of these functions should be collapsed into the initial "various routines to search disconnected root dentry" commit. Also, drop the spaces in "* cifs_sb". 743 static void 744 find_rdelem_by_path(char *full_path, struct inode **newInode, full_path should probably be const here. 745 struct cifs_sb_info * cifs_sb) 746 { 747 struct cifs_rdelem *rdelem; 748 749 spin_lock(&cifs_sb->rtdislock); 750 list_for_each_entry(rdelem, &cifs_sb->rtdislist, rdlist) { 751 if (!strcmp(rdelem->rdname, full_path)) { 752 *newInode = ilookup(rdelem->rdinode->i_sb, 753 rdelem->rdinode->i_ino); 754 break; The other find_rdelem_* functions remove the rtdislist entry when the item is found, however this function retains it. I find this a little confusing/inconsistent. 888 cifs_d_common_releasedelete(const struct dentry *dentry) 889 { ... 895 /* disconnected root dentries that did not get spliced */ 896 if (IS_ROOT(dentry) && dentry->d_flags & DCACHE_DISCONNECTED) { Extra parentheses would be nice here, e.g. if (IS_ROOT(dentry) && (dentry->d_flags & DCACHE_DISCONNECTED)) { 618 create_root_dis_dentry(struct super_block *sb, struct inode *rinode, 619 char *fpath) fpath should probably be const here too. 620 { ... 642 rdelem = 643 cifs_alloc_rdelem(fpath, dentry, rinode); No need to split this across lines. 661 static struct dentry * 662 cifs_get_root(struct smb_vol *vol, struct super_block *sb) ... 708 child = lookup_one_len(p, dentry, s - p); 709 mutex_unlock(&dir->i_mutex); 710 dput(dentry); 711 dentry = child; 712 } while (!IS_ERR(dentry)); 713 714 if (IS_ERR(dentry) && (PTR_ERR(dentry) == -EACCES) && *s) 715 dentry = create_root_dis_dentry(sb, rinode, full_path); lookup_one_len() can return -EACCES if a path component corresponds to "." or ".." - I expect a disconnected root dentry shouldn't be created in such a case? 760 struct dentry * 761 cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry, 762 unsigned int flags) ... 820 if (pTcon->unix_ext) { 821 rc = cifs_get_inode_info_unix(&newInode, full_path, 822 parent_dir_inode->i_sb, xid); 823 } else 824 rc = cifs_get_inode_info(&newInode, full_path, NULL, 825 parent_dir_inode->i_sb, xid, NULL); The use of braces here is inconsistent.
(In reply to David Disseldorp from comment #50) David, Thanks fore the review and comments. Regarding find_rdelem_by_path() function, it is just a search function unlike the two functions find_rdelem_by_inode() and find_rdelem_by_dentry() which are like search and purge functions to search and purge spliced and unspliced rdelem entries respectively. At the time of calling find_rdelem_by_path(), we have not spliced the dentry and two lookups could be racing in cifs_lookup() function. So, perhaps a function rename could be more explanatory: find_rdelem_by_path() to search_unspliced_rdelem_by_path(), find_rdelem_by_dentry() to purge_unspliced_rdelem_by_dentry() find_rdelem_by_inode() to purge_spliced_rdelem_by_inode(). Regarding . and .., I think this applies to the full path such as /a/b/c/d etc. during the mount. If someone were to specify a path such as /a/b/./c or /a/b/../b/c, I think . or .. would not matter since we would have encountered -EACCES for either current or parent directory respectively, already. Do these explanations make sense? Rest of the comments are valid and will fix them. But at this point, I do not have a Windows 2012 server handy to recreate the setup (I could never recreate this using a Samba server), am working on securing one though. I hope one or more of the subscribers to this bug is/are able to test the patchset.
still doesn't work with either the attached patch or those from https://git.samba.org/?p=ddiss/linux.git;a=shortlog;h=refs/heads/bso8950_mount_restricted_root_subdir tested now against Windows Server 2012 R2 (as reported by server). now I am using sec=krb5, but that shouldn't make a difference in this part.
never mind, now access without root works. but I have a similar problem now. I want to use sec=krb5,multiuser when the mounting creds have no access anywhere in the tree, but the real users do.
Hi Alex, Thanks for testing. Let me gather my thoughts on this, it has been a while, will respond shortly...
(In reply to alex_y_xu from comment #53) Indeed, thanks for the test feedback Alex. Which kernel did you use? @shirish: your proposed rename sounds good. Did you get around to making the other changes?
(In reply to David Disseldorp from comment #56) > (In reply to alex_y_xu from comment #53) > Indeed, thanks for the test feedback Alex. Which kernel did you use? can't be bothered to fish for the date in my reflog, but it was linux.git master at the time (around 4.0.0)
(In reply to David Disseldorp from comment #56) David, will work on making those changes. Sorry for really late reply, was busy at work deadline and then on vacation for two weeks.
Created attachment 11762 [details] make shares unaccessible at root level mountable This patch should fix it. See relevant thread on the linux-cifs mailing list [1]. My only concern is when a connexion is shared between mount points. I need to go back to it and test some more... 1: http://thread.gmane.org/gmane.linux.kernel.cifs/11244
Variation of my patch has been accepted upstream. This bug can be closed.