From 906294ae9a388648cff313490f1efee2a56cb88e Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Fri, 12 Jun 2026 13:00:12 +0300 Subject: [PATCH] Fix ec_check_combination() short tmp_buf allocation --- src/osd/osd_rmw.cpp | 43 +++++++++++++++++++++------------------- src/osd/osd_rmw_test.cpp | 30 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/osd/osd_rmw.cpp b/src/osd/osd_rmw.cpp index c34a0d76..779b8671 100644 --- a/src/osd/osd_rmw.cpp +++ b/src/osd/osd_rmw.cpp @@ -1183,7 +1183,7 @@ static uint64_t c_n_k(uint64_t n, uint64_t k) static std::vector ec_check_combination(osd_rmw_stripe_t *stripes, int stripe_count, int *subset, int pg_size, int pg_minsize, bool is_xor, - uint32_t chunk_size, uint32_t bitmap_size, uint8_t *tmp_buf) + uint32_t chunk_size, uint32_t bitmap_size, std::vector& tmp_buf) { osd_num_t fake_osd_set[pg_size]; for (int i = 0; i < pg_size; i++) @@ -1211,13 +1211,15 @@ static std::vector ec_check_combination(osd_rmw_stripe_t *stripes, int stri { // missing chunks are recovered in read_bufs and write_bufs are used as source for parity bs.missing = true; - bs.read_buf = bs.write_buf = tmp_buf+i*chunk_size; - bs.bmp_buf = tmp_buf + stripe_count*chunk_size + i*bitmap_size; + assert(tmp_buf.size() >= (i+1)*(chunk_size+bitmap_size)); + bs.read_buf = bs.write_buf = tmp_buf.data() + i*(chunk_size+bitmap_size); + bs.bmp_buf = bs.read_buf + chunk_size; } else if (i >= pg_minsize) { // parity chunks are regenerated in their write_bufs, so use a temporary buffer - bs.write_buf = tmp_buf+i*chunk_size; + assert(tmp_buf.size() >= (i+1)*(chunk_size+bitmap_size)); + bs.write_buf = tmp_buf.data() + i*(chunk_size+bitmap_size); } } if (is_xor) @@ -1281,7 +1283,8 @@ static int count_roles(osd_rmw_stripe_t *stripes, std::vector & valid_chunk std::vector ec_find_good(osd_rmw_stripe_t *stripes, int stripe_count, int pg_size, int pg_minsize, bool is_xor, uint32_t chunk_size, uint32_t bitmap_size, uint64_t max_bruteforce, bool find_best) { - std::vector found_valid; + std::vector final_valid; + int final_roles = 0; std::vector> live_variants(pg_size); int eq_to[stripe_count]; int live_roles = 0, live_total = 0; @@ -1326,8 +1329,8 @@ std::vector ec_find_good(osd_rmw_stripe_t *stripes, int stripe_count, int p // Nothing to validate, just return all live chunks for (int i = 0; i < stripe_count; i++) if (!stripes[i].read_error) - found_valid.push_back(i); - return found_valid; + final_valid.push_back(i); + return final_valid; } // Try to locate errors using brute force if there isn't too many combinations bool brute_force = c_n_k(live_roles, pg_minsize) <= max_bruteforce; @@ -1341,7 +1344,7 @@ std::vector ec_find_good(osd_rmw_stripe_t *stripes, int stripe_count, int p } // Select all combinations with items except the last one (== anything to compare) first_combination(combination, pg_minsize, live_roles); - uint8_t *tmp_buf = (uint8_t*)malloc_or_die(stripe_count*(chunk_size+bitmap_size)); + std::vector tmp_buf(pg_size*(chunk_size+bitmap_size)); do { // Then loop over all subvariants (if some roles have multiple diverged variants of data) @@ -1362,22 +1365,24 @@ std::vector ec_find_good(osd_rmw_stripe_t *stripes, int stripe_count, int p // like 1 2 3 -> valid 4 5 and 1 3 4 -> valid 2 5 if (valid_chunks.size() > 0) { - if (found_valid.size() >= valid_chunks.size() && found_valid != valid_chunks) + int valid_roles = count_roles(stripes, valid_chunks, pg_size); + if (!final_valid.size() || final_valid == valid_chunks || find_best && valid_roles > final_roles) + { + final_valid = valid_chunks; + final_roles = valid_roles; + } + else { // Ambiguity: we found multiple valid sets and don't know which one is correct printf("Scrub found 2 different correct chunk subsets: OSD "); - for (int i = 0; i < found_valid.size(); i++) - printf(i > 0 ? ", %ju" : "%ju", stripes[found_valid[i]].osd_num); + for (int i = 0; i < final_valid.size(); i++) + printf(i > 0 ? ", %ju" : "%ju", stripes[final_valid[i]].osd_num); printf(" and OSD "); for (int i = 0; i < valid_chunks.size(); i++) printf(i > 0 ? ", %ju" : "%ju", stripes[valid_chunks[i]].osd_num); printf("\n"); - found_valid.clear(); - goto out; - } - else if (!found_valid.size() && (find_best || count_roles(stripes, valid_chunks, pg_size) >= pg_size)) - { - found_valid = valid_chunks; + final_valid.clear(); + return {}; } } // Select next subvariant @@ -1399,7 +1404,5 @@ std::vector ec_find_good(osd_rmw_stripe_t *stripes, int stripe_count, int p break; } } while (next_combination(combination, pg_minsize, live_roles)); -out: - free(tmp_buf); - return found_valid; + return final_valid; } diff --git a/src/osd/osd_rmw_test.cpp b/src/osd/osd_rmw_test.cpp index f3eaa5af..5023bb9e 100644 --- a/src/osd/osd_rmw_test.cpp +++ b/src/osd/osd_rmw_test.cpp @@ -33,6 +33,7 @@ void test_ec43_error_bruteforce(); void test_recover_53_d5(); void test_recover_22(); void test_ec_find_good_multi_chunks(); +void test_ec_find_good_42_no_good(); int main(int narg, char *args[]) { @@ -74,6 +75,7 @@ int main(int narg, char *args[]) // Error bruteforce test_ec43_error_bruteforce(); test_ec_find_good_multi_chunks(); + test_ec_find_good_42_no_good(); // Test 19 test_recover_53_d5(); // Test 20 @@ -1422,6 +1424,34 @@ void test_recover_22() use_ec(4, 2, false); } +void test_ec_find_good_42_no_good() +{ + use_ec(6, 4, true); + osd_rmw_stripe_t stripes[6] = {}; + split_stripes(4, 4096, 0, 4096 * 4, stripes); + uint8_t *write_buf = (uint8_t*)malloc_or_die(4096 * 6); + set_pattern(write_buf+0*4096, 4096, PATTERN0); + set_pattern(write_buf+1*4096, 4096, PATTERN1); + set_pattern(write_buf+2*4096, 4096, PATTERN2); + set_pattern(write_buf+3*4096, 4096, PATTERN3); + set_pattern(write_buf+4*4096, 4096, 1); + set_pattern(write_buf+5*4096, 4096, 2); + memset(stripes, 0, sizeof(stripes)); + for (int i = 0; i < 6; i++) + { + stripes[i].read_start = 0; + stripes[i].read_end = 4096; + stripes[i].read_buf = write_buf+i*4096; + stripes[i].role = i; + stripes[i].osd_num = i+1; + } + auto res = ec_find_good(stripes, 5, 6, 4, false, 4096, 0, 100, true); + assert_eq_vec(res, std::vector()); + // Done + free(write_buf); + use_ec(6, 4, false); +} + void test_ec_find_good_multi_chunks() { use_ec(7, 4, true);