[PATCH] x86/cpu/intel: replace deprecated strcpy with strscpy

Ruben Wauters posted 1 patch 7 months, 1 week ago
There is a newer version of this series
arch/x86/kernel/cpu/intel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] x86/cpu/intel: replace deprecated strcpy with strscpy
Posted by Ruben Wauters 7 months, 1 week ago
strcpy is deprecated due to lack of bounds checking.
This patch replaces strcpy with strscpy, the recommended alternative for
null terminated strings, to follow best practices.

Signed-off-by: Ruben Wauters <rubenru09@aol.com>
---
 arch/x86/kernel/cpu/intel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 584dd55bf739..b49bba30434d 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -607,7 +607,7 @@ static void init_intel(struct cpuinfo_x86 *c)
 		}
 
 		if (p)
-			strcpy(c->x86_model_id, p);
+			strscpy(c->x86_model_id, p);
 	}
 #endif
 
-- 
2.48.1
Re: [PATCH] x86/cpu/intel: replace deprecated strcpy with strscpy
Posted by H. Peter Anvin 7 months, 1 week ago
On May 7, 2025 11:51:36 AM PDT, Ruben Wauters <rubenru09@aol.com> wrote:
>strcpy is deprecated due to lack of bounds checking.
>This patch replaces strcpy with strscpy, the recommended alternative for
>null terminated strings, to follow best practices.
>
>Signed-off-by: Ruben Wauters <rubenru09@aol.com>
>---
> arch/x86/kernel/cpu/intel.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
>index 584dd55bf739..b49bba30434d 100644
>--- a/arch/x86/kernel/cpu/intel.c
>+++ b/arch/x86/kernel/cpu/intel.c
>@@ -607,7 +607,7 @@ static void init_intel(struct cpuinfo_x86 *c)
> 		}
> 
> 		if (p)
>-			strcpy(c->x86_model_id, p);
>+			strscpy(c->x86_model_id, p);
> 	}
> #endif
> 

strscpy() needs a buffer length; this patch wouldn't even compile!

Not to mention that the string in question is generated in such a way that cannot be unterminated.
Re: [PATCH] x86/cpu/intel: replace deprecated strcpy with strscpy
Posted by Ruben Wauters 7 months, 1 week ago
On Wed, 2025-05-07 at 13:14 -0700, H. Peter Anvin wrote:
> On May 7, 2025 11:51:36 AM PDT, Ruben Wauters <rubenru09@aol.com>
> wrote:
> > strcpy is deprecated due to lack of bounds checking.
> > This patch replaces strcpy with strscpy, the recommended
> > alternative for
> > null terminated strings, to follow best practices.
> > 
> > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> > ---
> > arch/x86/kernel/cpu/intel.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/intel.c
> > b/arch/x86/kernel/cpu/intel.c
> > index 584dd55bf739..b49bba30434d 100644
> > --- a/arch/x86/kernel/cpu/intel.c
> > +++ b/arch/x86/kernel/cpu/intel.c
> > @@ -607,7 +607,7 @@ static void init_intel(struct cpuinfo_x86 *c)
> > 		}
> > 
> > 		if (p)
> > -			strcpy(c->x86_model_id, p);
> > +			strscpy(c->x86_model_id, p);
> > 	}
> > #endif
> > 
> 
> strscpy() needs a buffer length; this patch wouldn't even compile!

Hi, this is incorrect, strscpy is defined in string.h as
#define strscpy(dst, src, ...)	\
	CONCATENATE(__strscpy, COUNT_ARGS(__VA_ARGS__))(dst, src,
__VA_ARGS__)

the third parameter is optional, and it works perfectly fine with two
parameters. I have compiled it, and there are no errors.

> Not to mention that the string in question is generated in such a way
> that cannot be unterminated.

I'm not entirely sure what you mean here? The assignments above are
null terminated strings, which the two parameter version works fine
with.
Re: [PATCH] x86/cpu/intel: replace deprecated strcpy with strscpy
Posted by Ruben Wauters 7 months ago
On Wed, 2025-05-07 at 21:30 +0100, Ruben Wauters wrote:
> On Wed, 2025-05-07 at 13:14 -0700, H. Peter Anvin wrote:
> > On May 7, 2025 11:51:36 AM PDT, Ruben Wauters <rubenru09@aol.com>
> > wrote:
> > > strcpy is deprecated due to lack of bounds checking.
> > > This patch replaces strcpy with strscpy, the recommended
> > > alternative for
> > > null terminated strings, to follow best practices.
> > > 
> > > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
> > > ---
> > > arch/x86/kernel/cpu/intel.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/x86/kernel/cpu/intel.c
> > > b/arch/x86/kernel/cpu/intel.c
> > > index 584dd55bf739..b49bba30434d 100644
> > > --- a/arch/x86/kernel/cpu/intel.c
> > > +++ b/arch/x86/kernel/cpu/intel.c
> > > @@ -607,7 +607,7 @@ static void init_intel(struct cpuinfo_x86 *c)
> > > 		}
> > > 
> > > 		if (p)
> > > -			strcpy(c->x86_model_id, p);
> > > +			strscpy(c->x86_model_id, p);
> > > 	}
> > > #endif
> > > 
> > 
> > strscpy() needs a buffer length; this patch wouldn't even compile!
> 
> Hi, this is incorrect, strscpy is defined in string.h as
> #define strscpy(dst, src, ...)	\
> 	CONCATENATE(__strscpy, COUNT_ARGS(__VA_ARGS__))(dst, src,
> __VA_ARGS__)
> 
> the third parameter is optional, and it works perfectly fine with two
> parameters. I have compiled it, and there are no errors.
> 
> > Not to mention that the string in question is generated in such a
> > way
> > that cannot be unterminated.
> 
> I'm not entirely sure what you mean here? The assignments above are
> null terminated strings, which the two parameter version works fine
> with.

Hello

Just wanted to check that everything was ok with this patch, and that
any concerns were addressed or explained. Please do let me know if
there is anything I need to do or change to get this patch applied.

Ruben Wauters
Re: [PATCH] x86/cpu/intel: replace deprecated strcpy with strscpy
Posted by H. Peter Anvin 7 months ago
On May 14, 2025 12:16:20 PM PDT, Ruben Wauters <rubenru09@aol.com> wrote:
>On Wed, 2025-05-07 at 21:30 +0100, Ruben Wauters wrote:
>> On Wed, 2025-05-07 at 13:14 -0700, H. Peter Anvin wrote:
>> > On May 7, 2025 11:51:36 AM PDT, Ruben Wauters <rubenru09@aol.com>
>> > wrote:
>> > > strcpy is deprecated due to lack of bounds checking.
>> > > This patch replaces strcpy with strscpy, the recommended
>> > > alternative for
>> > > null terminated strings, to follow best practices.
>> > > 
>> > > Signed-off-by: Ruben Wauters <rubenru09@aol.com>
>> > > ---
>> > > arch/x86/kernel/cpu/intel.c | 2 +-
>> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > > 
>> > > diff --git a/arch/x86/kernel/cpu/intel.c
>> > > b/arch/x86/kernel/cpu/intel.c
>> > > index 584dd55bf739..b49bba30434d 100644
>> > > --- a/arch/x86/kernel/cpu/intel.c
>> > > +++ b/arch/x86/kernel/cpu/intel.c
>> > > @@ -607,7 +607,7 @@ static void init_intel(struct cpuinfo_x86 *c)
>> > > 		}
>> > > 
>> > > 		if (p)
>> > > -			strcpy(c->x86_model_id, p);
>> > > +			strscpy(c->x86_model_id, p);
>> > > 	}
>> > > #endif
>> > > 
>> > 
>> > strscpy() needs a buffer length; this patch wouldn't even compile!
>> 
>> Hi, this is incorrect, strscpy is defined in string.h as
>> #define strscpy(dst, src, ...)	\
>> 	CONCATENATE(__strscpy, COUNT_ARGS(__VA_ARGS__))(dst, src,
>> __VA_ARGS__)
>> 
>> the third parameter is optional, and it works perfectly fine with two
>> parameters. I have compiled it, and there are no errors.
>> 
>> > Not to mention that the string in question is generated in such a
>> > way
>> > that cannot be unterminated.
>> 
>> I'm not entirely sure what you mean here? The assignments above are
>> null terminated strings, which the two parameter version works fine
>> with.
>
>Hello
>
>Just wanted to check that everything was ok with this patch, and that
>any concerns were addressed or explained. Please do let me know if
>there is anything I need to do or change to get this patch applied.
>
>Ruben Wauters
>

Yes, I stand corrected. 

I still think it is superfluous (or arguably a memcpy would be better, since this is a fixed length) but it doesn't hurt enything.