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
This commit is contained in:
Claude
2025-11-08 16:26:12 +00:00
parent e5039d1774
commit 43768781c8
3 changed files with 25 additions and 13 deletions
+1 -1
View File
@@ -5,7 +5,7 @@
#include <stdbool.h> #include <stdbool.h>
typedef struct { typedef struct {
char data[64]; char data[32];
} SHA256; } SHA256;
typedef struct { typedef struct {
+3 -2
View File
@@ -341,17 +341,18 @@ int file_tree_write(FileTree *ft, string path,
uint64_t last_chunk_index = (off + len - 1) / f->chunk_size; uint64_t last_chunk_index = (off + len - 1) / f->chunk_size;
if (last_chunk_index >= f->num_chunks) { 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)); SHA256 *new_chunks = sys_malloc((last_chunk_index+1) * sizeof(SHA256));
if (new_chunks == NULL) if (new_chunks == NULL)
return FILETREE_NOMEM; return FILETREE_NOMEM;
if (f->chunks) { if (f->chunks) {
if (f->num_chunks > 0) 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); sys_free(f->chunks);
} }
f->chunks = new_chunks; f->chunks = new_chunks;
f->num_chunks = last_chunk_index+1; 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)); memset(&f->chunks[i], 0, sizeof(SHA256));
} }
+19 -8
View File
@@ -92,10 +92,12 @@ all_chunk_servers_holding_chunk(MetadataServer *state, SHA256 hash, int *out, in
{ {
int num = 0; int num = 0;
for (int i = 0; i < state->num_chunk_servers; i++) { for (int i = 0; i < state->num_chunk_servers; i++) {
if (num < max && chunk_server_peer_contains(&state->chunk_servers[i], hash)) if (chunk_server_peer_contains(&state->chunk_servers[i], hash)) {
if (num < max)
out[num] = i; out[num] = i;
num++; num++;
} }
}
return 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].old_hash = old_hash;
results[i].new_hash = new_hash; results[i].new_hash = new_hash;
results[i].num_addrs = 0;
uint32_t num_locations; uint32_t num_locations;
if (!binary_read(&reader, &num_locations, sizeof(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))) if (!binary_read(&reader, &is_ipv4, sizeof(is_ipv4)))
return -1; return -1;
Address addr; Address addr = {0};
addr.is_ipv4 = is_ipv4; addr.is_ipv4 = is_ipv4;
if (is_ipv4) { if (is_ipv4) {
@@ -736,9 +739,13 @@ static int process_chunk_server_auth(MetadataServer *state,
if (!binary_read(&reader, &port, sizeof(port))) if (!binary_read(&reader, &port, sizeof(port)))
return -1; return -1;
if (chunk_server->num_addrs < MAX_SERVER_ADDRS) if (chunk_server->num_addrs < MAX_SERVER_ADDRS) {
chunk_server->addrs[chunk_server->num_addrs++] = Address addr = {0};
(Address) { .ipv4=ipv4, .is_ipv4=true, .port=port }; 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))) if (!binary_read(&reader, &port, sizeof(port)))
return -1; return -1;
if (chunk_server->num_addrs < MAX_SERVER_ADDRS) if (chunk_server->num_addrs < MAX_SERVER_ADDRS) {
chunk_server->addrs[chunk_server->num_addrs++] = Address addr = {0};
(Address) { .is_ipv4=true, .ipv6=ipv6, .port=port }; addr.ipv6 = ipv6;
addr.is_ipv4 = false;
addr.port = port;
chunk_server->addrs[chunk_server->num_addrs++] = addr;
}
} }
} }