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

[Naman Jain <namjain@linux.microsoft.com>]

On 11/2/2026 3:59 PM, Shradha Gupta wrote:
> 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

Reviewed-by: Naman Jain <namjain@linux.microsoft.com>

Regards,
Naman