From 43768781c821683f4824f5b5a198774abfef5127 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Nov 2025 16:26:12 +0000 Subject: [PATCH] Fix valgrind uninitialized memory errors Fixed multiple uninitialized memory issues detected by valgrind: 1. SHA256 struct size mismatch: Changed from 64 bytes to 32 bytes to match the actual output of sha256() function (which produces 256 bits = 32 bytes). 2. File tree chunk allocation: Fixed loop that wasn't initializing newly allocated chunks because num_chunks was updated before the initialization loop. Now saves old_num_chunks first. 3. Metadata server process_client_write: Initialized results[i].num_addrs to 0 before use to prevent reading uninitialized value. 4. Address struct initialization: Zero-initialize Address structs before setting fields to ensure the entire union is initialized (union contains either IPv4 or IPv6, leaving unused bytes uninitialized). 5. Fixed bug in IPv6 address creation: Changed incorrect is_ipv4=true to is_ipv4=false for IPv6 addresses. 6. Fixed all_chunk_servers_holding_chunk: Function was incrementing counter for all chunk servers instead of only those containing the hash, causing access to uninitialized array elements. All valgrind errors resolved: ERROR SUMMARY: 0 errors from 0 contexts --- src/basic.h | 2 +- src/file_tree.c | 5 +++-- src/metadata_server.c | 31 +++++++++++++++++++++---------- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/basic.h b/src/basic.h index 232df36..c5df9d5 100644 --- a/src/basic.h +++ b/src/basic.h @@ -5,7 +5,7 @@ #include typedef struct { - char data[64]; + char data[32]; } SHA256; typedef struct { diff --git a/src/file_tree.c b/src/file_tree.c index 066b1a2..8a51ff6 100644 --- a/src/file_tree.c +++ b/src/file_tree.c @@ -341,17 +341,18 @@ int file_tree_write(FileTree *ft, string path, uint64_t last_chunk_index = (off + len - 1) / f->chunk_size; if (last_chunk_index >= f->num_chunks) { + uint64_t old_num_chunks = f->num_chunks; SHA256 *new_chunks = sys_malloc((last_chunk_index+1) * sizeof(SHA256)); if (new_chunks == NULL) return FILETREE_NOMEM; if (f->chunks) { if (f->num_chunks > 0) - memcpy(new_chunks, f->chunks, f->num_chunks); + memcpy(new_chunks, f->chunks, f->num_chunks * sizeof(SHA256)); sys_free(f->chunks); } f->chunks = new_chunks; f->num_chunks = last_chunk_index+1; - for (uint64_t i = f->num_chunks; i < last_chunk_index+1; i++) + for (uint64_t i = old_num_chunks; i < last_chunk_index+1; i++) memset(&f->chunks[i], 0, sizeof(SHA256)); } diff --git a/src/metadata_server.c b/src/metadata_server.c index f9b0303..5db4978 100644 --- a/src/metadata_server.c +++ b/src/metadata_server.c @@ -92,9 +92,11 @@ all_chunk_servers_holding_chunk(MetadataServer *state, SHA256 hash, int *out, in { int num = 0; for (int i = 0; i < state->num_chunk_servers; i++) { - if (num < max && chunk_server_peer_contains(&state->chunk_servers[i], hash)) - out[num] = i; - num++; + if (chunk_server_peer_contains(&state->chunk_servers[i], hash)) { + if (num < max) + out[num] = i; + num++; + } } return num; } @@ -579,6 +581,7 @@ process_client_write(MetadataServer *state, int conn_idx, ByteView msg) results[i].old_hash = old_hash; results[i].new_hash = new_hash; + results[i].num_addrs = 0; uint32_t num_locations; if (!binary_read(&reader, &num_locations, sizeof(num_locations))) @@ -590,7 +593,7 @@ process_client_write(MetadataServer *state, int conn_idx, ByteView msg) if (!binary_read(&reader, &is_ipv4, sizeof(is_ipv4))) return -1; - Address addr; + Address addr = {0}; addr.is_ipv4 = is_ipv4; if (is_ipv4) { @@ -736,9 +739,13 @@ static int process_chunk_server_auth(MetadataServer *state, if (!binary_read(&reader, &port, sizeof(port))) return -1; - if (chunk_server->num_addrs < MAX_SERVER_ADDRS) - chunk_server->addrs[chunk_server->num_addrs++] = - (Address) { .ipv4=ipv4, .is_ipv4=true, .port=port }; + if (chunk_server->num_addrs < MAX_SERVER_ADDRS) { + Address addr = {0}; + addr.ipv4 = ipv4; + addr.is_ipv4 = true; + addr.port = port; + chunk_server->addrs[chunk_server->num_addrs++] = addr; + } } } @@ -758,9 +765,13 @@ static int process_chunk_server_auth(MetadataServer *state, if (!binary_read(&reader, &port, sizeof(port))) return -1; - if (chunk_server->num_addrs < MAX_SERVER_ADDRS) - chunk_server->addrs[chunk_server->num_addrs++] = - (Address) { .is_ipv4=true, .ipv6=ipv6, .port=port }; + if (chunk_server->num_addrs < MAX_SERVER_ADDRS) { + Address addr = {0}; + addr.ipv6 = ipv6; + addr.is_ipv4 = false; + addr.port = port; + chunk_server->addrs[chunk_server->num_addrs++] = addr; + } } }