applypatch: Fix a memory leak in ApplyImagePatch().
$ valgrind --leak-check=full out/host/linux-x86/nativetest64/recovery_host_test/recovery_host_test
==36755== 112 bytes in 1 blocks are definitely lost in loss record 4 of 16
==36755== at 0x40307C4: malloc (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:270)
==36755== by 0x40C1669: operator new(unsigned long) (external/libcxxabi/src/cxa_new_delete.cpp:46)
==36755== by 0x18D6A8: ApplyImagePatch(unsigned char const*, unsigned long, Value const*, std::__1::function<unsigned long (unsigned char const*, unsigned long)>, sha_state_st*, Value const*) (bootable/recovery/applypatch/imgpatch.cpp:62)
==36755== by 0x18D02B: ApplyImagePatch(unsigned char const*, unsigned long, unsigned char const*, unsigned long, std::__1::function<unsigned long (unsigned char const*, unsigned long)>) (bootable/recovery/applypatch/imgpatch.cpp:134)
==36755== by 0x160D15: GenerateTarget(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*) (bootable/recovery/tests/component/imgdiff_test.cpp:85)
==36755== by 0x11FA7D: verify_patched_image(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) (bootable/recovery/tests/component/imgdiff_test.cpp:96)
==36755== by 0x12966C: ImgdiffTest_zip_mode_smoke_trailer_zeros_Test::TestBody() (bootable/recovery/tests/component/imgdiff_test.cpp:295)
==36755== by 0x235EF9: testing::Test::Run() (external/googletest/googletest/src/gtest.cc:2455)
==36755== by 0x236CBF: testing::TestInfo::Run() (external/googletest/googletest/src/gtest.cc:2653)
==36755== by 0x2372D6: testing::TestCase::Run() (external/googletest/googletest/src/gtest.cc:2771)
==36755== by 0x23EEE6: testing::internal::UnitTestImpl::RunAllTests() (external/googletest/googletest/src/gtest.cc:4648)
==36755== by 0x23EB45: testing::UnitTest::Run() (external/googletest/googletest/src/gtest.cc:2455)
std::unique_ptr<z_stream, decltype(&deflateEnd)> strm(new z_stream(), deflateEnd);
Only the internally allocated buffers inside 'strm' would be free'd by
deflateEnd(), but not 'strm' itself.
This CL fixes the issue by moving 'strm' to stack variable. Note that we
only need to call deflateEnd() on successful return of deflateInit2().
Test: recovery_host_test && recovery_component_test
Change-Id: I39b9bdf62376b8029f95cab82c8542bfcb874009
diff --git a/applypatch/imgpatch.cpp b/applypatch/imgpatch.cpp
index 7a43ddb..25ba0a1 100644
--- a/applypatch/imgpatch.cpp
+++ b/applypatch/imgpatch.cpp
@@ -59,13 +59,13 @@
int mem_level = Read4(deflate_header + 52);
int strategy = Read4(deflate_header + 56);
- std::unique_ptr<z_stream, decltype(&deflateEnd)> strm(new z_stream(), deflateEnd);
- strm->zalloc = Z_NULL;
- strm->zfree = Z_NULL;
- strm->opaque = Z_NULL;
- strm->avail_in = 0;
- strm->next_in = nullptr;
- int ret = deflateInit2(strm.get(), level, method, window_bits, mem_level, strategy);
+ z_stream strm;
+ strm.zalloc = Z_NULL;
+ strm.zfree = Z_NULL;
+ strm.opaque = Z_NULL;
+ strm.avail_in = 0;
+ strm.next_in = nullptr;
+ int ret = deflateInit2(&strm, level, method, window_bits, mem_level, strategy);
if (ret != Z_OK) {
LOG(ERROR) << "Failed to init uncompressed data deflation: " << ret;
return false;
@@ -76,18 +76,19 @@
size_t actual_target_length = 0;
size_t total_written = 0;
static constexpr size_t buffer_size = 32768;
- auto compression_sink = [&](const uint8_t* data, size_t len) -> size_t {
+ auto compression_sink = [&strm, &actual_target_length, &expected_target_length, &total_written,
+ &ret, &ctx, &sink](const uint8_t* data, size_t len) -> size_t {
// The input patch length for an update never exceeds INT_MAX.
- strm->avail_in = len;
- strm->next_in = data;
+ strm.avail_in = len;
+ strm.next_in = data;
do {
std::vector<uint8_t> buffer(buffer_size);
- strm->avail_out = buffer_size;
- strm->next_out = buffer.data();
+ strm.avail_out = buffer_size;
+ strm.next_out = buffer.data();
if (actual_target_length + len < expected_target_length) {
- ret = deflate(strm.get(), Z_NO_FLUSH);
+ ret = deflate(&strm, Z_NO_FLUSH);
} else {
- ret = deflate(strm.get(), Z_FINISH);
+ ret = deflate(&strm, Z_FINISH);
}
if (ret != Z_OK && ret != Z_STREAM_END) {
LOG(ERROR) << "Failed to deflate stream: " << ret;
@@ -95,20 +96,24 @@
return 0;
}
- size_t have = buffer_size - strm->avail_out;
+ size_t have = buffer_size - strm.avail_out;
total_written += have;
if (sink(buffer.data(), have) != have) {
LOG(ERROR) << "Failed to write " << have << " compressed bytes to output.";
return 0;
}
if (ctx) SHA1_Update(ctx, buffer.data(), have);
- } while ((strm->avail_in != 0 || strm->avail_out == 0) && ret != Z_STREAM_END);
+ } while ((strm.avail_in != 0 || strm.avail_out == 0) && ret != Z_STREAM_END);
actual_target_length += len;
return len;
};
- if (ApplyBSDiffPatch(src_data, src_len, patch, patch_offset, compression_sink, nullptr) != 0) {
+ int bspatch_result =
+ ApplyBSDiffPatch(src_data, src_len, patch, patch_offset, compression_sink, nullptr);
+ deflateEnd(&strm);
+
+ if (bspatch_result != 0) {
return false;
}