|
 |
168450 |
From 5075b961a29ff9c418e1fefe78432e95dd0a5fcc Mon Sep 17 00:00:00 2001
|
|
|
168450 |
From: Michal Schmidt <mschmidt@redhat.com>
|
|
|
168450 |
Date: Wed, 1 Feb 2023 22:41:06 +0100
|
|
|
168450 |
Subject: [PATCH 1/3] util: fix overflow in remap_node_name()
|
|
|
168450 |
|
|
|
168450 |
The function remap_node_name() assumes the parameter 'nodedesc' is at
|
|
|
168450 |
least IB_SMP_DATA_SIZE + 1 (i.e. 65) bytes long, because it passes it to
|
|
|
168450 |
clean_nodedesc() that writes a nul-terminator to it at offset
|
|
|
168450 |
IB_SMP_DATA_SIZE. Callers in infiniband-diags/saquery.c pass
|
|
|
168450 |
a (struct ib_node_desc_t).description as the argument, which is only
|
|
|
168450 |
IB_NODE_DESCRIPTION_SIZE (i.e. 64) bytes long. This is an overflow.
|
|
|
168450 |
|
|
|
168450 |
An odd thing about remap_node_name() is that it may (but does not
|
|
|
168450 |
always) rewrite the nodedesc in-place. Callers do not appear to
|
|
|
168450 |
appreciate this behavior. Most of them are various print_* and dump_*
|
|
|
168450 |
functions where rewriting the input makes no sense. Some callers make a
|
|
|
168450 |
local copy of the nodedesc first, possibly to protect the original.
|
|
|
168450 |
One caller (infiniband-diags/saquery.c:print_node_records()) checks if
|
|
|
168450 |
either the original description or the remapped one matches a given
|
|
|
168450 |
requested_name - so it looks like it prefers the original to be
|
|
|
168450 |
not rewritten.
|
|
|
168450 |
|
|
|
168450 |
Let's make remap_node_name() a bit safer and more convenient to use.
|
|
|
168450 |
Allocate a fixed-sized copy first. Then use strncpy to copy from
|
|
|
168450 |
'nodedesc', never reading more than IB_SMP_DATA_SIZE (64) bytes.
|
|
|
168450 |
Apply clean_nodedesc() on the correctly-sized copy. This solves the
|
|
|
168450 |
overflow bug. Also, the in-place rewrite of 'nodedesc' is gone and it
|
|
|
168450 |
can become a (const char*).
|
|
|
168450 |
|
|
|
168450 |
The overflow was found by a static checker (covscan).
|
|
|
168450 |
|
|
|
168450 |
Fixes: d974c4e398d2 ("Fix max length of node description (ibnetdiscover and smpquery)")
|
|
|
168450 |
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
|
|
|
168450 |
---
|
|
|
168450 |
util/node_name_map.c | 12 +++++++++---
|
|
|
168450 |
util/node_name_map.h | 3 +--
|
|
|
168450 |
2 files changed, 10 insertions(+), 5 deletions(-)
|
|
|
168450 |
|
|
|
168450 |
diff --git a/util/node_name_map.c b/util/node_name_map.c
|
|
|
168450 |
index 30b73eb1448e..511cb92ef19c 100644
|
|
|
168450 |
--- a/util/node_name_map.c
|
|
|
168450 |
+++ b/util/node_name_map.c
|
|
|
168450 |
@@ -95,7 +95,7 @@ void close_node_name_map(nn_map_t * map)
|
|
|
168450 |
free(map);
|
|
|
168450 |
}
|
|
|
168450 |
|
|
|
168450 |
-char *remap_node_name(nn_map_t * map, uint64_t target_guid, char *nodedesc)
|
|
|
168450 |
+char *remap_node_name(nn_map_t * map, uint64_t target_guid, const char *nodedesc)
|
|
|
168450 |
{
|
|
|
168450 |
char *rc = NULL;
|
|
|
168450 |
name_map_item_t *item = NULL;
|
|
|
168450 |
@@ -108,8 +108,14 @@ char *remap_node_name(nn_map_t * map, uint64_t target_guid, char *nodedesc)
|
|
|
168450 |
rc = strdup(item->name);
|
|
|
168450 |
|
|
|
168450 |
done:
|
|
|
168450 |
- if (rc == NULL)
|
|
|
168450 |
- rc = strdup(clean_nodedesc(nodedesc));
|
|
|
168450 |
+ if (rc == NULL) {
|
|
|
168450 |
+ rc = malloc(IB_SMP_DATA_SIZE + 1);
|
|
|
168450 |
+ if (rc) {
|
|
|
168450 |
+ strncpy(rc, nodedesc, IB_SMP_DATA_SIZE);
|
|
|
168450 |
+ rc[IB_SMP_DATA_SIZE] = '\0';
|
|
|
168450 |
+ clean_nodedesc(rc);
|
|
|
168450 |
+ }
|
|
|
168450 |
+ }
|
|
|
168450 |
return (rc);
|
|
|
168450 |
}
|
|
|
168450 |
|
|
|
168450 |
diff --git a/util/node_name_map.h b/util/node_name_map.h
|
|
|
168450 |
index e78d274b116e..d83d672782c4 100644
|
|
|
168450 |
--- a/util/node_name_map.h
|
|
|
168450 |
+++ b/util/node_name_map.h
|
|
|
168450 |
@@ -12,8 +12,7 @@ typedef struct nn_map nn_map_t;
|
|
|
168450 |
|
|
|
168450 |
nn_map_t *open_node_name_map(const char *node_name_map);
|
|
|
168450 |
void close_node_name_map(nn_map_t *map);
|
|
|
168450 |
-/* NOTE: parameter "nodedesc" may be modified here. */
|
|
|
168450 |
-char *remap_node_name(nn_map_t *map, uint64_t target_guid, char *nodedesc);
|
|
|
168450 |
+char *remap_node_name(nn_map_t *map, uint64_t target_guid, const char *nodedesc);
|
|
|
168450 |
char *clean_nodedesc(char *nodedesc);
|
|
|
168450 |
|
|
|
168450 |
#endif
|
|
|
168450 |
--
|
|
|
168450 |
2.39.1
|
|
|
168450 |
|