libFuzzer: Fix a leak in compile_filter() cleanup
This crash was found by running libFuzzer+ASan. Under some
circumstances, the cleanup performed in compile_filter() skips
free(3)-ing some things before returning. This change restructures the
function so the cleanup is always performed.
Bug: None
Test: make tests (with ASan)
Change-Id: I5fd22ecc6a400d7ef44ad0c1ccfcd2fafeaa04ed
diff --git a/syscall_filter.c b/syscall_filter.c
index 0012325..adc4b3c 100644
--- a/syscall_filter.c
+++ b/syscall_filter.c
@@ -629,6 +629,7 @@
int compile_filter(FILE *initial_file, struct sock_fprog *prog,
int use_ret_trap, int allow_logging)
{
+ int ret = 0;
struct bpf_labels labels;
labels.count = 0;
@@ -657,10 +658,8 @@
if (compile_file(initial_file, head, &arg_blocks, &labels, use_ret_trap,
allow_logging, 0 /* include_level */) != 0) {
warn("compile_filter: compile_file() failed");
- free_block_list(head);
- free_block_list(arg_blocks);
- free_label_strings(&labels);
- return -1;
+ ret = -1;
+ goto free_filter;
}
/*
@@ -675,30 +674,41 @@
/* Allocate the final buffer, now that we know its size. */
size_t final_filter_len =
head->total_len + (arg_blocks ? arg_blocks->total_len : 0);
- if (final_filter_len > BPF_MAXINSNS)
- return -1;
+ if (final_filter_len > BPF_MAXINSNS) {
+ ret = -1;
+ goto free_filter;
+ }
struct sock_filter *final_filter =
calloc(final_filter_len, sizeof(struct sock_filter));
- if (flatten_block_list(head, final_filter, 0, final_filter_len) < 0)
- return -1;
+ if (flatten_block_list(head, final_filter, 0, final_filter_len) < 0) {
+ free(final_filter);
+ ret = -1;
+ goto free_filter;
+ }
if (flatten_block_list(arg_blocks, final_filter, head->total_len,
- final_filter_len) < 0)
- return -1;
+ final_filter_len) < 0) {
+ free(final_filter);
+ ret = -1;
+ goto free_filter;
+ }
- free_block_list(head);
- free_block_list(arg_blocks);
-
- if (bpf_resolve_jumps(&labels, final_filter, final_filter_len) < 0)
- return -1;
-
- free_label_strings(&labels);
+ if (bpf_resolve_jumps(&labels, final_filter, final_filter_len) < 0) {
+ free(final_filter);
+ ret = -1;
+ goto free_filter;
+ }
prog->filter = final_filter;
prog->len = final_filter_len;
- return 0;
+
+free_filter:
+ free_block_list(head);
+ free_block_list(arg_blocks);
+ free_label_strings(&labels);
+ return ret;
}
int flatten_block_list(struct filter_block *head, struct sock_filter *filter,
diff --git a/syscall_filter_unittest.cc b/syscall_filter_unittest.cc
index 98a28c3..8dd1828 100644
--- a/syscall_filter_unittest.cc
+++ b/syscall_filter_unittest.cc
@@ -1616,4 +1616,21 @@
ASSERT_NE(res, 0);
}
+TEST(FilterTest, error_cleanup_leak) {
+ struct sock_fprog actual;
+ const char *policy =
+ "read:&&\n"
+ "read:&&";
+
+ FILE *policy_file = write_policy_to_pipe(policy, strlen(policy));
+ ASSERT_NE(policy_file, nullptr);
+ int res = compile_filter(policy_file, &actual, USE_RET_KILL, NO_LOGGING);
+ fclose(policy_file);
+
+ /*
+ * Policy is malformed, but process should not leak.
+ */
+ ASSERT_EQ(res, -1);
+}
+
#endif // !__ANDROID__