From 7bae43964c598410aa918316b46d22985d78c42b Mon Sep 17 00:00:00 2001
From: Rongqi Sun <sunrongqi@huawei.com>
Date: Wed, 8 May 2024 08:16:13 +0000
Subject: [PATCH] crush/builder: free 'crush_rule' before return

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
    #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
    #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
    #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
    #5 0xaaaab01f5a6c in thread3(void*) /root/ceph-19.0.0/src/test/erasure-code/TestErasureCodeShec.cc:2756:11
    #6 0xffffa2dad5c4 in start_thread nptl/./nptl/pthread_create.c:442:8
    #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 <sunrongqi@huawei.com>
---
 src/crush/CrushWrapper.cc | 2 ++
 src/crush/CrushWrapper.h  | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index e434d1a17d851..da5424033219c 100644
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -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);
@@ -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);
diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index 7018ca498c67f..89de2024d35d4 100644
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -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) {