linux-kernel - [PATCH v3] driver core: Fix size calculation of symlink name for devlink_(add|remove)

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: <20240712-devlink_fix-v3-1-fa1c5172ffc7@quicinc.com>
Date: Fri, 12 Jul 2024 19:52:31 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
 "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Saravana Kannan <saravanak@...gle.com>, linux-kernel@...r.kernel.org, 
 Zijun Hu <zijun_hu@...oud.com>, Zijun Hu <quic_zijuhu@...cinc.com>
Subject: [PATCH v3] driver core: Fix size calculation of symlink name for
 devlink_(add|remove)_symlinks()

From: Zijun Hu <quic_zijuhu@...cinc.com>

devlink_(add|remove)_symlinks() kzalloc() memory to save symlink name
for both supplier and consumer, but do not explicitly take into account
consumer's prefix "consumer:", so cause disadvantages listed below:
1) it seems wrong for the algorithm to calculate memory size
2) readers maybe need to count characters one by one of both prefix
   strings to confirm calculated memory size
3) it is relatively easy to introduce new bug if either prefix string
   is modified in future
solved by taking into account consumer's prefix as well.

Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
---
Changes in v3:
- Correct commit message
- Link to v2: https://lore.kernel.org/r/20240709-devlink_fix-v2-1-22254f1138e1@quicinc.com

Changes in v2:
- Correct commit message and add inline comments
- Remove fix tag
- Link to v1: https://lore.kernel.org/r/20240707-devlink_fix-v1-1-623acb431cd8@quicinc.com
---
 drivers/base/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2b4c0624b704..51209db7ff84 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -572,7 +572,11 @@ static int devlink_add_symlinks(struct device *dev)
 	len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)),
 		  strlen(dev_bus_name(con)) + strlen(dev_name(con)));
 	len += strlen(":");
-	len += strlen("supplier:") + 1;
+	/*
+	 * we kzalloc() memory for symlink name of both supplier and
+	 * consumer, so explicitly take into account both prefix.
+	 */
+	len += max(strlen("supplier:"), strlen("consumer:")) + 1;
 	buf = kzalloc(len, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
@@ -623,7 +627,7 @@ static void devlink_remove_symlinks(struct device *dev)
 	len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)),
 		  strlen(dev_bus_name(con)) + strlen(dev_name(con)));
 	len += strlen(":");
-	len += strlen("supplier:") + 1;
+	len += max(strlen("supplier:"), strlen("consumer:")) + 1;
 	buf = kzalloc(len, GFP_KERNEL);
 	if (!buf) {
 		WARN(1, "Unable to properly free device link symlinks!\n");

---
base-commit: c6653f49e4fd3b0d52c12a1fc814d6c5b234ea15
change-id: 20240707-devlink_fix-0fa46dedfe95

Best regards,
-- 
Zijun Hu <quic_zijuhu@...cinc.com>