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

[Greg KH]

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@xxxxxxxxxx>

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