linux-kernel - Re: [PATCH] ksmbd: Replace strcpy + strcat with scnprintf in convert_to_nt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251118223513.241aed65@pumpkin>
Date: Tue, 18 Nov 2025 22:35:13 +0000
From: David Laight <david.laight.linux@...il.com>
To: Thorsten Blum <thorsten.blum@...ux.dev>
Cc: Namjae Jeon <linkinjeon@...nel.org>, Steve French <smfrench@...il.com>,
 Sergey Senozhatsky <senozhatsky@...omium.org>, Tom Talpey <tom@...pey.com>,
 linux-cifs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ksmbd: Replace strcpy + strcat with scnprintf in
 convert_to_nt_pathname

On Tue, 18 Nov 2025 13:25:56 +0100
Thorsten Blum <thorsten.blum@...ux.dev> wrote:

> strcpy() is deprecated and using strcat() is discouraged; use the safer
> scnprintf() instead.  No functional changes.
> 
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@...ux.dev>
> ---
>  fs/smb/server/misc.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/smb/server/misc.c b/fs/smb/server/misc.c
> index cb2a11ffb23f..86411f947989 100644
> --- a/fs/smb/server/misc.c
> +++ b/fs/smb/server/misc.c
> @@ -164,6 +164,7 @@ char *convert_to_nt_pathname(struct ksmbd_share_config *share,
>  {
>  	char *pathname, *ab_pathname, *nt_pathname;
>  	int share_path_len = share->path_sz;
> +	size_t nt_pathname_len;
>  
>  	pathname = kmalloc(PATH_MAX, KSMBD_DEFAULT_GFP);
>  	if (!pathname)
> @@ -180,15 +181,15 @@ char *convert_to_nt_pathname(struct ksmbd_share_config *share,
>  		goto free_pathname;
>  	}
>  
> -	nt_pathname = kzalloc(strlen(&ab_pathname[share_path_len]) + 2,
> -			      KSMBD_DEFAULT_GFP);
> +	nt_pathname_len = strlen(&ab_pathname[share_path_len]) + 2;
> +	nt_pathname = kzalloc(nt_pathname_len, KSMBD_DEFAULT_GFP);
>  	if (!nt_pathname) {
>  		nt_pathname = ERR_PTR(-ENOMEM);
>  		goto free_pathname;
>  	}
> -	if (ab_pathname[share_path_len] == '\0')
> -		strcpy(nt_pathname, "/");
> -	strcat(nt_pathname, &ab_pathname[share_path_len]);
> +	scnprintf(nt_pathname, nt_pathname_len,
> +		  ab_pathname[share_path_len] == '\0' ? "/%s" : "%s",
> +		  &ab_pathname[share_path_len]);

Ugg...
If nothing else non-constant formats are definitely frowned upon.
Never mind the non-trivial cpu cost of printf.

OTOH once you've got the string length, just use memcpy().
That way you know you won't overflow the malloc buffer even
if someone changes the string on you.

	David



>  
>  	ksmbd_conv_path_to_windows(nt_pathname);
>