linux-kernel - Re: [PATCH RESEND] apparmor: Replace deprecated strcpy with memcpy in gen_symlink

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1da23c89-dc2c-41cb-8260-098deb8ae917@canonical.com>
Date: Wed, 26 Nov 2025 17:32:56 -0800
From: John Johansen <john.johansen@...onical.com>
To: Thorsten Blum <thorsten.blum@...ux.dev>, Paul Moore
 <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
 "Serge E. Hallyn" <serge@...lyn.com>
Cc: apparmor@...ts.ubuntu.com, linux-security-module@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND] apparmor: Replace deprecated strcpy with memcpy in
 gen_symlink_name

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@...ux.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@...onical.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);