[PATCH] tools: hv: Zero unused bytes in KVP pool records

[PATCH] tools: hv: Zero unused bytes in KVP pool records

[Shradha Gupta]

From: Shradha Gupta <shradhagupta@microsoft.com>

KVP pool files store fixed-size key and value arrays. New records copy only
the supplied key and value lengths into uninitialized storage, but the
daemon writes each record in full. This can persist uninitialized heap
bytes in the unused array tails. Shorter value replacements and
string-based deletion compaction can also leave stale data in those tails.

Zero new records before populating them, clear values before replacement,
and copy complete records when compacting a pool after deletion.

Also clear unused tails when loading existing records so GET and
enumeration do not return legacy residual bytes. These sanitized records
reach disk on the next successful update to that pool; loading alone does
not rewrite the file.

The record layout and logical key/value strings are unchanged.

Fixes: db425334e5bb ("Tools: hv: Fully support the new KVP verbs in the user level daemon")
Signed-off-by: Shradha Gupta <shradhagupta@microsoft.com>
---
 tools/hv/hv_kvp_daemon.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 1f64c680b..d57729f60 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -203,6 +203,7 @@ static void kvp_update_mem_state(int pool)
 {
 	FILE *filep;
 	size_t records_read = 0;
+	size_t i, len;
 	struct kvp_record *record = kvp_file_info[pool].records;
 	struct kvp_record *readp;
 	int num_blocks = kvp_file_info[pool].num_blocks;
@@ -248,6 +249,15 @@ static void kvp_update_mem_state(int pool)
 		break;
 	}
 
+	/* Discard stale bytes after strings in existing pool records. */
+	for (i = 0; i < records_read; i++) {
+		len = strnlen(record[i].key, sizeof(record[i].key));
+		memset(record[i].key + len, 0, sizeof(record[i].key) - len);
+		len = strnlen(record[i].value, sizeof(record[i].value));
+		memset(record[i].value + len, 0,
+		       sizeof(record[i].value) - len);
+	}
+
 	kvp_file_info[pool].num_blocks = num_blocks;
 	kvp_file_info[pool].records = record;
 	kvp_file_info[pool].num_records = records_read;
@@ -327,8 +337,7 @@ static int kvp_key_delete(int pool, const __u8 *key, int key_size)
 		j = i;
 		k = j + 1;
 		for (; k < num_records; k++) {
-			strcpy(record[j].key, record[k].key);
-			strcpy(record[j].value, record[k].value);
+			memcpy(&record[j], &record[k], sizeof(record[j]));
 			j++;
 		}
 
@@ -383,6 +392,7 @@ static int kvp_key_add_or_modify(int pool, const __u8 *key, int key_size,
 		 * Found a match; just update the value -
 		 * this is the modify case.
 		 */
+		memset(record[i].value, 0, sizeof(record[i].value));
 		memcpy(record[i].value, value, value_size);
 		kvp_update_file(pool);
 		if (debug)
@@ -406,6 +416,7 @@ static int kvp_key_add_or_modify(int pool, const __u8 *key, int key_size,
 		kvp_file_info[pool].num_blocks++;
 
 	}
+	memset(&record[i], 0, sizeof(record[i]));
 	memcpy(record[i].value, value, value_size);
 	memcpy(record[i].key, key, key_size);
 	kvp_file_info[pool].records = record;

base-commit: 20a33c6b6cbe7e4805dfb6cd68816937443aa148
-- 
2.43.0