Codebase list liberasurecode / c9136a6
Fix valgrind-check and memory leak Can you believe that we ware testing the memory leak with valgrind to just *bash scripts* instead of actual binaries on liberasurecode_test and libec_slap? That is why we cannot find such an easy memory leak[1] at the gate. Now this patch enable to run the valgrind against to the binaries. With this fix, we found various memory leak at liberasurecode_test as follows and this patch also fixes them: - If we create fake fragments, we're responsible for freeing all of the frags as well as the array holding the pointers to the frags. - If we allocate any space, we're responsible for freeing it. - If we create an EC descriptor, we're responsible for destroying it. - If we create a fragment or skip array, we're responsible for freeing it. - If that happens inside a loop, we're responsible for doing it *inside that same loop*. In addition to the test fix, this patch fixes following memory leaks at the code which is affected to other users (pyeclib, OpenStack Swift) * Refuse to decode fragments that aren't even long enough to include fragment headers. * Fix a small memory leak in the builtin rs_vand implementation. Closes-Bug: #1665242 Co-Authored-By: Tim Burke <tim@swiftstack.com> 1: https://review.openstack.org/#/c/431812 Change-Id: I96f124e4e536bbd7544208acc084de1cda5c19b2 Kota Tsuyuzaki 7 years ago
4 changed file(s) with 111 addition(s) and 29 deletion(s). Raw diff Collapse all Expand all
+14
-5
Makefile.am less more
6161
6262 VALGRIND_EXEC_COMMAND = $(LIBTOOL_COMMAND) valgrind --tool=memcheck \
6363 --error-exitcode=1 --leak-check=yes --track-fds=yes \
64 --malloc-fill=A5 --free-fill=DE --fullpath-after=.
64 --malloc-fill=A5 --free-fill=DE --fullpath-after=. --trace-children=yes
6565
6666 valgrind-test: check
67 @$(VALGRIND_EXEC_COMMAND) ./test/alg_sig_test
68 @$(VALGRIND_EXEC_COMMAND) ./test/liberasurecode_test
69 @$(VALGRIND_EXEC_COMMAND) ./test/test_xor_hd_code
70 @$(VALGRIND_EXEC_COMMAND) ./test/libec_slap
67 $(eval SONAMES := $(shell find $(abs_top_builddir) -name '*.so'))
68 $(eval SODIRS := $(dir $(SONAMES)))
69 $(eval LD_LIBRARY_PATH := LD_LIBRARY_PATH="$(LD_LIBRARY_PATH):$(subst / ,/:,$(SODIRS))")
70 $(eval DYLD_LIBRARY_PATH := DYLD_LIBRARY_PATH="$(DYLD_LIBRARY_PATH):$(subst / ,/:,$(SODIRS))")
71 $(eval DYLD_FALLBACK_LIBRARY_PATH := DYLD_FALLBACK_LIBRARY_PATH=$(DYLD_FALLBACK_LIBRARY_PATH):"$(subst / ,/:,$(SODIRS))")
72 @$(LD_LIBRARY_PATH) $(DYLD_LIBRARY_PATH) $(DYLD_FALLBACK_LIBRARY_PATH) $(VALGRIND_EXEC_COMMAND) \
73 ./test/alg_sig_test
74 @$(LD_LIBRARY_PATH) $(DYLD_LIBRARY_PATH) $(DYLD_FALLBACK_LIBRARY_PATH) $(VALGRIND_EXEC_COMMAND) \
75 ./test/.libs/liberasurecode_test
76 @$(LD_LIBRARY_PATH) $(DYLD_LIBRARY_PATH) $(DYLD_FALLBACK_LIBRARY_PATH) $(VALGRIND_EXEC_COMMAND) \
77 ./test/test_xor_hd_code
78 @$(LD_LIBRARY_PATH) $(DYLD_LIBRARY_PATH) $(DYLD_FALLBACK_LIBRARY_PATH) $(VALGRIND_EXEC_COMMAND) \
79 ./test/.libs/libec_slap
7180
7281 CLEANFILES = cscope.in.out cscope.out cscope.po.out
7382
+1
-1
src/builtin/rs_vand/liberasurecode_rs_vand.c less more
542542 }
543543 region_dot_product(first_k_available, parity[destination_idx - k], parity_row, k, blocksize);
544544 }
545
545 free(parity_row);
546546 free(decoding_matrix);
547547 free(inverse_decoding_matrix);
548548 free(first_k_available);
+7
-0
src/erasurecode.c less more
586586 goto out;
587587 }
588588
589 if (fragment_len < sizeof(fragment_header_t)) {
590 log_error("Fragments not long enough to include headers! "
591 "Need %zu, but got %lu.", sizeof(fragment_header_t),
592 (unsigned long)fragment_len);
593 ret = -EBADHEADER;
594 goto out;
595 }
589596 for (i = 0; i < num_fragments; ++i) {
590597 /* Verify metadata checksum */
591598 if (is_invalid_fragment_header(
+89
-23
test/liberasurecode_test.c less more
372372 static int create_fake_frags_no_meta(char ***array, int num_frags,
373373 const char *data, int data_len)
374374 {
375 // N.B. The difference from creat_frags_arry is to creat new
376 // memory allocation and set a copy of data/parity there. The
377 // allocated memory should be maintained by the caller.
375378 int _num_frags = 0;
376379 int i = 0;
377380 char **ptr = NULL;
400403 struct ec_args *args,
401404 int *skips)
402405 {
406 // N.B. this function sets pointer reference to the ***array
407 // from **data and **parity so DO NOT free each value of
408 // the array independently because the data and parity will
409 // be expected to be cleanup via liberasurecode_encode_cleanup
403410 int num_frags = 0;
404411 int i = 0;
405412 char **ptr = NULL;
430437 return num_frags;
431438 }
432439
440 static void cleanup_avail_frags(char **avail_frags,
441 int num_frags)
442 {
443 while(num_frags > 0) free(avail_frags[--num_frags]);
444 free(avail_frags);
445 }
433446 static int encode_failure_stub(void *desc, char **data,
434447 char **parity, int blocksize)
435448 {
581594 assert(rc < 0);
582595 instance->common.ops->encode = orig_encode_func;
583596
597 liberasurecode_instance_destroy(desc);
584598 free(orig_data);
585599 }
586600
612626
613627 rc = liberasurecode_encode_cleanup(desc, encoded_data, encoded_parity);
614628 assert(rc == 0);
629 liberasurecode_instance_destroy(desc);
615630 free(orig_data);
616631 }
617632
628643 int *skips = create_skips_array(&null_args, -1);
629644 char *decoded_data = NULL;
630645 uint64_t decoded_data_len = 0;
631 const char *fake_data = " ";
646 // fake_data len should be bigger than fragment_header_t for
647 // the verifications
648 int fake_data_len = 1024;
649 char *fake_data = create_buffer(fake_data_len, 'y');
632650
633651 desc = liberasurecode_instance_create(EC_BACKEND_NULL, &null_args);
634652 if (-EBACKENDNOTAVAIL == desc) {
640658 // test with invalid fragments (no metadata headers)
641659 num_avail_frags = create_fake_frags_no_meta(&avail_frags, (null_args.k +
642660 null_args.m),
643 fake_data, strlen(fake_data));
661 fake_data, fake_data_len);
644662 assert(num_avail_frags > 0);
663 free(fake_data);
645664
646665 rc = liberasurecode_decode(desc, avail_frags, num_avail_frags,
647 strlen(fake_data), 1,
666 fake_data_len, 1,
648667 &decoded_data, &decoded_data_len);
649668 // no metadata headers w/ force_metadata_checks results in EBADHEADER
650669 assert(rc == -EBADHEADER);
651670
652671 rc = liberasurecode_decode(desc, avail_frags, num_avail_frags,
653 strlen(fake_data), 0,
672 fake_data_len, 0,
654673 &decoded_data, &decoded_data_len);
655674 // no metadata headers w/o force_metadata_checks also results in EBADHEADER
656675 assert(rc == -EBADHEADER);
676
677 // encoded_fragment_len is too small
678 rc = liberasurecode_decode(desc, avail_frags, num_avail_frags,
679 1, 1, &decoded_data, &decoded_data_len);
680 assert(rc == -EBADHEADER);
681
682 cleanup_avail_frags(avail_frags, num_avail_frags);
657683
658684 // test with num_fragments < (k)
659685 num_avail_frags = create_fake_frags_no_meta(&avail_frags, (null_args.k - 1),
660686 " ", 1);
661687 assert(num_avail_frags > 0);
662688 rc = liberasurecode_decode(desc, avail_frags, num_avail_frags,
663 strlen(fake_data), 1,
689 fake_data_len, 1,
664690 &decoded_data, &decoded_data_len);
665691 assert(rc == -EINSUFFFRAGS);
692
693 cleanup_avail_frags(avail_frags, num_avail_frags);
666694
667695 rc = liberasurecode_encode(desc, orig_data, orig_data_size,
668696 &encoded_data, &encoded_parity, &encoded_fragment_len);
691719 encoded_fragment_len, 1,
692720 &decoded_data, NULL);
693721 assert(rc < 0);
722
694723 free(skips);
695724 liberasurecode_encode_cleanup(desc, encoded_data, encoded_parity);
725 liberasurecode_instance_destroy(desc);
726 // N.B. create_frags_array sets pointer reference of either encoded_data
727 // or encoded_parity and they are cleaned up via
728 // liberasurecode_encode_cleanup
696729 free(avail_frags);
697730 free(orig_data);
698
699731 }
700732
701733 static void test_decode_cleanup_invalid_args()
716748
717749 rc = liberasurecode_decode_cleanup(desc, NULL);
718750 assert(rc == 0);
719
751 liberasurecode_instance_destroy(desc);
720752 free(orig_data);
721753 }
722754
752784 assert(rc < 0);
753785
754786 free(out_frag);
787 free(avail_frags);
755788
756789 // Test for EINSUFFFRAGS
757790 // we have to call encode to get fragments which have valid header.
765798 rc = liberasurecode_reconstruct_fragment(desc, encoded_data, 1, encoded_fragment_len, 1, out_frag);
766799
767800 assert(rc == -EINSUFFFRAGS);
801 free(orig_data);
768802 free(out_frag);
769 free(avail_frags);
770803 liberasurecode_encode_cleanup(desc, encoded_data, encoded_parity);
804 liberasurecode_instance_destroy(desc);
771805 }
772806
773807 static void test_fragments_needed_invalid_args()
796830
797831 rc = liberasurecode_fragments_needed(desc, &frags_to_recon, &frags_to_exclude, NULL);
798832 assert(rc < 0);
833 liberasurecode_instance_destroy(desc);
799834 }
800835
801836 static void test_get_fragment_metadata_invalid_args() {
846881
847882 rc = liberasurecode_verify_stripe_metadata(desc, frags, 0);
848883 assert(rc == -EINVALIDPARAMS);
849
884 liberasurecode_instance_destroy(desc);
885 free(frags);
850886 }
851887
852888 static void test_get_fragment_partition()
866902 desc = liberasurecode_instance_create(EC_BACKEND_NULL, &null_args);
867903 if (-EBACKENDNOTAVAIL == desc) {
868904 fprintf (stderr, "Backend library not available!\n");
905 free(orig_data);
906 free(skips);
869907 return;
870908 }
871909 assert(desc > 0);
875913
876914 missing = alloc_and_set_buffer(sizeof(char*) * null_args.k, -1);
877915
878 for(i = 0; i < null_args.m; i++) skips[i] = 1;
916 for(i = 0; i < null_args.m; i++) {
917 skips[i] = 1;
918 /* get_fragment_partition is going to NULL all the entries in
919 * encoded_data and encoded_parity before populating them again
920 * from avail_frags. Since we're explicitly *not* including these
921 * data frags in avail_frags, free them now.
922 */
923 if (i < null_args.k) {
924 free(encoded_data[i]);
925 } else {
926 free(encoded_parity[i - null_args.k]);
927 }
928 }
879929 num_avail_frags = create_frags_array(&avail_frags, encoded_data,
880930 encoded_parity, &null_args, skips);
881931
884934 assert(0 == rc);
885935
886936 for(i = 0; i < null_args.m; i++) assert(missing[i] == i);
887 assert(missing[++i] == -1);
888
937 // Loop already pushed us one past
938 assert(missing[i] == -1);
939
940 free(avail_frags);
889941 free(missing);
890942
891 for(i = 0; i < null_args.m + 1; i++) skips[i] = 1;
943 skips[i] = 1;
944 if (i < null_args.k) {
945 free(encoded_data[i]);
946 } else {
947 free(encoded_parity[i - null_args.k]);
948 }
949
892950 num_avail_frags = create_frags_array(&avail_frags, encoded_data,
893951 encoded_parity, &null_args, skips);
894952
904962 free(missing);
905963 free(skips);
906964 liberasurecode_encode_cleanup(desc, encoded_data, encoded_parity);
965 liberasurecode_instance_destroy(desc);
907966 free(avail_frags);
908967 free(orig_data);
909968 }
9991058 rc = liberasurecode_decode_cleanup(desc, decoded_data);
10001059 assert(rc == 0);
10011060
1002 if (desc) {
1003 assert(0 == liberasurecode_instance_destroy(desc));
1004 }
1005
1061 assert(0 == liberasurecode_instance_destroy(desc));
10061062 free(orig_data);
10071063 free(avail_frags);
10081064 }
10741130 rc = liberasurecode_reconstruct_fragment(desc, avail_frags, num_avail_frags, encoded_fragment_len, i, out);
10751131 assert(rc == 0);
10761132 assert(memcmp(out, cmp, encoded_fragment_len) == 0);
1133 free(avail_frags);
10771134 }
10781135 free(orig_data);
10791136 free(out);
1080 free(avail_frags);
10811137 liberasurecode_encode_cleanup(desc, encoded_data, encoded_parity);
1138 liberasurecode_instance_destroy(desc);
10821139 }
10831140
10841141 static void test_fragments_needed_impl(const ec_backend_id_t be_id,
12021259 assert(is_valid_fragment == 1);
12031260 i++;
12041261 }
1262 liberasurecode_instance_destroy(desc);
12051263 free(fragments_to_reconstruct);
12061264 free(fragments_to_exclude);
12071265 free(fragments_needed);
12741332 assert(be_version == be->common.ec_backend_version);
12751333 }
12761334 liberasurecode_encode_cleanup(desc, encoded_data, encoded_parity);
1335 liberasurecode_instance_destroy(desc);
12771336 free(orig_data);
12781337 }
12791338
13861445 EC_BACKEND_ISA_L_RS_VAND, &specific_1010_args);
13871446 if (-EBACKENDNOTAVAIL == desc) {
13881447 fprintf (stderr, "Backend library not available!\n");
1448 free(orig_data);
1449 free(skips);
13891450 return;
13901451 }
13911452 assert(desc > 0);
14351496 assert(rc == 0);
14361497
14371498 free(out_frag);
1499 free(avail_frags);
14381500
14391501 // cleanup all
14401502 rc = liberasurecode_encode_cleanup(desc, encoded_data, encoded_parity);
14431505 rc = liberasurecode_decode_cleanup(desc, decoded_data);
14441506 assert(rc == 0);
14451507
1446 if (desc) {
1447 assert(0 == liberasurecode_instance_destroy(desc));
1448 }
1449
1508 assert(0 == liberasurecode_instance_destroy(desc));
14501509 free(orig_data);
1451 free(avail_frags);
14521510 free(skips);
14531511 }
14541512
15091567
15101568 if (-EBACKENDNOTAVAIL == desc) {
15111569 fprintf (stderr, "Backend library not available!\n");
1570 free(orig_data);
1571 free(skip);
15121572 return;
15131573 }
15141574 assert(desc > 0);
15261586 assert(0 == rc);
15271587
15281588 liberasurecode_encode_cleanup(desc, encoded_data, encoded_parity);
1589 liberasurecode_instance_destroy(desc);
15291590 free(orig_data);
15301591 free(skip);
1592 free(avail_frags);
15311593 }
15321594
15331595 static void verify_fragment_metadata_mismatch_impl(const ec_backend_id_t be_id, struct ec_args *args,
15501612
15511613 if (-EBACKENDNOTAVAIL == desc) {
15521614 fprintf (stderr, "Backend library not available!\n");
1615 free(orig_data);
1616 free(skip);
15531617 return;
15541618 }
15551619 assert(desc > 0);
16141678 }
16151679 }
16161680 liberasurecode_encode_cleanup(desc, encoded_data, encoded_parity);
1681 liberasurecode_instance_destroy(desc);
1682 free(avail_frags);
16171683 free(orig_data);
16181684 free(skip);
16191685 }