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

On Jul 14, 2026 Zhan Xusheng <[email protected]> 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: [email protected]
> Signed-off-by: Zhan Xusheng <[email protected]>
> ---
>  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