Skip to content

Commit

Permalink
[lsan] Fix leaks detected by sanitier
Browse files Browse the repository at this point in the history
There were some leaks detected when running the test suite. But for `bcc_elf_get_buildid` which did not free the elf object, the rest of the leaks were isolated in the tests themselves which did not free some resources here and there.

This diff clears those leaks. This will allow running the tests suite in the future with LSAN enabled, helping in catching possible future leaks earlier.

Ran the sanitizer using:
```
docker run --privileged \
                   --pid=host \
                   -v $(pwd):/bcc \
                   -v /sys/kernel/debug:/sys/kernel/debug:rw \
                   -v /lib/modules:/lib/modules:ro \
                   -v /usr/src:/usr/src:ro \
                   -v /usr/include/linux:/usr/include/linux:ro \
                   bcc-docker \
                   /bin/bash -c \
                   'mkdir -p /bcc/build && cd /bcc/build && \
                    cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_LLVM_NATIVECODEGEN=OFF -DCMAKE_SANITIZE_TYPE=leak .. && make -j9'
```

followed by tests.

Before:

```
docker run -ti \
                    --privileged \
                    --network=host \
                    --pid=host \
                    -v $(pwd):/bcc \
                    -v /sys/kernel/debug:/sys/kernel/debug:rw \
                    -v /lib/modules:/lib/modules:ro \
                    -v /usr/src:/usr/src:ro \
                    -e CTEST_OUTPUT_ON_FAILURE=1 \
                    bcc-docker \
                    /bin/bash -c \
                    '/bcc/build/tests/wrapper.sh \
                        c_test_all sudo /bcc/build/tests/cc/test_libbcc' > /tmp/out
grep 'Indirect leak' /tmp/out | wc -l
99
grep 'Direct leak' /tmp/out | wc -l
4
```

Full out file available in https://gist.github.com/chantra/caa3c6f6a274895d8743fe9e48a7c528

After:
```
docker run -ti \
                    --privileged \
                    --network=host \
                    --pid=host \
                    -v $(pwd):/bcc \
                    -v /sys/kernel/debug:/sys/kernel/debug:rw \
                    -v /lib/modules:/lib/modules:ro \
                    -v /usr/src:/usr/src:ro \
                    -e CTEST_OUTPUT_ON_FAILURE=1 \
                    bcc-docker \
                    /bin/bash -c \
                    '/bcc/build/tests/wrapper.sh \
                        c_test_all sudo /bcc/build/tests/cc/test_libbcc'
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_libbcc is a Catch v1.4.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
searching for modules in /proc/[pid]/maps
-------------------------------------------------------------------------------
/bcc/tests/cc/test_c_api.cc:497
...............................................................................

/bcc/tests/cc/test_c_api.cc:499: FAILED:
  REQUIRE( dummy_maps != __null )
with expansion:
  NULL != 0

-------------------------------------------------------------------------------
test bpf table
-------------------------------------------------------------------------------
/bcc/tests/cc/test_bpf_table.cc:24
...............................................................................

/bcc/tests/cc/test_bpf_table.cc:24: FAILED:
  {Unknown expression after the reported line}
due to unexpected exception with message:
  bad_function_call

-------------------------------------------------------------------------------
test bpf percpu tables
-------------------------------------------------------------------------------
/bcc/tests/cc/test_bpf_table.cc:94
...............................................................................

/bcc/tests/cc/test_bpf_table.cc:94: FAILED:
  {Unknown expression after the reported line}
due to unexpected exception with message:
  bad_function_call

-------------------------------------------------------------------------------
test bpf stack_id table
-------------------------------------------------------------------------------
/bcc/tests/cc/test_bpf_table.cc:227
...............................................................................

/bcc/tests/cc/test_bpf_table.cc:268: FAILED:
  REQUIRE( addrs.size() > 0 )
with expansion:
  0 > 0

Parse error:
    4@i%ra+1r
-------^
===============================================================================
test cases:  51 |  47 passed | 1 failed | 3 failed as expected
assertions: 984 | 980 passed | 1 failed | 3 failed as expected

Failed
```
  • Loading branch information
chantra authored and yonghong-song committed Aug 1, 2022
1 parent 8528919 commit 6616092
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 7 deletions.
9 changes: 7 additions & 2 deletions src/cc/bcc_elf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1040,14 +1040,19 @@ int bcc_elf_get_buildid(const char *path, char *buildid)
{
Elf *e;
int fd;
int rc = -1;

if (openelf(path, &e, &fd) < 0)
return -1;

if (!find_buildid(e, buildid))
return -1;
goto exit;

return 0;
rc = 0;
exit:
elf_end(e);
close(fd);
return rc;
}

int bcc_elf_symbol_str(const char *path, size_t section_idx,
Expand Down
9 changes: 4 additions & 5 deletions tests/cc/test_bpf_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ TEST_CASE("test bpf table", ebpf::bpf_module_rw_engine_enabled() ? "[bpf_table]"
BPF_TABLE("hash", int, int, myhash, 128);
)";

ebpf::BPF *bpf(new ebpf::BPF);
ebpf::BPF bpf;
ebpf::StatusTuple res(0);
std::vector<std::pair<std::string, std::string>> elements;
res = bpf->init(BPF_PROGRAM);
res = bpf.init(BPF_PROGRAM);
REQUIRE(res.ok());

ebpf::BPFTable t = bpf->get_table("myhash");
ebpf::BPFTable t = bpf.get_table("myhash");

// update element
std::string value;
Expand Down Expand Up @@ -78,8 +78,7 @@ TEST_CASE("test bpf table", ebpf::bpf_module_rw_engine_enabled() ? "[bpf_table]"
REQUIRE(res.ok());
REQUIRE(elements.size() == 0);

// delete bpf_module, call to key/leaf printf/scanf must fail
delete bpf;


res = t.update_value("0x07", "0x42");
REQUIRE(!res.ok());
Expand Down
10 changes: 10 additions & 0 deletions tests/cc/test_c_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,11 @@ TEST_CASE("resolve symbol addresses for a given PID", "[c_api]") {
REQUIRE(bcc_symcache_resolve_name(lazy_resolver, "/tmp/libz.so.1", "zlibVersion",
&lazy_addr) == 0);
REQUIRE(lazy_addr == addr);
bcc_free_symcache(resolver, child);
bcc_free_symcache(lazy_resolver, child);
}
bcc_free_symcache(resolver, getpid());
bcc_free_symcache(lazy_resolver, getpid());
}

#define STACK_SIZE (1024 * 1024)
Expand Down Expand Up @@ -412,6 +416,8 @@ TEST_CASE("resolve symbols using /tmp/perf-pid.map", "[c_api]") {
REQUIRE(sym.module);
REQUIRE(string(sym.module) == perf_map_path(child));
REQUIRE(string("right_next_door_fn") == sym.name);
bcc_free_symcache(resolver, child);

}

SECTION("separate namespace") {
Expand All @@ -428,6 +434,7 @@ TEST_CASE("resolve symbols using /tmp/perf-pid.map", "[c_api]") {
REQUIRE(string(sym.module) == perf_map_path(1));
REQUIRE(string("dummy_fn") == sym.name);
unlink("/tmp/perf-1.map");
bcc_free_symcache(resolver, child);
}

SECTION("separate pid and mount namespace") {
Expand All @@ -444,6 +451,7 @@ TEST_CASE("resolve symbols using /tmp/perf-pid.map", "[c_api]") {
// child is PID 1 in its namespace
REQUIRE(string(sym.module) == perf_map_path(1));
REQUIRE(string("dummy_fn") == sym.name);
bcc_free_symcache(resolver, child);
}

SECTION("separate pid and mount namespace, perf-map in host") {
Expand All @@ -465,6 +473,7 @@ TEST_CASE("resolve symbols using /tmp/perf-pid.map", "[c_api]") {
REQUIRE(string("dummy_fn") == sym.name);

unlink(path.c_str());
bcc_free_symcache(resolver, child);
}


Expand Down Expand Up @@ -598,6 +607,7 @@ TEST_CASE("resolve global addr in libc in this process", "[c_api][!mayfail]") {
res = bcc_resolve_global_addr(pid, sopath, local_addr, 0, &global_addr);
REQUIRE(res == 0);
REQUIRE(global_addr == (search.start + local_addr - search.file_offset));
free(sopath);
}

/* Consider the following scenario: we have some process that maps in a shared library [1] with a
Expand Down
1 change: 1 addition & 0 deletions tests/cc/test_usdt_probes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ TEST_CASE("test probing running Ruby process in namespaces",
std::string module = pid_root + "usr/local/bin/ruby";
REQUIRE(bcc_resolve_symname(module.c_str(), "rb_gc_mark", 0x0, ruby_pid, nullptr, &sym) == 0);
REQUIRE(std::string(sym.module).find(pid_root, 1) == std::string::npos);
bcc_procutils_free(sym.module);
}
}

Expand Down

0 comments on commit 6616092

Please sign in to comment.