[PATCH] audit: fix potential integer overflow in audit_log_n_string()

[PATCH] audit: fix potential integer overflow in audit_log_n_string()

[Zhan Xusheng]

audit_log_n_string() computes new_len as "slen + 3" (enclosing quotes
plus the NUL terminator) and stores it into an int, while slen is a
size_t.  For a sufficiently large slen the addition can overflow and/or
the result be truncated when assigned to the int new_len, so the
"new_len > avail" check can be bypassed and the subsequent
memcpy(ptr, string, slen) can write past the skb tail.

This is the same class of bug that was fixed for the hex sibling in
commit 65dfde57d1e2 ("audit: fix potential integer overflow in
audit_log_n_hex()"); both helpers are reached through
audit_log_n_untrustedstring() with the same length source.

Make new_len a size_t and use check_add_overflow() to catch the
overflow, mirroring the audit_log_n_hex() fix.  No functional change for
the in-tree callers, which all pass bounded lengths.

Fixes: 168b7173959f ("AUDIT: Clean up logging of untrusted strings")
Cc: stable@vger.kernel.org
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/audit.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 562476937fa7..547ae0cebec9 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2120,7 +2120,8 @@ void audit_log_n_hex(struct audit_buffer *ab, const unsigned char *buf,
 void audit_log_n_string(struct audit_buffer *ab, const char *string,
 			size_t slen)
 {
-	int avail, new_len;
+	int avail;
+	size_t new_len;
 	unsigned char *ptr;
 	struct sk_buff *skb;
 
@@ -2130,7 +2131,13 @@ void audit_log_n_string(struct audit_buffer *ab, const char *string,
 	BUG_ON(!ab->skb);
 	skb = ab->skb;
 	avail = skb_tailroom(skb);
-	new_len = slen + 3;	/* enclosing quotes + null terminator */
+
+	/* enclosing quotes + null terminator */
+	if (check_add_overflow(slen, (size_t)3, &new_len)) {
+		audit_log_format(ab, "\"?\"");
+		return;
+	}
+
 	if (new_len > avail) {
 		avail = audit_expand(ab, new_len);
 		if (!avail)
-- 
2.43.0

Re: [PATCH] audit: fix potential integer overflow in audit_log_n_string()

[Paul Moore]

On Jul 14, 2026 Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:
> 
> audit_log_n_string() computes new_len as "slen + 3" (enclosing quotes
> plus the NUL terminator) and stores it into an int, while slen is a
> size_t.  For a sufficiently large slen the addition can overflow and/or
> the result be truncated when assigned to the int new_len, so the
> "new_len > avail" check can be bypassed and the subsequent
> memcpy(ptr, string, slen) can write past the skb tail.
> 
> This is the same class of bug that was fixed for the hex sibling in
> commit 65dfde57d1e2 ("audit: fix potential integer overflow in
> audit_log_n_hex()"); both helpers are reached through
> audit_log_n_untrustedstring() with the same length source.
> 
> Make new_len a size_t and use check_add_overflow() to catch the
> overflow, mirroring the audit_log_n_hex() fix.  No functional change for
> the in-tree callers, which all pass bounded lengths.
> 
> Fixes: 168b7173959f ("AUDIT: Clean up logging of untrusted strings")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  kernel/audit.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 562476937fa7..547ae0cebec9 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2120,7 +2120,8 @@ void audit_log_n_hex(struct audit_buffer *ab, const unsigned char *buf,
>  void audit_log_n_string(struct audit_buffer *ab, const char *string,
>  			size_t slen)
>  {
> -	int avail, new_len;
> +	int avail;
> +	size_t new_len;
>  	unsigned char *ptr;
>  	struct sk_buff *skb;
>  
> @@ -2130,7 +2131,13 @@ void audit_log_n_string(struct audit_buffer *ab, const char *string,
>  	BUG_ON(!ab->skb);
>  	skb = ab->skb;
>  	avail = skb_tailroom(skb);
> -	new_len = slen + 3;	/* enclosing quotes + null terminator */
> +
> +	/* enclosing quotes + null terminator */
> +	if (check_add_overflow(slen, (size_t)3, &new_len)) {

You shouldn't need the size_t cast for the '3' constant.

> +		audit_log_format(ab, "\"?\"");

There is no need for the additional quotes, see the related
audit_log_n_hex() fix you mentioned in the commit description:

 audit_log_format(ab, "?");

> +		return;
> +	}
> +
>  	if (new_len > avail) {
>  		avail = audit_expand(ab, new_len);
>  		if (!avail)
> -- 
> 2.43.0

--
paul-moore.com