On Mon, 19 Mar 2012 18:21:28 -0400 Thomas Hadig <thomas.hadig@xxxxxxxxxx> wrote: > Hi, > > I am using the CIFS module from Linux kernel 3.2.9. > > We have the following setup: > > In DNS, domain.net resolves to our secondary AD server ad2.domain.net > Our primary domain controller is ad1.domain.net and has a DFS redirection set up from \\ad1\share\Users to \\files\share\Users. > > When mounting \\domain.net\share\Users I get an error. > > I traced this down to the cifs_mount function in fs/cifs/connect.c. In there, rc is set to 0 at the beginning but never reset. > In short, adding an rc=0; line after the label try_mount_again: fixes the issue. > > Detailed information: > - Connecting to \\domain.net\share\Users connects to ad2 with a SMB_COM_NEGOTIATE, SMB_COM_SESSION_SETUP_ANDX, and SMB_COM_TREE_CONNECT_ANDX. > - SMB_COM_TREE_CONNECT_ANDX fails with STATUS_BAD_NETWORK_NAME causing rc to be non-zero and jumping to remote_path_check: > - It connects to ad2's IPC$ share and retrieves the DFS_REFERRAL which leads to \\ad1\share and jumps to try_mount_again: > - It closes the samba session to ad2. > - Now, it connects to ad1 with SMB_COM_NEGOTIATE, SMB_COM_SESSION_SETUP_ANDX, and SMB_COM_TREE_CONNECT_ANDX. > - This time, it succeeds and continues with SMB_COM_TRANSACTION2/ TRANS2_QUERY_FS_INFORMATION/SMB_QUERY_FS_DEVICE_INFO and SMB_QUERY_FS_ATTRIBUTE_INFO. > - It comes to: > /* check if a whole path is not remote */ > if (!rc && tcon) { > /* build_path_to_root works only when we have a valid tcon */ > where rc is still non-zero and, thus, does NOT check again for a DFS_REFERRAL. > - I closes the samba session to ad1 and exists with the -6 mount error from the initial tree connect. > > The correct behavior would be to reset rc to 0 when connecting to a different samba session. > > > Patch info: > > rc = bdi_setup_and_register(&cifs_sb->bdi, "cifs", BDI_CAP_MAP_COPY); > if (rc) > return rc; > > #ifdef CONFIG_CIFS_DFS_UPCALL > try_mount_again: > /* cleanup activities if we're chasing a referral */ > if (referral_walks_count) { > if (tcon) > cifs_put_tcon(tcon); > else if (pSesInfo) > cifs_put_smb_ses(pSesInfo); > > FreeXid(xid); > } > #endif > tcon = NULL; > pSesInfo = NULL; > srvTcp = NULL; > full_path = NULL; > tlink = NULL; > + rc=0; > > xid = GetXid(); > > /* get a reference to a tcp session */ > srvTcp = cifs_get_tcp_session(volume_info); > if (IS_ERR(srvTcp)) { > rc = PTR_ERR(srvTcp); > bdi_destroy(&cifs_sb->bdi); > goto out; > } > > > > Thanks > Thomas > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html Nice analysis. That looks correct to me too... Care to spin up a patch with a description and signed-off-by line? You can add my Reviewed-by if so... -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html