On Tue, 20 Mar 2012 06:38:26 -0400
Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> 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...
>
FWIW, this looks like a regression that was introduced in commit
724d9f1c. That patch should be appropriate for stable as well. When you
add that line, you should also be able to get rid of the initialization
of rc in the declaration too.
--
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
