linux-kernel - Re: [RESEND PATCH] drivers: base: power: Optimize array out-of-bounds access logic

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 for Android: free password hash cracker in your pocket

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

Message-ID: <2024120206-gigolo-parish-2be3@gregkh>
Date: Mon, 2 Dec 2024 07:21:45 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: xueqin Luo <luoxueqin@...inos.cn>
Cc: rafael@...nel.org, pavel@....cz, len.brown@...el.com,
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH] drivers: base: power: Optimize array
 out-of-bounds access logic

On Mon, Dec 02, 2024 at 11:23:06AM +0800, xueqin Luo wrote:
> The code previously used snprintf to format a string into a buffer and
> manually checked for potential buffer overflows by comparing the returned
> length with the buffer size. This approach introduced unnecessary
> complexity and was prone to subtle errors.

What errors are in the original code here?  Is it incorrect?

> Replaced snprintf with scnprintf, which directly returns the actual number
> of characters written to the buffer (excluding the null terminator). This
> change eliminates the need for manual overflow checks and simplifies the
> buffer offset and size adjustment logic.

Your lines should be wrapped at 72 columns, right?

> 
> Signed-off-by: xueqin Luo <luoxueqin@...inos.cn>

Why is this a resend?  What was wrong with the first version?

> ---
>  drivers/base/power/trace.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/base/power/trace.c b/drivers/base/power/trace.c
> index cd6e559648b2..d8da7195bb00 100644
> --- a/drivers/base/power/trace.c
> +++ b/drivers/base/power/trace.c
> @@ -238,10 +238,8 @@ int show_trace_dev_match(char *buf, size_t size)
>  		unsigned int hash = hash_string(DEVSEED, dev_name(dev),
>  						DEVHASH);
>  		if (hash == value) {
> -			int len = snprintf(buf, size, "%s\n",
> +			int len = scnprintf(buf, size, "%s\n",
>  					    dev_driver_string(dev));
> -			if (len > size)
> -				len = size;

How was this tested?  I think if code could just be cleaned up
automatically like this, it would have already, right?

thanks,

greg k-h