linux-kernel - [PATCH] coresight: cti: move fwnode_handle

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

Message-Id: <20240612-coresight-cti-platform-handle-put-v1-1-6817c5854e2b@gmail.com>
Date: Wed, 12 Jun 2024 15:11:01 +0200
From: Javier Carrasco <javier.carrasco.cruz@...il.com>
To: Suzuki K Poulose <suzuki.poulose@....com>, 
 Mike Leach <mike.leach@...aro.org>, James Clark <james.clark@....com>, 
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org, 
 linux-kernel@...r.kernel.org, 
 Javier Carrasco <javier.carrasco.cruz@...il.com>
Subject: [PATCH] coresight: cti: move fwnode_handle_put to the early break

The explicit call to fwnode_handle_put() is only required if the
fwnode_for_each_child_node() loop contains early exits. On the other
hand, it is not required otherwise, and its usage is then unnecessary.
The current approach is not buggy (NULL pointer is ignored), but can be
optimized by limiting the calls to fwnode_handle_put() to the case where
it is really necessary.

Move fwnode_handle_put() to the early break within the loop to avoid
calling the function when it is not required.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@...il.com>
---
Note that this issue was already mentioned during the patch review [1],
but the outcome was that fwnode_handle_put() is required for early
exits. Apparently, that was the end of the discussion, but the call to
the function that decrements the refcount is unnecessary if the end of
the loop is reached without any break/goto/return.

This issue was found while analyzing the code, and I don't have the
hardware to validate it beyond compilation and static analysis. Any
tests to catch regressions with real hardware are always welcome.

Link: https://lore.kernel.org/all/20191119231912.12768-8-mike.leach@linaro.org/ [1]
---
 drivers/hwtracing/coresight/coresight-cti-platform.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c
index ccef04f27f12..5aec37be1177 100644
--- a/drivers/hwtracing/coresight/coresight-cti-platform.c
+++ b/drivers/hwtracing/coresight/coresight-cti-platform.c
@@ -426,10 +426,11 @@ static int cti_plat_create_impdef_connections(struct device *dev,
 		if (cti_plat_node_name_eq(child, CTI_DT_CONNS))
 			rc = cti_plat_create_connection(dev, drvdata,
 							child);
-		if (rc != 0)
+		if (rc != 0) {
+			fwnode_handle_put(child);
 			break;
+		}
 	}
-	fwnode_handle_put(child);
 
 	return rc;
 }

---
base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670
change-id: 20240612-coresight-cti-platform-handle-put-d4d962762383

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@...il.com>