linux-kernel - Re: [PATCH v2] soundwire: debugfs: move debug statement outside of error handling

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aF2Em76RGWVpJfg-@vaman>
Date: Thu, 26 Jun 2025 10:34:19 -0700
From: Vinod Koul <vkoul@...nel.org>
To: Rodrigo Gobbi <rodrigo.gobbi.7@...il.com>
Cc: yung-chuan.liao@...ux.intel.com, pierre-louis.bossart@...ux.dev,
	sanyog.r.kale@...el.com, ~lkcamp/patches@...ts.sr.ht,
	linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] soundwire: debugfs: move debug statement outside of
 error handling

On 21-06-25, 16:09, Rodrigo Gobbi wrote:
> The start_t and finish_t variables are not properly initialized
> if errors happens over request_firmware actions.
> This was also detected by smatch:
> 
> drivers/soundwire/debugfs.c:301 cmd_go() error: uninitialized symbol 'finish_t'.
> drivers/soundwire/debugfs.c:301 cmd_go() error: uninitialized symbol 'start_t'.
> 
> Initialize those variables and move the debug statement outside of
> firmware error handling.
> 
> Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@...il.com>
> Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> Closes: https://lore.kernel.org/linux-sound/0db6d0bf-7bac-43a7-b624-a00d3d2bf829@stanley.mountain/
> Fixes: bb5cb09eedce ("soundwire: debugfs: add interface for BPT/BRA transfers")
> ---
> @Vinod Koul, tks for the suggestion. I`ve kept the ktime_t initialization, after moving
> the debug statement, they are not actually needed, but I think there is no harm to keep that.
> Tks and regards.

Thanks, but I guess lets avoid superfluous initialization. we should
drop this as well

> 
> Changelog:
> v2: moving debug statement
> v1: https://lore.kernel.org/linux-sound/20250617015230.54981-1-rodrigo.gobbi.7@gmail.com/#t
> ---
>  drivers/soundwire/debugfs.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c
> index 3099ea074f10..42d2aba285f5 100644
> --- a/drivers/soundwire/debugfs.c
> +++ b/drivers/soundwire/debugfs.c
> @@ -237,8 +237,8 @@ static int cmd_go(void *data, u64 value)
>  {
>  	const struct firmware *fw = NULL;
>  	struct sdw_slave *slave = data;
> -	ktime_t start_t;
> -	ktime_t finish_t;
> +	ktime_t start_t = 0;
> +	ktime_t finish_t = 0;
>  	int ret;
>  
>  	if (value != 1)
> @@ -291,6 +291,9 @@ static int cmd_go(void *data, u64 value)
>  
>  	finish_t = ktime_get();
>  
> +	dev_dbg(&slave->dev, "command completed, num_byte %zu status %d, time %lld ms\n",
> +		num_bytes, ret, div_u64(finish_t - start_t, NSEC_PER_MSEC));
> +
>  out:
>  	if (fw)
>  		release_firmware(fw);
> @@ -298,9 +301,6 @@ static int cmd_go(void *data, u64 value)
>  	pm_runtime_mark_last_busy(&slave->dev);
>  	pm_runtime_put(&slave->dev);
>  
> -	dev_dbg(&slave->dev, "command completed, num_byte %zu status %d, time %lld ms\n",
> -		num_bytes, ret, div_u64(finish_t - start_t, NSEC_PER_MSEC));
> -
>  	return ret;
>  }
>  DEFINE_DEBUGFS_ATTRIBUTE(cmd_go_fops, NULL,
> -- 
> 2.49.0

-- 
~Vinod