linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ] mesh: Fix keyring snprintf usage range checking
@ 2022-06-14 17:13 Brian Gix
  2022-06-14 18:23 ` [BlueZ] " bluez.test.bot
  2022-06-14 20:50 ` [PATCH BlueZ] " patchwork-bot+bluetooth
  0 siblings, 2 replies; 3+ messages in thread
From: Brian Gix @ 2022-06-14 17:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, luiz.dentz, brian.gix, tedd.an

snprintf performs it's own range checking and returns a negative value
if string construction fails. Not checking the return value throws a
warning at compile time on GCC 12 and later. This patch removes
redundent range chacking and checks all snprintf return values.
---
 mesh/keyring.c | 68 +++++++++++++++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/mesh/keyring.c b/mesh/keyring.c
index 6d539e74e..995a4b88f 100644
--- a/mesh/keyring.c
+++ b/mesh/keyring.c
@@ -39,26 +39,24 @@ static int open_key_file(struct mesh_node *node, const char *key_dir,
 {
 	const char *node_path;
 	char fname[PATH_MAX];
-	int len;
+	int ret;
 
 	if (!node)
 		return -1;
 
 	node_path = node_get_storage_dir(node);
 
-	if (strlen(node_path) + strlen(key_dir) + 1 + 3 >= PATH_MAX)
-		return -1;
-
 	if (flags & O_CREAT) {
-		len = snprintf(fname, PATH_MAX, "%s%s", node_path, key_dir);
-		if (len >= PATH_MAX)
+		ret = snprintf(fname, PATH_MAX, "%s%s", node_path, key_dir);
+		if (ret < 0)
 			return -1;
+
 		if (mkdir(fname, 0755) != 0 && errno != EEXIST)
 			l_error("Failed to create dir(%d): %s", errno, fname);
 	}
 
-	len = snprintf(fname, PATH_MAX, "%s%s/%3.3x", node_path, key_dir, idx);
-	if (len >= PATH_MAX)
+	ret = snprintf(fname, PATH_MAX, "%s%s/%3.3x", node_path, key_dir, idx);
+	if (ret < 0)
 		return -1;
 
 	if (flags & O_CREAT)
@@ -157,7 +155,7 @@ bool keyring_finalize_app_keys(struct mesh_node *node, uint16_t net_idx)
 	const char *node_path;
 	char key_dir[PATH_MAX];
 	DIR *dir;
-	int dir_fd;
+	int ret, dir_fd;
 	struct dirent *entry;
 
 	if (!node)
@@ -165,10 +163,10 @@ bool keyring_finalize_app_keys(struct mesh_node *node, uint16_t net_idx)
 
 	node_path = node_get_storage_dir(node);
 
-	if (strlen(node_path) + strlen(app_key_dir) + 1 >= PATH_MAX)
+	ret = snprintf(key_dir, PATH_MAX, "%s%s", node_path, app_key_dir);
+	if (ret < 0)
 		return false;
 
-	snprintf(key_dir, PATH_MAX, "%s%s", node_path, app_key_dir);
 	dir = opendir(key_dir);
 	if (!dir) {
 		if (errno == ENOENT)
@@ -197,7 +195,7 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 	const char *node_path;
 	char key_file[PATH_MAX];
 	bool result = true;
-	int fd, i;
+	int ret, fd, i;
 
 	if (!IS_UNICAST_RANGE(unicast, count))
 		return false;
@@ -207,17 +205,19 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 
 	node_path = node_get_storage_dir(node);
 
-	if (strlen(node_path) + strlen(dev_key_dir) + 1 + 4 >= PATH_MAX)
+	ret = snprintf(key_file, PATH_MAX, "%s%s", node_path, dev_key_dir);
+	if (ret < 0)
 		return false;
 
-	snprintf(key_file, PATH_MAX, "%s%s", node_path, dev_key_dir);
-
 	if (mkdir(key_file, 0755) != 0 && errno != EEXIST)
 		l_error("Failed to create dir(%d): %s", errno, key_file);
 
 	for (i = 0; i < count; i++) {
-		snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
+		ret = snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
 						dev_key_dir, unicast + i);
+		if (ret < 0)
+			return false;
+
 		l_debug("Put Dev Key %s", key_file);
 
 		fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC, 0600);
@@ -272,7 +272,7 @@ bool keyring_get_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 	const char *node_path;
 	char key_file[PATH_MAX];
 	bool result = false;
-	int fd;
+	int ret, fd;
 
 	if (!IS_UNICAST(unicast))
 		return false;
@@ -282,8 +282,11 @@ bool keyring_get_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 
 	node_path = node_get_storage_dir(node);
 
-	snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path, dev_key_dir,
+	ret = snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path, dev_key_dir,
 								unicast);
+	if (ret < 0)
+		return false;
+
 	fd = open(key_file, O_RDONLY);
 	if (fd >= 0) {
 		if (read(fd, dev_key, 16) == 16)
@@ -299,13 +302,17 @@ bool keyring_del_net_key(struct mesh_node *node, uint16_t net_idx)
 {
 	const char *node_path;
 	char key_file[PATH_MAX];
+	int ret;
 
 	if (!node)
 		return false;
 
 	node_path = node_get_storage_dir(node);
-	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,
+	ret = snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,
 								net_idx);
+	if (ret < 0)
+		return false;
+
 	l_debug("RM Net Key %s", key_file);
 	remove(key_file);
 
@@ -319,13 +326,17 @@ bool keyring_del_app_key(struct mesh_node *node, uint16_t app_idx)
 {
 	const char *node_path;
 	char key_file[PATH_MAX];
+	int ret;
 
 	if (!node)
 		return false;
 
 	node_path = node_get_storage_dir(node);
-	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,
+	ret = snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,
 								app_idx);
+	if (ret < 0)
+		return false;
+
 	l_debug("RM App Key %s", key_file);
 	remove(key_file);
 
@@ -337,7 +348,7 @@ bool keyring_del_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 {
 	const char *node_path;
 	char key_file[PATH_MAX];
-	int i;
+	int ret, i;
 
 	if (!IS_UNICAST_RANGE(unicast, count))
 		return false;
@@ -348,8 +359,11 @@ bool keyring_del_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 	node_path = node_get_storage_dir(node);
 
 	for (i = 0; i < count; i++) {
-		snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
+		ret = snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
 						dev_key_dir, unicast + i);
+		if (ret < 0)
+			return false;
+
 		l_debug("RM Dev Key %s", key_file);
 		remove(key_file);
 	}
@@ -361,18 +375,16 @@ static DIR *open_key_dir(const char *node_path, const char *key_dir_name)
 {
 	char dir_path[PATH_MAX];
 	DIR *key_dir;
+	int ret;
 
-	if (strlen(node_path) + strlen(key_dir_name) + 1 >= PATH_MAX)
+	ret = snprintf(dir_path, PATH_MAX, "%s%s", node_path, key_dir_name);
+	if (ret < 0)
 		return NULL;
 
-	snprintf(dir_path, PATH_MAX, "%s%s", node_path, key_dir_name);
-
 	key_dir = opendir(dir_path);
-	if (!key_dir) {
+	if (!key_dir)
 		l_error("Failed to open keyring storage directory: %s",
 								dir_path);
-		return NULL;
-	}
 
 	return key_dir;
 }
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* RE: [BlueZ] mesh: Fix keyring snprintf usage range checking
  2022-06-14 17:13 [PATCH BlueZ] mesh: Fix keyring snprintf usage range checking Brian Gix
@ 2022-06-14 18:23 ` bluez.test.bot
  2022-06-14 20:50 ` [PATCH BlueZ] " patchwork-bot+bluetooth
  1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2022-06-14 18:23 UTC (permalink / raw)
  To: linux-bluetooth, brian.gix

[-- Attachment #1: Type: text/plain, Size: 995 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=650293

---Test result---

Test Summary:
CheckPatch                    PASS      1.66 seconds
GitLint                       PASS      1.09 seconds
Prep - Setup ELL              PASS      41.51 seconds
Build - Prep                  PASS      0.63 seconds
Build - Configure             PASS      8.40 seconds
Build - Make                  PASS      1203.66 seconds
Make Check                    PASS      11.51 seconds
Make Check w/Valgrind         PASS      438.58 seconds
Make Distcheck                PASS      228.44 seconds
Build w/ext ELL - Configure   PASS      8.35 seconds
Build w/ext ELL - Make        PASS      1177.97 seconds
Incremental Build with patchesPASS      0.00 seconds



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH BlueZ] mesh: Fix keyring snprintf usage range checking
  2022-06-14 17:13 [PATCH BlueZ] mesh: Fix keyring snprintf usage range checking Brian Gix
  2022-06-14 18:23 ` [BlueZ] " bluez.test.bot
@ 2022-06-14 20:50 ` patchwork-bot+bluetooth
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+bluetooth @ 2022-06-14 20:50 UTC (permalink / raw)
  To: Brian Gix; +Cc: linux-bluetooth, marcel, luiz.dentz, tedd.an

Hello:

This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Tue, 14 Jun 2022 10:13:38 -0700 you wrote:
> snprintf performs it's own range checking and returns a negative value
> if string construction fails. Not checking the return value throws a
> warning at compile time on GCC 12 and later. This patch removes
> redundent range chacking and checks all snprintf return values.
> ---
>  mesh/keyring.c | 68 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 40 insertions(+), 28 deletions(-)

Here is the summary with links:
  - [BlueZ] mesh: Fix keyring snprintf usage range checking
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5cc08527c0aa

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-14 20:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-14 17:13 [PATCH BlueZ] mesh: Fix keyring snprintf usage range checking Brian Gix
2022-06-14 18:23 ` [BlueZ] " bluez.test.bot
2022-06-14 20:50 ` [PATCH BlueZ] " patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).