ksmbd: Replace strcpy + strcat with scnprintf in convert_to_nt_pathname

[PATCH] ksmbd: Replace strcpy + strcat with scnprintf in convert_to_nt_pathname

Posted by Thorsten Blum 1 week, 6 days ago

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@linux.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]);
 
 	ksmbd_conv_path_to_windows(nt_pathname);
 
-- 
2.51.1

Re: [PATCH] ksmbd: Replace strcpy + strcat with scnprintf in convert_to_nt_pathname

Posted by David Laight 1 week, 6 days ago

On Tue, 18 Nov 2025 13:25:56 +0100
Thorsten Blum <thorsten.blum@linux.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@linux.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);
>

Re: [PATCH] ksmbd: Replace strcpy + strcat with scnprintf in convert_to_nt_pathname

Posted by Thorsten Blum 1 week, 6 days ago

On 18. Nov 2025, at 23:35, David Laight wrote:
> On Tue, 18 Nov 2025 13:25:56 +0100
> Thorsten Blum <thorsten.blum@linux.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@linux.dev>
>> ---
>> [...]
> 
> 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.

Ok, I'll submit a v2.

Thanks,
Thorsten