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@xxxxxxxxxx> wrote:
> 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
--
Thanks,
Steve
--
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
