PawletOS/android_bootable_recovery
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

applypatch: Fix a memory leak in ApplyImagePatch().

Browse Source 
$ 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
This commit is contained in:
Tao Bao
2017-10-24 12:25:41 -07:00
parent d830858952
commit fdec103335
1 changed files with 22 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
applypatch/imgpatch.cpp
View File

@@ -59,13 +59,13 @@ static bool ApplyBSDiffPatchAndStreamOutput(const uint8_t* src_data, size_t src_
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 @@ static bool ApplyBSDiffPatchAndStreamOutput(const uint8_t* src_data, size_t src_
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 @@ static bool ApplyBSDiffPatchAndStreamOutput(const uint8_t* src_data, size_t src_
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;
}