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

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

[Thomas Hadig]

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

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

[Jeff Layton]

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...

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

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

[Jeff Layton]

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>

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

[Steve French]

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

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



-- 
Thanks,

Steve

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

[Jeff Layton]

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>