1. Patchew
  2. linux
  3. View series

[PATCH] platform/x86: surfacepro3_button: replace deprecated strcpy() with strscpy()

Miguel García posted 1 patch 2 months, 1 week ago
There is a newer version of this series
drivers/platform/surface/surfacepro3_button.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] platform/x86: surfacepro3_button: replace deprecated strcpy() with strscpy()
Posted by Miguel García 2 months, 1 week ago 
strcpy() is deprecated for NUL-terminated strings. Replace it with
strscpy() to guarantee NUL-termination. 'name' is a fixed-size local
buffer.

Signed-off-by: Miguel García <miguelgarciaroman8@gmail.com>
---
 drivers/platform/surface/surfacepro3_button.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
index 2755601f979c..9616548283a1 100644
--- a/drivers/platform/surface/surfacepro3_button.c
+++ b/drivers/platform/surface/surfacepro3_button.c
@@ -211,7 +211,7 @@ static int surface_button_add(struct acpi_device *device)
 	}
 
 	name = acpi_device_name(device);
-	strcpy(name, SURFACE_BUTTON_DEVICE_NAME);
+	strscpy(name, SURFACE_BUTTON_DEVICE_NAME, sizeof(name));
 	snprintf(button->phys, sizeof(button->phys), "%s/buttons", hid);
 
 	input->name = name;
-- 
2.34.1
Re: [PATCH] platform/x86: surfacepro3_button: replace deprecated strcpy() with strscpy()
Posted by Krzysztof Kozlowski 2 months, 1 week ago 
On 24/07/2025 09:45, Miguel García wrote:
> strcpy() is deprecated for NUL-terminated strings. Replace it with
> strscpy() to guarantee NUL-termination. 'name' is a fixed-size local
> buffer.
> 
> Signed-off-by: Miguel García <miguelgarciaroman8@gmail.com>
> ---
>  drivers/platform/surface/surfacepro3_button.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
> index 2755601f979c..9616548283a1 100644
> --- a/drivers/platform/surface/surfacepro3_button.c
> +++ b/drivers/platform/surface/surfacepro3_button.c
> @@ -211,7 +211,7 @@ static int surface_button_add(struct acpi_device *device)
>  	}
>  
>  	name = acpi_device_name(device);
> -	strcpy(name, SURFACE_BUTTON_DEVICE_NAME);
> +	strscpy(name, SURFACE_BUTTON_DEVICE_NAME, sizeof(name));


Why are you copying four/eight characters if the string is around 16?

How did you test it (and I doubt you did since you change multiple
different files from different architectures)?


Best regards,
Krzysztof
Re: [PATCH] platform/x86: surfacepro3_button: replace deprecated strcpy() with strscpy()
Posted by Miguel García Román 2 months, 1 week ago 
Hi all,

thanks for the review. You were right about sizeof(name) being the pointer size.
I’ve sent v3 bounding with MAX_ACPI_DEVICE_NAME_LEN. Compile-tested only (x86_64
defconfig/allmodconfig, W=1); no Surface hardware. Apologies for the noise.

Best regards,

El lun, 28 jul 2025 a las 7:30, Krzysztof Kozlowski
(<krzk@kernel.org>) escribió:
>
> On 24/07/2025 09:45, Miguel García wrote:
> > strcpy() is deprecated for NUL-terminated strings. Replace it with
> > strscpy() to guarantee NUL-termination. 'name' is a fixed-size local
> > buffer.
> >
> > Signed-off-by: Miguel García <miguelgarciaroman8@gmail.com>
> > ---
> >  drivers/platform/surface/surfacepro3_button.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
> > index 2755601f979c..9616548283a1 100644
> > --- a/drivers/platform/surface/surfacepro3_button.c
> > +++ b/drivers/platform/surface/surfacepro3_button.c
> > @@ -211,7 +211,7 @@ static int surface_button_add(struct acpi_device *device)
> >       }
> >
> >       name = acpi_device_name(device);
> > -     strcpy(name, SURFACE_BUTTON_DEVICE_NAME);
> > +     strscpy(name, SURFACE_BUTTON_DEVICE_NAME, sizeof(name));
>
>
> Why are you copying four/eight characters if the string is around 16?
>
> How did you test it (and I doubt you did since you change multiple
> different files from different architectures)?
>
>
> Best regards,
> Krzysztof
[PATCH v2] platform/x86: surfacepro3_button: replace deprecated strcpy() with strscpy()
Posted by Miguel García 2 months, 1 week ago 
Signed-off-by: Miguel García <miguelgarciaroman8@gmail.com>
---
 drivers/platform/surface/surfacepro3_button.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
index 2755601f979c..772e107151f6 100644
--- a/drivers/platform/surface/surfacepro3_button.c
+++ b/drivers/platform/surface/surfacepro3_button.c
@@ -211,7 +211,7 @@ static int surface_button_add(struct acpi_device *device)
 	}
 
 	name = acpi_device_name(device);
-	strcpy(name, SURFACE_BUTTON_DEVICE_NAME);
+	strscpy(name, SURFACE_BUTTON_DEVICE_NAME, MAX_ACPI_DEVICE_NAME_LEN);
 	snprintf(button->phys, sizeof(button->phys), "%s/buttons", hid);
 
 	input->name = name;
-- 
2.34.1
[PATCH v3] platform/x86: surfacepro3_button: replace deprecated strcpy() with strscpy()
Posted by Miguel García 2 months, 1 week ago 
Replace strcpy() with strscpy() when copying SURFACE_BUTTON_DEVICE_NAME
into the device’s embedded name buffer returned by acpi_device_name().
Bound the copy with MAX_ACPI_DEVICE_NAME_LEN to guarantee NUL-termination
and avoid pointer-sized sizeof() mistakes.

This is a mechanical safety improvement; functional behavior is unchanged.

Signed-off-by: Miguel García <miguelgarciaroman8@gmail.com>
---
v2:
 - Use MAX_ACPI_DEVICE_NAME_LEN instead of sizeof(name).

v3:
 - Add full commit message (v2 was sent without message).

Testing:
 - Build-tested on x86_64 (defconfig, allmodconfig, W=1).
 - No runtime testing on Surface hardware

 drivers/platform/surface/surfacepro3_button.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
index 2755601f979c..772e107151f6 100644
--- a/drivers/platform/surface/surfacepro3_button.c
+++ b/drivers/platform/surface/surfacepro3_button.c
@@ -211,7 +211,7 @@ static int surface_button_add(struct acpi_device *device)
 	}
 
 	name = acpi_device_name(device);
-	strcpy(name, SURFACE_BUTTON_DEVICE_NAME);
+	strscpy(name, SURFACE_BUTTON_DEVICE_NAME, MAX_ACPI_DEVICE_NAME_LEN);
 	snprintf(button->phys, sizeof(button->phys), "%s/buttons", hid);
 
 	input->name = name;
-- 
2.34.1
Re: [PATCH v3] platform/x86: surfacepro3_button: replace deprecated strcpy() with strscpy()
Posted by Ilpo Järvinen 1 month, 2 weeks ago 
On Mon, 28 Jul 2025, Miguel García wrote:

> Replace strcpy() with strscpy() when copying SURFACE_BUTTON_DEVICE_NAME
> into the device’s embedded name buffer returned by acpi_device_name().
> Bound the copy with MAX_ACPI_DEVICE_NAME_LEN to guarantee NUL-termination
> and avoid pointer-sized sizeof() mistakes.
> 
> This is a mechanical safety improvement; functional behavior is unchanged.
> 
> Signed-off-by: Miguel García <miguelgarciaroman8@gmail.com>
> ---
> v2:
>  - Use MAX_ACPI_DEVICE_NAME_LEN instead of sizeof(name).
> 
> v3:
>  - Add full commit message (v2 was sent without message).
> 
> Testing:
>  - Build-tested on x86_64 (defconfig, allmodconfig, W=1).
>  - No runtime testing on Surface hardware
> 
>  drivers/platform/surface/surfacepro3_button.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
> index 2755601f979c..772e107151f6 100644
> --- a/drivers/platform/surface/surfacepro3_button.c
> +++ b/drivers/platform/surface/surfacepro3_button.c
> @@ -211,7 +211,7 @@ static int surface_button_add(struct acpi_device *device)
>  	}
>  
>  	name = acpi_device_name(device);
> -	strcpy(name, SURFACE_BUTTON_DEVICE_NAME);
> +	strscpy(name, SURFACE_BUTTON_DEVICE_NAME, MAX_ACPI_DEVICE_NAME_LEN);

As mentioned earlier, I'd prefer this to use the two argument version of 
strscpy():

	strscpy(acpi_device_name(device), SURFACE_BUTTON_DEVICE_NAME);

...Changing to that may mean changes to name variable as well (remove 
it?).

-- 
 i.
Re: [PATCH] platform/x86: surfacepro3_button: replace deprecated strcpy() with strscpy()
Posted by Ilpo Järvinen 2 months, 1 week ago 
On Thu, 24 Jul 2025, Miguel García wrote:

> strcpy() is deprecated for NUL-terminated strings. Replace it with
> strscpy() to guarantee NUL-termination. 'name' is a fixed-size local
> buffer.
> 
> Signed-off-by: Miguel García <miguelgarciaroman8@gmail.com>
> ---
>  drivers/platform/surface/surfacepro3_button.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
> index 2755601f979c..9616548283a1 100644
> --- a/drivers/platform/surface/surfacepro3_button.c
> +++ b/drivers/platform/surface/surfacepro3_button.c
> @@ -211,7 +211,7 @@ static int surface_button_add(struct acpi_device *device)
>  	}
>  
>  	name = acpi_device_name(device);
> -	strcpy(name, SURFACE_BUTTON_DEVICE_NAME);
> +	strscpy(name, SURFACE_BUTTON_DEVICE_NAME, sizeof(name));

strscpy() should nowadays support 2 args variant through clever macro 
trickery.

-- 
 i.
Re: [PATCH] platform/x86: surfacepro3_button: replace deprecated strcpy() with strscpy()
Posted by Chen, Yu C 2 months, 1 week ago 
On 7/28/2025 4:32 AM, Ilpo Järvinen wrote:
> On Thu, 24 Jul 2025, Miguel García wrote:
> 
>> strcpy() is deprecated for NUL-terminated strings. Replace it with
>> strscpy() to guarantee NUL-termination. 'name' is a fixed-size local
>> buffer.
>>
>> Signed-off-by: Miguel García <miguelgarciaroman8@gmail.com>
>> ---
>>   drivers/platform/surface/surfacepro3_button.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
>> index 2755601f979c..9616548283a1 100644
>> --- a/drivers/platform/surface/surfacepro3_button.c
>> +++ b/drivers/platform/surface/surfacepro3_button.c
>> @@ -211,7 +211,7 @@ static int surface_button_add(struct acpi_device *device)
>>   	}
>>   
>>   	name = acpi_device_name(device);
>> -	strcpy(name, SURFACE_BUTTON_DEVICE_NAME);
>> +	strscpy(name, SURFACE_BUTTON_DEVICE_NAME, sizeof(name));
> 
> strscpy() should nowadays support 2 args variant through clever macro
> trickery.
> 

Yup,  something like
strscpy(name, SURFACE_BUTTON_DEVICE_NAME);
should be fine, because
name is a array, and SURFACE_BUTTON_DEVICE_NAME is smaller than
the size of name.

thanks,
Chenyu

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.