Re: [PATCH v2 next] drivers/misc/kgdbts: Replace strlen() strcpy() pair with strscpy()

[Daniel Thompson <daniel@riscstar.com>]

On Fri, Jun 26, 2026 at 09:38:21AM +0100, David Laight wrote:
> Use the result of strscpy() for the options overflow check,
> if too long delete the config string.

This still seems like very little information for future adventurers to
go on.

IIUC you are setting config[0] = 0 because that puts the string back as
it was before the strscpy() altered it by writing in a truncated value?
And that is based on the assumption that config is still zero-filled when
kgdbts_option_setup() runs?

If that's why this code was added then explain that rather than
explaining what it does.

Also, if that really is the rationale then would strlen()/memcpy() be
cleaner? That way you would never write a bad value into config in the
first place.


Daniel.



> Use two argument strscpy(config, kmessage) to ensure no overflow.
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>
> v2: Note that an overlong config string gets deleted.
>
> This is one of a group of patches that remove potentially unbounded
> strcpy() calls.
>
> They are mostly replaced by strscpy() or, when strlen() has just been
> called, with memcpy() (usually including the '\0').
>
> Calls with copy string literals into arrays are left unchanged.
> They are safe and easily detected as such.
>
> The changes were made by getting the compiler to detect the calls and
> then fixing the code by hand.
>
> Note that all the changes are only compile tested.
>
> Some Makefiles were changed to allow files to contain strcpy().
> As well as 'difficult to fix' files, this included 'show' functions
> as they really need to use sysfs_emit() or seq_printf().
>
> All the patches are being sent individually to avoid very long cc lists.
> Apologies for the terse commit messages and likely unexpected tags.
> (There are about 100 patches in total.)
>
>  drivers/misc/kgdbts.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
> index 9d3218330f0a..2c8f10b8ac74 100644
> --- a/drivers/misc/kgdbts.c
> +++ b/drivers/misc/kgdbts.c
> @@ -1069,11 +1069,10 @@ static void kgdbts_run_tests(void)
>
>  static int __init kgdbts_option_setup(char *opt)
>  {
> -	if (strlen(opt) >= MAX_CONFIG_LEN) {
> +	if (strscpy(config, opt) < 0) {
> +		config[0] = 0;
>  		printk(KERN_ERR "kgdbts: config string too long\n");
> -		return 1;
>  	}
> -	strcpy(config, opt);
>  	return 1;
>  }
>
> @@ -1144,7 +1143,7 @@ static int param_set_kgdbts_var(const char *kmessage,
>
>  	/* Only copy in the string if the init function has not run yet */
>  	if (configured < 0) {
> -		strcpy(config, kmessage);
> +		strscpy(config, kmessage);
>  		return 0;
>  	}
>
> @@ -1153,7 +1152,7 @@ static int param_set_kgdbts_var(const char *kmessage,
>  		return -EBUSY;
>  	}
>
> -	strcpy(config, kmessage);
> +	strscpy(config, kmessage);
>  	/* Chop out \n char as a result of echo */
>  	if (len && config[len - 1] == '\n')
>  		config[len - 1] = '\0';
> --
> 2.39.5
>