Re: [PATCH] smb: client: fix native SMB symlink traversal

[Stefan Metzmacher <metze@samba.org>]

Am 02.07.25 um 19:40 schrieb Paulo Alcantara:
> We've seen customers having shares mounted in paths like /??/C:/ or
> /??/UNC/foo.example.com/share in order to get their native SMB
> symlinks successfully followed from different mounts.
> 
> After commit 12b466eb52d9 ("cifs: Fix creating and resolving absolute NT-style symlinks"),
> the client would then convert absolute paths from /??/C:/ to "/mnt/c/"
> by default.  The absolute paths would vary depending on the value of
> symlink= mount option.
> 
> Fix this by restoring old behavior of not trying to convert absolute
> paths by default.  Only do this if symlinkroot= was _explicitly_ set.
> 
> Before patch:
> 
>    $ mount.cifs //w22-fs0/test2 /mnt/1 -o vers=3.1.1,username=xxx,password=yyy
>    $ ls -l /mnt/1/symlink2
>    lrwxr-xr-x 1 root root 15 Jun 20 14:22 /mnt/1/symlink2 -> /mnt/c/testfile
>    $ mkdir -p /??/C:; echo foo > //??/C:/testfile
>    $ cat /mnt/1/symlink2
>    cat: /mnt/1/symlink2: No such file or directory
> 
> After patch:
> 
>    $ mount.cifs //w22-fs0/test2 /mnt/1 -o vers=3.1.1,username=xxx,password=yyy
>    $ ls -l /mnt/1/symlink2
>    lrwxr-xr-x 1 root root 15 Jun 20 14:22 /mnt/1/symlink2 -> '/??/C:/testfile'
>    $ mkdir -p /??/C:; echo foo > //??/C:/testfile
>    $ cat /mnt/1/symlink2
>    foo
> 
> Cc: linux-cifs@vger.kernel.org
> Cc: Pierguido Lambri <plambri@redhat.com>
> Cc: David Howells <dhowells@redhat.com>
> Fixes: 12b466eb52d9 ("cifs: Fix creating and resolving absolute NT-style symlinks")
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> ---
>   fs/smb/client/fs_context.c | 13 ++++---------
>   fs/smb/client/reparse.c    | 22 +++++++++++++---------
>   2 files changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
> index a634a34d4086..d8d2d4a739e8 100644
> --- a/fs/smb/client/fs_context.c
> +++ b/fs/smb/client/fs_context.c
> @@ -1825,9 +1825,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
>   			goto cifs_parse_mount_err;
>   		}
>   		kfree(ctx->symlinkroot);
> -		ctx->symlinkroot = kstrdup(param->string, GFP_KERNEL);
> -		if (!ctx->symlinkroot)
> +		ctx->symlinkroot = kstrndup(param->string, PATH_MAX, GFP_KERNEL);

Should we really truncate the string instead of generating an error?
I really don't know, maybe it is a good thing, but we should have a comment
that explains it why we truncate.

Thanks!
metze