4.4-stable review patch. If anyone has any objections, please let me know.
------------------
[ Upstream commit 40920c2bb119fd49ba03e2f97a172171781be442 ]
Commit 1a967d6c9b39c226be1b45f13acd4d8a5ab3dc44 ("correctly to anonymous authentication for the NTLM(v2) authentication") introduces a regression in handling errors related to attempting a guest connection to a Windows share which requires authentication. This should result in a permission denied error but actually causes the kernel module to enter a never-ending loop trying to follow a DFS referal which doesn't exist.
The base cause of this is the failure now occurs later in the process during tree connect and not at the session setup setup and all errors in tree connect are interpreted as needing to follow the DFS paths which isn't in this case correct. So, check the returned error against EACCES and fail if this is returned error.
Feedback from Aurelien:
PS> net user guest /activate:no PS> mkdir C:\guestshare PS> icacls C:\guestshare /grant 'Everyone:(OI)(CI)F' PS> new-smbshare -name guestshare -path C:\guestshare -fullaccess Everyone
I've tested v3.10, v4.4, master, master+your patch using default options (empty or no user "NU") and user=abc (U).
NT_LOGON_FAILURE in session setup: LF This is what you seem to have in 3.10.
NT_ACCESS_DENIED in tree connect to the share: AD This is what you get before your infinite loop.
| NU U -------------------------------- 3.10 | LF LF 4.4 | LF LF master | AD LF master+patch | AD LF
No infinite DFS loop :( All these issues result in mount failing very fast with permission denied.
I guess it could be from either the Windows version or the share/folder ACL. A deeper analysis of the packets might reveal more.
In any case I did not notice any issues for on a basic DFS setup with the patch so I don't think it introduced any regressions, which is probably all that matters. It still bothers me a little I couldn't hit the bug.
I've included kernel output w/ debugging output and network capture of my tests if anyone want to have a look at it. (master+patch = ml-guestfix).
Signed-off-by: Mark Syms mark.syms@citrix.com Reviewed-by: Aurelien Aptel aaptel@suse.com Tested-by: Aurelien Aptel aaptel@suse.com Acked-by: Pavel Shilovsky pshilov@microsoft.com Signed-off-by: Steve French smfrench@gmail.com Signed-off-by: Sasha Levin sashal@kernel.org --- fs/cifs/connect.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 077ad3a06c9a..1eeb4780c3ed 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -3674,6 +3674,9 @@ try_mount_again: if (IS_ERR(tcon)) { rc = PTR_ERR(tcon); tcon = NULL; + if (rc == -EACCES) + goto mount_fail_check; + goto remote_path_check; }