linux-kernel - Re: [PATCH] cxl: core-pmu: Fix the usage of uninitialized variable

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZxjreAV-re-rfJsv@rric.localdomain>
Date: Wed, 23 Oct 2024 14:26:32 +0200
From: Robert Richter <rrichter@....com>
To: Zicheng Qu <quzicheng@...wei.com>
Cc: dave@...olabs.net, jonathan.cameron@...wei.com, dave.jiang@...el.com,
	alison.schofield@...el.com, vishal.l.verma@...el.com,
	ira.weiny@...el.com, dan.j.williams@...el.com,
	linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
	tanghui20@...wei.com, zhangqiao22@...wei.com,
	judy.chenhui@...wei.com
Subject: Re: [PATCH] cxl: core-pmu: Fix the usage of uninitialized variable

On 23.10.24 10:56:10, Zicheng Qu wrote:
> In the devm_cxl_pmu_add() function, the variable rc might be
> uninitialized before its first use 'if (rc) goto err', leading
> to undefined behavior since its value depends on the compiler.
> Currently, the switch statement is limited to the CXL_PMU_MEMDEV type.
> If additional types are introduced, it could lead to similar concerns.
> If the type range remains unchanged, using a switch case is unnecessary.
> To enhance code extensibility and stability, it is recommended to
> address this potential aspect.
> 
> Cc: stable@...r.kernel.org # v6.6+
> Fixes: 1ad3f701c399 ("cxl/pci: Find and register CXL PMU devices")
> Signed-off-by: Zicheng Qu <quzicheng@...wei.com>
> ---
>  drivers/cxl/core/pmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/cxl/core/pmu.c b/drivers/cxl/core/pmu.c
> index 5d8e06b0ba6e..2d12887c9915 100644
> --- a/drivers/cxl/core/pmu.c
> +++ b/drivers/cxl/core/pmu.c
> @@ -51,6 +51,9 @@ int devm_cxl_pmu_add(struct device *parent, struct cxl_pmu_regs *regs,
>  	case CXL_PMU_MEMDEV:
>  		rc = dev_set_name(dev, "pmu_mem%d.%d", assoc_id, index);
>  		break;
> +	default:
> +		rc = -EINVAL;
> +		break;

You might consider to use the -ENOENT error code and/or to
preinitialize rc with the error code. The default case can be dropped
then.

However, all those variants look ok:

Reviewed-by: Robert Richter <rrichter@....com>

>  	}
>  	if (rc)
>  		goto err;
> -- 
> 2.34.1
>