2017-11-01 18:47:51

by Long Li

[permalink] [raw]
Subject: [Revised PATCH v2] hv: kvp: Avoid reading past allocated blocks from KVP file

From: Paul Meyer <[email protected]>

While reading in more than one block (50) of KVP records, the allocation
goes per block, but the reads used the total number of allocated records
(without resetting the pointer/stream). This causes the records buffer to
overrun when the refresh reads more than one block over the previous
capacity (e.g. reading more than 100 KVP records whereas the in-memory
database was empty before).

Fix this by reading the correct number of KVP records from file each time.

Changes since v1:
1. Properly wrapped comment texts.
2. Added the 2nd Signed-off-by.

Signed-off-by: Paul Meyer <[email protected]>
Signed-off-by: Long Li <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 66 ++++++++----------------------------------------
1 file changed, 10 insertions(+), 56 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index eaa3bec..2094036 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -193,11 +193,13 @@ static void kvp_update_mem_state(int pool)
for (;;) {
readp = &record[records_read];
records_read += fread(readp, sizeof(struct kvp_record),
- ENTRIES_PER_BLOCK * num_blocks,
- filep);
+ ENTRIES_PER_BLOCK * num_blocks - records_read,
+ filep);

if (ferror(filep)) {
- syslog(LOG_ERR, "Failed to read file, pool: %d", pool);
+ syslog(LOG_ERR,
+ "Failed to read file, pool: %d; error: %d %s",
+ pool, errno, strerror(errno));
exit(EXIT_FAILURE);
}

@@ -224,15 +226,11 @@ static void kvp_update_mem_state(int pool)
fclose(filep);
kvp_release_lock(pool);
}
+
static int kvp_file_init(void)
{
int fd;
- FILE *filep;
- size_t records_read;
char *fname;
- struct kvp_record *record;
- struct kvp_record *readp;
- int num_blocks;
int i;
int alloc_unit = sizeof(struct kvp_record) * ENTRIES_PER_BLOCK;

@@ -246,61 +244,17 @@ static int kvp_file_init(void)

for (i = 0; i < KVP_POOL_COUNT; i++) {
fname = kvp_file_info[i].fname;
- records_read = 0;
- num_blocks = 1;
sprintf(fname, "%s/.kvp_pool_%d", KVP_CONFIG_LOC, i);
fd = open(fname, O_RDWR | O_CREAT | O_CLOEXEC, 0644 /* rw-r--r-- */);

if (fd == -1)
return 1;

-
- filep = fopen(fname, "re");
- if (!filep) {
- close(fd);
- return 1;
- }
-
- record = malloc(alloc_unit * num_blocks);
- if (record == NULL) {
- fclose(filep);
- close(fd);
- return 1;
- }
- for (;;) {
- readp = &record[records_read];
- records_read += fread(readp, sizeof(struct kvp_record),
- ENTRIES_PER_BLOCK,
- filep);
-
- if (ferror(filep)) {
- syslog(LOG_ERR, "Failed to read file, pool: %d",
- i);
- exit(EXIT_FAILURE);
- }
-
- if (!feof(filep)) {
- /*
- * We have more data to read.
- */
- num_blocks++;
- record = realloc(record, alloc_unit *
- num_blocks);
- if (record == NULL) {
- fclose(filep);
- close(fd);
- return 1;
- }
- continue;
- }
- break;
- }
kvp_file_info[i].fd = fd;
- kvp_file_info[i].num_blocks = num_blocks;
- kvp_file_info[i].records = record;
- kvp_file_info[i].num_records = records_read;
- fclose(filep);
-
+ kvp_file_info[i].num_blocks = 1;
+ kvp_file_info[i].records = malloc(alloc_unit);
+ kvp_file_info[i].num_records = 0;
+ kvp_update_mem_state(i);
}

return 0;
--
2.7.4


From 1583512123865185860@xxx Wed Nov 08 15:27:22 +0000 2017
X-GM-THRID: 1583483204746654793
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread