[PATCH RESEND] apparmor: Replace deprecated strcpy with memcpy in gen_symlink_name

Thorsten Blum posted 1 patch 2 months, 1 week ago
security/apparmor/apparmorfs.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
[PATCH RESEND] apparmor: Replace deprecated strcpy with memcpy in gen_symlink_name
Posted by Thorsten Blum 2 months, 1 week ago
strcpy() is deprecated; use memcpy() instead. Unlike strcpy(), memcpy()
does not copy the NUL terminator from the source string, which would be
overwritten anyway on every iteration when using strcpy(). snprintf()
then ensures that 'char *s' is NUL-terminated.

Replace the hard-coded path length to remove the magic number 6, and add
a comment explaining the extra 11 bytes.

Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 security/apparmor/apparmorfs.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 391a586d0557..4b2752200ce2 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -1602,16 +1602,20 @@ static char *gen_symlink_name(int depth, const char *dirname, const char *fname)
 {
 	char *buffer, *s;
 	int error;
-	int size = depth * 6 + strlen(dirname) + strlen(fname) + 11;
+	const char *path = "../../";
+	size_t path_len = strlen(path);
+	int size;
 
+	/* Extra 11 bytes: "raw_data" (9) + two slashes "//" (2) */
+	size = depth * path_len + strlen(dirname) + strlen(fname) + 11;
 	s = buffer = kmalloc(size, GFP_KERNEL);
 	if (!buffer)
 		return ERR_PTR(-ENOMEM);
 
 	for (; depth > 0; depth--) {
-		strcpy(s, "../../");
-		s += 6;
-		size -= 6;
+		memcpy(s, path, path_len);
+		s += path_len;
+		size -= path_len;
 	}
 
 	error = snprintf(s, size, "raw_data/%s/%s", dirname, fname);
-- 
2.51.1
Re: [PATCH RESEND] apparmor: Replace deprecated strcpy with memcpy in gen_symlink_name
Posted by John Johansen 2 months, 1 week ago
On 11/26/25 08:57, Thorsten Blum wrote:
> strcpy() is deprecated; use memcpy() instead. Unlike strcpy(), memcpy()
> does not copy the NUL terminator from the source string, which would be
> overwritten anyway on every iteration when using strcpy(). snprintf()
> then ensures that 'char *s' is NUL-terminated.
> 
> Replace the hard-coded path length to remove the magic number 6, and add
> a comment explaining the extra 11 bytes.
> 
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>

hey Thorsten,

sorry I have actually pulled these in, and tested them. I didn't send out
the acks yet because I have another patch that I was waiting on a proper
signed-off-by: on.

I am going to have to pull that one so I can push. I'll add acks now but
the push isn't going to happen for a few hours.

Acked-by: John Johansen <john.johansen@canonical.com>

>   security/apparmor/apparmorfs.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 391a586d0557..4b2752200ce2 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -1602,16 +1602,20 @@ static char *gen_symlink_name(int depth, const char *dirname, const char *fname)
>   {
>   	char *buffer, *s;
>   	int error;
> -	int size = depth * 6 + strlen(dirname) + strlen(fname) + 11;
> +	const char *path = "../../";
> +	size_t path_len = strlen(path);
> +	int size;
>   
> +	/* Extra 11 bytes: "raw_data" (9) + two slashes "//" (2) */
> +	size = depth * path_len + strlen(dirname) + strlen(fname) + 11;
>   	s = buffer = kmalloc(size, GFP_KERNEL);
>   	if (!buffer)
>   		return ERR_PTR(-ENOMEM);
>   
>   	for (; depth > 0; depth--) {
> -		strcpy(s, "../../");
> -		s += 6;
> -		size -= 6;
> +		memcpy(s, path, path_len);
> +		s += path_len;
> +		size -= path_len;
>   	}
>   
>   	error = snprintf(s, size, "raw_data/%s/%s", dirname, fname);
Re: [PATCH RESEND] apparmor: Replace deprecated strcpy with memcpy in gen_symlink_name
Posted by Thorsten Blum 2 months, 1 week ago
On 27. Nov 2025, at 02:32, John Johansen wrote:
> hey Thorsten,
> 
> sorry I have actually pulled these in, and tested them. I didn't send out
> the acks yet because I have another patch that I was waiting on a proper
> signed-off-by: on.
> 
> I am going to have to pull that one so I can push. I'll add acks now but
> the push isn't going to happen for a few hours.
> 
> Acked-by: John Johansen <john.johansen@canonical.com>

Ah sorry for resending then, didn't know you looked at them already.

Thanks,
Thorsten
Re: [PATCH RESEND] apparmor: Replace deprecated strcpy with memcpy in gen_symlink_name
Posted by Thorsten Blum 6 days, 22 hours ago
Hi John,

On 27. Nov 2025, at 11:18, Thorsten Blum wrote:
> On 27. Nov 2025, at 02:32, John Johansen wrote:
>> hey Thorsten,
>> 
>> sorry I have actually pulled these in, and tested them. I didn't send out
>> the acks yet because I have another patch that I was waiting on a proper
>> signed-off-by: on.
>> 
>> I am going to have to pull that one so I can push. I'll add acks now but
>> the push isn't going to happen for a few hours.
>> 
>> Acked-by: John Johansen <john.johansen@canonical.com>
> 
> Ah sorry for resending then, didn't know you looked at them already.

Did you ever push the commits? I can't find them anywhere.

Thanks,
Thorsten
Re: [PATCH RESEND] apparmor: Replace deprecated strcpy with memcpy in gen_symlink_name
Posted by John Johansen 4 days, 3 hours ago
On 2/1/26 04:42, Thorsten Blum wrote:
> Hi John,
> 
> On 27. Nov 2025, at 11:18, Thorsten Blum wrote:
>> On 27. Nov 2025, at 02:32, John Johansen wrote:
>>> hey Thorsten,
>>>
>>> sorry I have actually pulled these in, and tested them. I didn't send out
>>> the acks yet because I have another patch that I was waiting on a proper
>>> signed-off-by: on.
>>>
>>> I am going to have to pull that one so I can push. I'll add acks now but
>>> the push isn't going to happen for a few hours.
>>>
>>> Acked-by: John Johansen <john.johansen@canonical.com>
>>
>> Ah sorry for resending then, didn't know you looked at them already.
> 
> Did you ever push the commits? I can't find them anywhere.
> 
sorry I am having "fun" email issue atm

it should be in apparmor-next. I have been staging that through gitlab

https://gitlab.com/apparmor/apparmor-kernel
and
https://gitlab.com/jjohansen/apparmor-kernel

trying to get better CI integration with the userspace side. It was stuck
again and I had to manually kick it last week.