Message ID | 1473164554-8062-1-git-send-email-sprabhu@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
This is not enough to fix DFS. I believe we also need [1] to handle DFS links to subdirs. I have SERVER_A with a DFS link to //SERVER_B/sub/dir. cifs_are_all_path_accessible() and cifs_get_root() walk along the path components and do queries on them. But they are not skipping the initial host + share name on the UNC resulting in failed queries: CLIENT_A_____ -> SERVER_B____ SMB Trans2 Request, QUERY_PATH_INFO, Query File All Info, Path: //SERVER_B____ SERVER_B____ -> CLIENT_A_____ SMB Trans2 Response, QUERY_PATH_INFO, Error: STATUS_OBJECT_NAME_NOT_FOUND The QUERY_PATH_INFO request here is using the first path component which is not a file path but an incomplete UNC path. That path is made by cifs_build_path_to_root() which only makes a full UNC path when in a DFS. It is not necessary on the servers I've tested (cf [1]). Therefore the fix is to not make a full UNC path in cifs_build_path_to_root() or to add a flag to its parameters to decide whether to prefix the host+share or not. I've sent patches for both solution on the thread [1] (look at all the messages of the thread). 1: http://marc.info/?i=20160801143204.7377b5a4@aaptelpc
On Tue, 2016-09-06 at 20:29 +0200, Aurélien Aptel wrote: > This is not enough to fix DFS. I believe we also need [1] to handle > DFS > links to subdirs. > > I have SERVER_A with a DFS link to //SERVER_B/sub/dir. > cifs_are_all_path_accessible() and cifs_get_root() walk along the > path > components and do queries on them. But they are not skipping the > initial > host + share name on the UNC resulting in failed queries: > > CLIENT_A_____ -> SERVER_B____ SMB Trans2 Request, > QUERY_PATH_INFO, Query File All Info, Path: //SERVER_B____ > SERVER_B____ -> CLIENT_A_____ SMB Trans2 Response, > QUERY_PATH_INFO, Error: STATUS_OBJECT_NAME_NOT_FOUND > > The QUERY_PATH_INFO request here is using the first path component > which > is not a file path but an incomplete UNC path. > > That path is made by cifs_build_path_to_root() which only makes a > full > UNC path when in a DFS. It is not necessary on the servers I've > tested > (cf [1]). > > Therefore the fix is to not make a full UNC path in > cifs_build_path_to_root() or to add a flag to its parameters to > decide > whether to prefix the host+share or not. I've sent patches for both > solution on the thread [1] (look at all the messages of the thread). > > 1: http://marc.info/?i=20160801143204.7377b5a4@aaptelpc > Hello Auriel, I have encountered a similar problem reported by a user a few days ago when mounting a DFS share hosted on a Netapp. The problem is caused when 1) The flag SMB_SHARE_IS_IN_DFS is set in the optional flags when a tcon connect is returned. 2) The subsequent Trans2 request with QUERY_PATH_INFO doesn't return a STATUS_PATH_NOT_COVERED which indicates that this is a DFS share and we need to call a GET_DFS_REFERRAL to obtain the DFS node to connect to. This is hit in Netapp because it returns SMB_SHARE_IS_IN_DFS even after the DFS node is resolved and the node returned doesn't contain a DFS share. Other servers(tested with MS Win2012 R2) do not and hence do not face this problem. A workaround for Netapp servers is to mount with the nodfs flag. I have a patch similar to your second variant which we have successfully tested in our lab and is currently undergoing user testing. I'll post it separately as soon as we have the results for that. In my opinion, the patch at the head of the thread here is not related to this problem and should go in ASAP to fix a regression which affects all DFS mounts. Sachin Prabhu -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sachin, Sachin Prabhu <sprabhu@redhat.com> writes: > This is hit in Netapp because it returns SMB_SHARE_IS_IN_DFS even after > the DFS node is resolved and the node returned doesn't contain a DFS > share. Other servers(tested with MS Win2012 R2) do not and hence do not > face this problem. A workaround for Netapp servers is to mount with the > nodfs flag. The issue I'm describing is happening on samba servers. Server A has 2 DFS links to server B. First link is pointing the the root of a share, second link is pointing to a subdir of the same share. mounting A share and trying to ls or cd in these links result in both pointing to the root of the share B. > I have a patch similar to your second variant which we have > successfully tested in our lab and is currently undergoing user > testing. I'll post it separately as soon as we have the results for > that. > > In my opinion, the patch at the head of the thread here is not related > to this problem and should go in ASAP to fix a regression which affects > all DFS mounts. Agreed. I can confirm that this patch lets me mount //SERVER/share/dfs-main where dfs-main is a DFS link. I was previously getting: mount error(22): Invalid argument Tested-by: Aurelien Aptel <aaptel@suse.com>
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 7ae0328..9878943 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -3661,14 +3661,16 @@ remote_path_check: goto mount_fail_check; } - rc = cifs_are_all_path_components_accessible(server, + if (rc != -EREMOTE) { + rc = cifs_are_all_path_components_accessible(server, xid, tcon, cifs_sb, full_path); - if (rc != 0) { - cifs_dbg(VFS, "cannot query dirs between root and final path, " - "enabling CIFS_MOUNT_USE_PREFIX_PATH\n"); - cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_USE_PREFIX_PATH; - rc = 0; + if (rc != 0) { + cifs_dbg(VFS, "cannot query dirs between root and final path, " + "enabling CIFS_MOUNT_USE_PREFIX_PATH\n"); + cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_USE_PREFIX_PATH; + rc = 0; + } } kfree(full_path); }
Patch a6b5058 results in -EREMOTE returned by is_path_accessible() in cifs_mount() to be ignored which breaks DFS mounting. Signed-off-by: Sachin Prabhu <sprabhu@redhat.com> --- fs/cifs/connect.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)