Skip to content

Commit a0573a9

Browse files
mrc0mmandbluca
authored andcommitted
sd-network: avoid leaking DHCPLease
If we fail any allocation prior adding the lease to the server lease hashmap. ==2103==ERROR: LeakSanitizer: detected memory leaks Direct leak of 128 byte(s) in 2 object(s) allocated from: #0 0x4a203e in __interceptor_calloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:77:3 #1 0x4f6341 in calloc (/build/fuzz-dhcp-server+0x4f6341) #2 0x4ec818 in add_lease /work/build/../../src/systemd/src/libsystemd-network/fuzz-dhcp-server.c:26:9 #3 0x4ec2bf in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/libsystemd-network/fuzz-dhcp-server.c:75:9 #4 0x4f68a8 in NaloFuzzerTestOneInput (/build/fuzz-dhcp-server+0x4f68a8) #5 0x5158b3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #6 0x51509a in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:514:3 #7 0x516769 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19 #8 0x517435 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5 #9 0x50679f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #10 0x507068 in LLVMFuzzerRunDriver /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:925:10 #11 0x4f6b25 in main (/build/fuzz-dhcp-server+0x4f6b25) #12 0x7f16084e3082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) DEDUP_TOKEN: __interceptor_calloc--calloc--add_lease SUMMARY: AddressSanitizer: 128 byte(s) leaked in 2 allocation(s). Found by Nallocufzz. (cherry picked from commit aca607d) (cherry picked from commit 18e08a4) (cherry picked from commit aa0fb9c)
1 parent 30767c8 commit a0573a9

File tree

3 files changed

+42
-18
lines changed

3 files changed

+42
-18
lines changed

src/libsystemd-network/dhcp-server-internal.h

+3
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ int dhcp_server_send_packet(sd_dhcp_server *server,
115115
void client_id_hash_func(const DHCPClientId *p, struct siphash *state);
116116
int client_id_compare_func(const DHCPClientId *a, const DHCPClientId *b);
117117

118+
DHCPLease *dhcp_lease_free(DHCPLease *lease);
119+
DEFINE_TRIVIAL_CLEANUP_FUNC(DHCPLease*, dhcp_lease_free);
120+
118121
#define log_dhcp_server_errno(server, error, fmt, ...) \
119122
log_interface_prefix_full_errno( \
120123
"DHCPv4 server: ", \

src/libsystemd-network/fuzz-dhcp-server.c

+38-15
Original file line numberDiff line numberDiff line change
@@ -17,36 +17,59 @@ ssize_t sendmsg(int sockfd, const struct msghdr *msg, int flags) {
1717
return 0;
1818
}
1919

20-
static void add_lease(sd_dhcp_server *server, const struct in_addr *server_address, uint8_t i) {
20+
static int add_lease(sd_dhcp_server *server, const struct in_addr *server_address, uint8_t i) {
21+
_cleanup_(dhcp_lease_freep) DHCPLease *lease = NULL;
2122
static const uint8_t chaddr[] = {3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3};
22-
DHCPLease *lease;
23+
int r;
2324

2425
assert(server);
2526

26-
assert_se(lease = new0(DHCPLease, 1));
27+
lease = new0(DHCPLease, 1);
28+
if (!lease)
29+
return -ENOMEM;
30+
31+
lease->client_id.data = malloc(2);
32+
if (!lease->client_id.data)
33+
return -ENOMEM;
34+
2735
lease->client_id.length = 2;
28-
assert_se(lease->client_id.data = malloc(2));
2936
lease->client_id.data[0] = 2;
3037
lease->client_id.data[1] = i;
38+
3139
lease->address = htobe32(UINT32_C(10) << 24 | i);
3240
lease->gateway = server_address->s_addr;
3341
lease->expiration = UINT64_MAX;
3442
lease->htype = ARPHRD_ETHER;
3543
lease->hlen = ETH_ALEN;
3644
memcpy(lease->chaddr, chaddr, ETH_ALEN);
37-
assert_se(hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease) >= 0);
38-
assert_se(hashmap_ensure_put(&server->bound_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease) >= 0);
39-
lease->server = server;
45+
46+
lease->server = server; /* This must be set just before hashmap_put(). */
47+
48+
r = hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease);
49+
if (r < 0)
50+
return r;
51+
52+
r = hashmap_ensure_put(&server->bound_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease);
53+
if (r < 0)
54+
return r;
55+
56+
TAKE_PTR(lease);
57+
58+
return 0;
4059
}
4160

42-
static void add_static_lease(sd_dhcp_server *server, uint8_t i) {
61+
static int add_static_lease(sd_dhcp_server *server, uint8_t i) {
4362
uint8_t id[2] = { 2, i };
63+
int r;
4464

4565
assert(server);
4666

47-
assert_se(sd_dhcp_server_set_static_lease(server,
48-
&(struct in_addr) { .s_addr = htobe32(UINT32_C(10) << 24 | i)},
49-
id, ELEMENTSOF(id)) >= 0);
67+
r = sd_dhcp_server_set_static_lease(
68+
server,
69+
&(struct in_addr) { .s_addr = htobe32(UINT32_C(10) << 24 | i)},
70+
id, ELEMENTSOF(id));
71+
72+
return r;
5073
}
5174

5275
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
@@ -66,12 +89,12 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
6689
assert_se(sd_dhcp_server_configure_pool(server, &address, 24, 0, 0) >= 0);
6790

6891
/* add leases to the pool to expose additional code paths */
69-
add_lease(server, &address, 2);
70-
add_lease(server, &address, 3);
92+
assert_se(add_lease(server, &address, 2) >= 0);
93+
assert_se(add_lease(server, &address, 3) >= 0);
7194

7295
/* add static leases */
73-
add_static_lease(server, 3);
74-
add_static_lease(server, 4);
96+
assert_se(add_static_lease(server, 3) >= 0);
97+
assert_se(add_static_lease(server, 4) >= 0);
7598

7699
(void) dhcp_server_handle_message(server, (DHCPMessage*) duped, size);
77100

src/libsystemd-network/sd-dhcp-server.c

+1-3
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
#define DHCP_DEFAULT_LEASE_TIME_USEC USEC_PER_HOUR
2828
#define DHCP_MAX_LEASE_TIME_USEC (USEC_PER_HOUR*12)
2929

30-
static DHCPLease *dhcp_lease_free(DHCPLease *lease) {
30+
DHCPLease *dhcp_lease_free(DHCPLease *lease) {
3131
if (!lease)
3232
return NULL;
3333

@@ -42,8 +42,6 @@ static DHCPLease *dhcp_lease_free(DHCPLease *lease) {
4242
return mfree(lease);
4343
}
4444

45-
DEFINE_TRIVIAL_CLEANUP_FUNC(DHCPLease*, dhcp_lease_free);
46-
4745
/* configures the server's address and subnet, and optionally the pool's size and offset into the subnet
4846
* the whole pool must fit into the subnet, and may not contain the first (any) nor last (broadcast) address
4947
* moreover, the server's own address may be in the pool, and is in that case reserved in order not to

0 commit comments

Comments
 (0)