Re: Issue with CIFS mounting of DFS ROOT mount when redirecting from one domain controller to the next

[Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Tue, 20 Mar 2012 09:37:43 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Wouldn't this need stable as well - the superblock simplification
> patch went in 10 months ago?
> 

Yeah, that's what I was trying to say in my later comment below, but I
guess I needed more coffee before I wrote it ;)

> On Tue, Mar 20, 2012 at 5:43 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Tue, 20 Mar 2012 06:38:26 -0400
> > Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >
> >> On Mon, 19 Mar 2012 18:21:28 -0400
> >> Thomas Hadig <thomas.hadig-O4KOoiensp/QT0dZR+AlfA@public.gmane.org> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> > 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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>