Skip to content

Commit

Permalink
crush/builder: free 'crush_rule' before return
Browse files Browse the repository at this point in the history
When sanitizer is enabled, unittest_erasure_code_shec shows:

```
=================================================================
==437976==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 2021708 byte(s) in 29731 object(s) allocated from:
    #0 0xaaaab0144820 in malloc (/root/ceph-19.0.0/build/bin/unittest_erasure_code_shec+0x1d4820) (BuildId: c0999ecf82504ed7e184ea4b4b9ae9d32faa1c46)
    #1 0xffffa770057c in crush_make_rule /root/ceph-19.0.0/src/crush/builder.c:104:9
    ceph#2 0xffffa7751638 in CrushWrapper::add_simple_rule_at(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int, int, std::ostream*) /root/ceph-19.0.0/src/crush/CrushWrapper.cc:2327:22
    ceph#3 0xffffa7751d2c in CrushWrapper::add_simple_rule(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int, std::ostream*) /root/ceph-19.0.0/src/crush/CrushWrapper.cc:2369:10
    ceph#4 0xffffa39d6198 in ceph::ErasureCode::create_rule(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CrushWrapper&, std::ostream*) const /root/ceph-19.0.0/src/erasure-code/ErasureCode.cc:76:18
    ceph#5 0xaaaab01f5a6c in thread3(void*) /root/ceph-19.0.0/src/test/erasure-code/TestErasureCodeShec.cc:2756:11
    ceph#6 0xffffa2dad5c4 in start_thread nptl/./nptl/pthread_create.c:442:8
    ceph#7 0xffffa2e15ed8  misc/../sysdeps/unix/sysv/linux/aarch64/clone.S:79

SUMMARY: AddressSanitizer: 2021708 byte(s) leaked in 29731 allocation(s).
```

When crush_add_rule exceeds the number of max_rules, crush_rule shoule
be freed right before returning in caller site.

Signed-off-by: Rongqi Sun <[email protected]>
  • Loading branch information
Svelar committed May 9, 2024
1 parent 4a987d7 commit 7bae439
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/crush/CrushWrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2351,6 +2351,7 @@ int CrushWrapper::add_simple_rule_at(
int ret = crush_add_rule(crush, rule, rno);
if(ret < 0) {
*err << "failed to add rule " << rno << " because " << cpp_strerror(ret);
free(rule);
return ret;
}
set_rule_name(rno, name);
Expand Down Expand Up @@ -2455,6 +2456,7 @@ int CrushWrapper::add_multi_osd_per_failure_domain_rule_at(
int ret = crush_add_rule(crush, rule, rno);
if(ret < 0) {
*err << "failed to add rule " << rno << " because " << cpp_strerror(ret);
free(rule);
return ret;
}
set_rule_name(rno, name);
Expand Down
3 changes: 3 additions & 0 deletions src/crush/CrushWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -1172,6 +1172,9 @@ class CrushWrapper {
crush_rule *n = crush_make_rule(len, type);
ceph_assert(n);
ruleno = crush_add_rule(crush, n, ruleno);
if (ruleno < 0) {
free(n);
}
return ruleno;
}
int set_rule_step(unsigned ruleno, unsigned step, int op, int arg1, int arg2) {
Expand Down

0 comments on commit 7bae439

Please sign in to comment.