Bug 219083 - Failure to mount DFS namespaces without ASCII symbols
Summary: Failure to mount DFS namespaces without ASCII symbols
Status: NEW
Alias: None
Product: File System
Classification: Unclassified
Component: CIFS (show other bugs)
Hardware: All Linux
: P3 normal
Assignee: fs_cifs
URL:
Keywords:
: 215440 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-07-22 10:59 UTC by Gleb Korobeynikov
Modified: 2024-10-29 00:39 UTC (History)
5 users (show)

See Also:
Kernel Version: 6.3.13+
Subsystem:
Regression: Yes
Bisected commit-id: 3ae872de410751fe5e629e04da491a632d95201c


Attachments
pcap file for wireshark (920 bytes, application/vnd.tcpdump.pcap)
2024-07-22 11:01 UTC, Gleb Korobeynikov
Details
dmesg (19.59 KB, text/plain)
2024-07-22 11:01 UTC, Gleb Korobeynikov
Details
patch (988 bytes, patch)
2024-07-22 11:08 UTC, Gleb Korobeynikov
Details | Diff
proposed patch (1.55 KB, application/mbox)
2024-08-08 16:03 UTC, Gleb Korobeynikov
Details

Description Gleb Korobeynikov 2024-07-22 10:59:46 UTC
Windows version of SMB host: Windows Server 2022 Standard x64
Kernel: 6.3.13(upstream)
CONFIG_CIFS_DFS_UPCALL

In the function cifs_inval_name_dfs_link_error(), a check was added for tcon->origin_fullpath (3ae872de410751fe5e629e04da491a632d95201c). I believe it's unnecessary because when mounting a dfs name without ASCII characters, we always fail at this check and exit the function, leading to dfs namespaces not being mounted

Steps to reproduce:

1. At Windows, create DFS namespace with name containing non-ASCII symbols (for example дфс)

2. mount -t cifs \\\\<smb_server>\\дфс  /tmp/dfs -o domain=...,user=...,password=...

result:
mount error(2): No such file or directory
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)

CIFS debug log:
[Mon Jul 22 11:00:24 2024] CIFS: Status code returned 0xc0000033 STATUS_OBJECT_NAME_INVALID
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/smb2maperror.c: Mapping SMB2 status code 0xc0000033 to POSIX err -2
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/dfs_cache.c: dfs_cache_noreq_update_tgthint: path: \test.local\дфс
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: __cifs_put_smb_ses: ses_count=2
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: __cifs_put_smb_ses: ses ipc: \\test.local\IPC$
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: cifs_put_tcon: tc_count=1
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: VFS: in cifs_put_tcon as Xid: 17 with uid: 0
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/smb2pdu.c: Tree Disconnect
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/fscache.c: cifs_fscache_release_super_cookie: (0x0000000000000000)
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: __cifs_put_smb_ses: ses_count=1
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: __cifs_put_smb_ses: ses ipc: \\DC.test.local\IPC$
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: VFS: in __cifs_put_smb_ses as Xid: 18 with uid: 0
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/smb2pdu.c: disconnect session 00000000360c6881
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: __cifs_put_smb_ses: ses_count=1
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: __cifs_put_smb_ses: ses ipc: \\test.local\IPC$
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: VFS: in __cifs_put_smb_ses as Xid: 19 with uid: 0
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/smb2pdu.c: disconnect session 00000000db1ddbb6
[Mon Jul 22 11:00:24 2024] CIFS: fs/smb/client/connect.c: VFS: leaving cifs_mount_put_conns (xid = 13) rc = 0
[Mon Jul 22 11:00:24 2024] CIFS: VFS: cifs_mount failed w/return code = -2
Comment 1 Gleb Korobeynikov 2024-07-22 11:01:10 UTC
Created attachment 306605 [details]
pcap file for wireshark
Comment 2 Gleb Korobeynikov 2024-07-22 11:01:31 UTC
Created attachment 306606 [details]
dmesg
Comment 3 Gleb Korobeynikov 2024-07-22 11:08:10 UTC
Created attachment 306607 [details]
patch
Comment 4 The Linux kernel's regression tracker (Thorsten Leemhuis) 2024-07-24 09:15:33 UTC
Please check if 6.10 (or 6.11-rc1 once it's out on Monday) is still affected
Comment 5 Gleb Korobeynikov 2024-07-24 09:19:07 UTC
(In reply to The Linux kernel's regression tracker (Thorsten Leemhuis) from comment #4)
> Please check if 6.10 (or 6.11-rc1 once it's out on Monday) is still affected

Alright, I will definitely check
Comment 6 Gleb Korobeynikov 2024-07-30 11:03:01 UTC
(In reply to Gleb Korobeynikov from comment #5)
> (In reply to The Linux kernel's regression tracker (Thorsten Leemhuis) from
> comment #4)
> > Please check if 6.10 (or 6.11-rc1 once it's out on Monday) is still
> affected
> 
> Alright, I will definitely check

Checked on 6.11-rc1. The reproduction issue happens identically.
Comment 7 The Linux kernel's regression tracker (Thorsten Leemhuis) 2024-07-30 11:15:44 UTC
(In reply to Gleb Korobeynikov from comment #6)

> Checked on 6.11-rc1. The reproduction issue happens identically.

Thx; I'd like to forward this by mail; can I CC you? this would expose your email to the public
Comment 8 Gleb Korobeynikov 2024-07-30 11:17:28 UTC
(In reply to The Linux kernel's regression tracker (Thorsten Leemhuis) from comment #7)
> (In reply to Gleb Korobeynikov from comment #6)
> 
> > Checked on 6.11-rc1. The reproduction issue happens identically.
> 
> Thx; I'd like to forward this by mail; can I CC you? this would expose your
> email to the public

Yes, of course
Comment 9 Paulo Alcantara 2024-08-05 01:54:09 UTC
Hi Gleb,

Thanks for the report and reproducer.  Sorry for the delay as I've
been quite busy with some other stuff.

The patch looks good as that check will always be true for a new @tcon
during mount as @tcon->origin_fullpath will only get set after tree
connecting to last share as well as checking if prefix paths are fully
accessible from it.

The problem seems worst than that because even with your patch, I
still can't access any directories under the mounted share:

  # mount.cifs //w22-root1.zelda.test/дфс /mnt/1 -o domain=zelda.test,user=xxx,password=yyy,nohandlecache
  # ls /mnt/1
  dir
  # cd /mnt/1/dir
  -bash: cd: /mnt/1/dir: No such file or directory

Or if mounting the DFS share with a prefix path:

  # mount.cifs //w22-root1.zelda.test/дфс/dir /mnt/1 -o domain=zelda.test,user=xxx,password=yyy,nohandlecache
  mount error(2): No such file or directory
  Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)

I've tested it with Windows 11 client and I don't see any
STATUS_OBJECT_NAME_INVALID errors being returned from server when
accessing directories and DFS links under the mounted share, which
really indicates another bug in cifs.ko that we haven't handled, yet.

By comparing the network traces from Windows 11 and cifs.ko client,
I've noticed that cifs.ko is sending the filenames
(e.g. w22-root1.zelda.test\дфс\...) in SMB2_CREATE requests with some
trailing NULL bytes, which might explain why we're getting those weird
STATUS_OBJECT_NAME_INVALID errors.  This is happening only with DFS
shares because we append the filenames with tree name for DFS path
normalization as required by MS-SMB2.
Comment 10 Gleb Korobeynikov 2024-08-05 09:10:21 UTC
(In reply to Paulo Alcantara from comment #9)
> Hi Gleb,
> 
> Thanks for the report and reproducer.  Sorry for the delay as I've
> been quite busy with some other stuff.
> 
> The patch looks good as that check will always be true for a new @tcon
> during mount as @tcon->origin_fullpath will only get set after tree
> connecting to last share as well as checking if prefix paths are fully
> accessible from it.
> 
> The problem seems worst than that because even with your patch, I
> still can't access any directories under the mounted share:
> 
>   # mount.cifs //w22-root1.zelda.test/дфс /mnt/1 -o
> domain=zelda.test,user=xxx,password=yyy,nohandlecache
>   # ls /mnt/1
>   dir
>   # cd /mnt/1/dir
>   -bash: cd: /mnt/1/dir: No such file or directory
> 
> Or if mounting the DFS share with a prefix path:
> 
>   # mount.cifs //w22-root1.zelda.test/дфс/dir /mnt/1 -o
> domain=zelda.test,user=xxx,password=yyy,nohandlecache
>   mount error(2): No such file or directory
>   Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel
> log messages (dmesg)
> 
> I've tested it with Windows 11 client and I don't see any
> STATUS_OBJECT_NAME_INVALID errors being returned from server when
> accessing directories and DFS links under the mounted share, which
> really indicates another bug in cifs.ko that we haven't handled, yet.
> 
> By comparing the network traces from Windows 11 and cifs.ko client,
> I've noticed that cifs.ko is sending the filenames
> (e.g. w22-root1.zelda.test\дфс\...) in SMB2_CREATE requests with some
> trailing NULL bytes, which might explain why we're getting those weird
> STATUS_OBJECT_NAME_INVALID errors.  This is happening only with DFS
> shares because we append the filenames with tree name for DFS path
> normalization as required by MS-SMB2.

Hi Paulo,

Thank you for your response! Will you accept my patch with always positive verification for the new @tcon->origin_fullpath?

I'll try to test this with Windows 11, I'll check the traces.
Comment 11 Paulo Alcantara 2024-08-05 18:02:27 UTC
Hi Gleb,

bugzilla-daemon@kernel.org writes:

> Thank you for your response! Will you accept my patch with always positive
> verification for the new @tcon->origin_fullpath?

It's not up to me deciding whether or not to apply it as I'm not the
upstream CIFS maintainer, but I'd rather have that check replaced with

        struct TCP_Server_Info *server = tcon->ses->server;
        ...
        spin_lock(&server->srv_lock);
        if (!server->leaf_fullpath) {
                spin_unlock(&server->srv_lock);
                return 0;
        }
        spin_unlock(&server->srv_lock);

to avoid having this workaround executed in non-DFS paths.

Can you test if the above also works for you?  If so, please send a
formal patch and I'll gladly review it.

Thanks!
Comment 12 Steve French 2024-08-05 21:01:32 UTC
No objections to the patch idea so far but let's see it on the mailing list so can get review feedback
Comment 13 Gleb Korobeynikov 2024-08-06 04:45:58 UTC
(In reply to Paulo Alcantara from comment #11)
> Hi Gleb,
> 
> bugzilla-daemon@kernel.org writes:
> 
> > Thank you for your response! Will you accept my patch with always positive
> > verification for the new @tcon->origin_fullpath?
> 
> It's not up to me deciding whether or not to apply it as I'm not the
> upstream CIFS maintainer, but I'd rather have that check replaced with
> 
>         struct TCP_Server_Info *server = tcon->ses->server;
>         ...
>         spin_lock(&server->srv_lock);
>         if (!server->leaf_fullpath) {
>                 spin_unlock(&server->srv_lock);
>                 return 0;
>         }
>         spin_unlock(&server->srv_lock);
> 
> to avoid having this workaround executed in non-DFS paths.
> 
> Can you test if the above also works for you?  If so, please send a
> formal patch and I'll gladly review it.
> 
> Thanks!

I'll check and write you the result.
Comment 14 nandaa 2024-08-08 06:07:34 UTC
Also duplicate here? https://bugzilla.kernel.org/show_bug.cgi?id=215440
Comment 15 Gleb Korobeynikov 2024-08-08 16:03:56 UTC
Created attachment 306700 [details]
proposed patch

Paulo, this check helped me. I'm attaching this patch.
Comment 16 Steve French 2024-08-09 04:06:16 UTC
I have merged the patch into cifs-2.6.git for-next tentatively (as we wait for any additional testing or review feedback)
Comment 17 Artem S. Tashkinov 2024-10-28 10:31:54 UTC
*** Bug 215440 has been marked as a duplicate of this bug. ***
Comment 18 Steve French 2024-10-29 00:39:31 UTC
Any updates on whether the patches addressed all the problems (and can be closed) - or whether additional fixes are needed.

Note You need to log in before you can comment on or make changes to this bug.