Leak Memory

[Hữu Quang Linh Lê]

I found some input causing leak memory. This is crash information.

INFO: Seed: 3904123746
INFO: Loaded 1 modules   (50397 inline 8-bit counters): 50397 [0x1267560, 0x1273a3d),
INFO: Loaded 1 PC tables (50397 PCs): 50397 [0x1273a40,0x1338810),
./encoder-fuzzer: Running 1 inputs 1 time(s) each.
Running: leak-034018c6753ae7d385399bf5c3071ba3863a95c8

=================================================================
==30238==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 984 byte(s) in 1 object(s) allocated from:
    #0 0x4b2f4e in posix_memalign /src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:218
    #1 0xc2f80c in x265::x265_malloc(unsigned long) /src/x265/source/common/common.cpp:81:9
    #2 0xc302d9 in x265_param_alloc /src/x265/source/common/param.cpp:95:25
    #3 0x7f059d in x265_encoder_open_169 /src/x265/source/encoder/api.cpp:97:37
    #4 0x6082d0 in x265_encode_image(void*, heif_image const*, heif_image_input_class) /src/libheif/libheif/heif_encoder_x265.cc:707:22
    #5 0x5d52d4 in heif::HeifContext::Image::encode_image_as_hevc(std::__1::shared_ptr<heif::HeifPixelImage>, heif_encoder*, heif_encoding_options const*, heif_image_input_class) /src/libheif/libheif/heif_context.cc:1700:27
    #6 0x5d4791 in heif::HeifContext::encode_image(std::__1::shared_ptr<heif::HeifPixelImage>, heif_encoder*, heif_encoding_options const*, heif_image_input_class, std::__1::shared_ptr<heif::HeifContext::Image>&) /src/libheif/libheif/heif_context.cc:1608:28
    #7 0x5b8f47 in heif_context_encode_image /src/libheif/libheif/heif.cc:1561:25
    #8 0x60bfa8 in LLVMFuzzerTestOneInput /src/libheif/libheif/encoder_fuzzer.cc:161:9
    #9 0x6465f5 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:529:15
    #10 0x60e076 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/libfuzzer/FuzzerDriver.cpp:286:6
    #11 0x619ba3 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:715:9
    #12 0x60d6ec in main /src/libfuzzer/FuzzerMain.cpp:19:10
    #13 0x7f9cd365582f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: 984 byte(s) leaked in 1 allocation(s).

Do you think this is a bug? I want to report to you so that I can fix this soon.

[Cosmin Stejerean]

As far as I can tell zoneParam might be the cause . In #508 a free was added for zoneParam in the fail block at the end, but in the non-error case zoneParam appears to be leaking. Having PARAM_NS::x265_param_free(zoneParam) also execute in the non-failure case appears to be solving this problem.

[houlei]

It is best to provide specific steps to reproduce.

[DaDaLynn]

I’ve encountered a similary memory-leak problem like yours, and by adding PARAM_NS::x265_param_free(zoneParam) in the non-failure case(as @Cosmin Stejerean said) solved it.

[Ade Miller]

Seems like the underlying issue here is that x265_param_free does not free any strings it may own. For example the following code will also leak memory because the code to free up x265_param strings, like numaPools` resides in Encoder::destroy()`.

  const x265_api* api = x265_api_get(8);
  x265_param* param = api->param_alloc();

  api->param_default(param);
  param->sourceWidth = 1920;
  param->sourceHeight = 1080;
  param->fpsNum = 1;
  param->fpsDenom = 30;
  //api->param_parse(param, "analysis-reuse-file", "filename");
  api->param_parse(param, "pools", "8,8,8,8");
  api->param_free(param);

ASAN output:

==9441==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7fcf79459bd0 in __interceptor_strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3abd0)
    #1 0x559b39a942fa in x265_param_parse ../../third_party/libx265/source/common/param.cpp:1196
    #2 0x559b373102b2 in media::Video_LXH265VideoEncoder_Leak_Test::TestBody() ../../media/internal/fm_video_encoder_test.cc:381
    #3 0x559b376bb65f in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../../third_party/googletest/googletest/src/gtest.cc:2433
    #4 0x559b376a9067 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../../third_party/googletest/googletest/src/gtest.cc:2469
    #5 0x559b3767012b in testing::Test::Run() ../../third_party/googletest/googletest/src/gtest.cc:2508
    #6 0x559b37671596 in testing::TestInfo::Run() ../../third_party/googletest/googletest/src/gtest.cc:2684
    #7 0x559b3767228b in testing::TestSuite::Run() ../../third_party/googletest/googletest/src/gtest.cc:2816
    #8 0x559b37690252 in testing::internal::UnitTestImpl::RunAllTests() ../../third_party/googletest/googletest/src/gtest.cc:5338
    #9 0x559b376bf4c0 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../../third_party/googletest/googletest/src/gtest.cc:2433
    #10 0x559b376ac6b6 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../../third_party/googletest/googletest/src/gtest.cc:2469
    #11 0x559b3768c8d7 in testing::UnitTest::Run() ../../third_party/googletest/googletest/src/gtest.cc:4925
    #12 0x559b375df3bd in RUN_ALL_TESTS() ../../third_party/googletest/googletest/include/gtest/gtest.h:2473
    #13 0x559b375dea2b in main ../../testing/test_main.cc:40
    #14 0x7fcf7259bbf6 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21bf6)

Objects leaked above:
0x602000004e70 (8 bytes)

SUMMARY: AddressSanitizer: 8 byte(s) leaked in 1 allocation(s).

So you might expect the following to work:

  const x265_api* api = x265_api_get(8);
  x265_param* param = api->param_alloc();

  api->param_default(param);
  param->sourceWidth = 1920;
  param->sourceHeight = 1080;
  param->fpsNum = 1;
  param->fpsDenom = 30;
  //api->param_parse(param, "analysis-reuse-file", "filename");
  api->param_parse(param, "pools", "8,8,8,8");
  x265_encoder* encoder = api->encoder_open(param);
  api->encoder_close(encoder);
  api->param_free(param);

This also leaks because x265_encoder copies param and keeps an internal copy, m_param, which it frees correctly. However, it also creates other copies. Which is does not free the string resources for and the orginal param is also (still) not freed correctly by x265_param_free.

It seems like the correct approach would be to have `x265_param_free` free up all the string resources owned by the x265_param, rather than having x265_encoder do this.