linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND] apparmor: replace sprintf with snprintf in aa_new_learning_profile
@ 2025-11-22 11:54 Thorsten Blum
  2025-11-27  1:34 ` John Johansen
  0 siblings, 1 reply; 2+ messages in thread
From: Thorsten Blum @ 2025-11-22 11:54 UTC (permalink / raw)
  To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
  Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel

Replace unbounded sprintf() calls with snprintf() to prevent potential
buffer overflows in aa_new_learning_profile(). While the current code
works correctly, snprintf() is safer and follows secure coding best
practices.  No functional changes.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 security/apparmor/policy.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 50d5345ff5cb..b09323867fea 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -697,24 +697,27 @@ struct aa_profile *aa_new_learning_profile(struct aa_profile *parent, bool hat,
 	struct aa_profile *p, *profile;
 	const char *bname;
 	char *name = NULL;
+	size_t name_sz;
 
 	AA_BUG(!parent);
 
 	if (base) {
-		name = kmalloc(strlen(parent->base.hname) + 8 + strlen(base),
-			       gfp);
+		name_sz = strlen(parent->base.hname) + 8 + strlen(base);
+		name = kmalloc(name_sz, gfp);
 		if (name) {
-			sprintf(name, "%s//null-%s", parent->base.hname, base);
+			snprintf(name, name_sz, "%s//null-%s",
+				 parent->base.hname, base);
 			goto name;
 		}
 		/* fall through to try shorter uniq */
 	}
 
-	name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, gfp);
+	name_sz = strlen(parent->base.hname) + 2 + 7 + 8;
+	name = kmalloc(name_sz, gfp);
 	if (!name)
 		return NULL;
-	sprintf(name, "%s//null-%x", parent->base.hname,
-		atomic_inc_return(&parent->ns->uniq_null));
+	snprintf(name, name_sz, "%s//null-%x", parent->base.hname,
+		 atomic_inc_return(&parent->ns->uniq_null));
 
 name:
 	/* lookup to see if this is a dup creation */
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH RESEND] apparmor: replace sprintf with snprintf in aa_new_learning_profile
  2025-11-22 11:54 [PATCH RESEND] apparmor: replace sprintf with snprintf in aa_new_learning_profile Thorsten Blum
@ 2025-11-27  1:34 ` John Johansen
  0 siblings, 0 replies; 2+ messages in thread
From: John Johansen @ 2025-11-27  1:34 UTC (permalink / raw)
  To: Thorsten Blum, Paul Moore, James Morris, Serge E. Hallyn
  Cc: apparmor, linux-security-module, linux-kernel

On 11/22/25 03:54, Thorsten Blum wrote:
> Replace unbounded sprintf() calls with snprintf() to prevent potential
> buffer overflows in aa_new_learning_profile(). While the current code
> works correctly, snprintf() is safer and follows secure coding best
> practices.  No functional changes.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>

I have pulled this into my tree


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

> ---
>   security/apparmor/policy.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
> index 50d5345ff5cb..b09323867fea 100644
> --- a/security/apparmor/policy.c
> +++ b/security/apparmor/policy.c
> @@ -697,24 +697,27 @@ struct aa_profile *aa_new_learning_profile(struct aa_profile *parent, bool hat,
>   	struct aa_profile *p, *profile;
>   	const char *bname;
>   	char *name = NULL;
> +	size_t name_sz;
>   
>   	AA_BUG(!parent);
>   
>   	if (base) {
> -		name = kmalloc(strlen(parent->base.hname) + 8 + strlen(base),
> -			       gfp);
> +		name_sz = strlen(parent->base.hname) + 8 + strlen(base);
> +		name = kmalloc(name_sz, gfp);
>   		if (name) {
> -			sprintf(name, "%s//null-%s", parent->base.hname, base);
> +			snprintf(name, name_sz, "%s//null-%s",
> +				 parent->base.hname, base);
>   			goto name;
>   		}
>   		/* fall through to try shorter uniq */
>   	}
>   
> -	name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, gfp);
> +	name_sz = strlen(parent->base.hname) + 2 + 7 + 8;
> +	name = kmalloc(name_sz, gfp);
>   	if (!name)
>   		return NULL;
> -	sprintf(name, "%s//null-%x", parent->base.hname,
> -		atomic_inc_return(&parent->ns->uniq_null));
> +	snprintf(name, name_sz, "%s//null-%x", parent->base.hname,
> +		 atomic_inc_return(&parent->ns->uniq_null));
>   
>   name:
>   	/* lookup to see if this is a dup creation */



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-11-27  1:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-22 11:54 [PATCH RESEND] apparmor: replace sprintf with snprintf in aa_new_learning_profile Thorsten Blum
2025-11-27  1:34 ` John Johansen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).