diff options
author | Eric Biggers <ebiggers@google.com> | 2023-03-23 19:55:50 +0000 |
---|---|---|
committer | Eric Biggers <ebiggers@google.com> | 2023-03-23 20:23:00 +0000 |
commit | ef4507b6c885af7fb3ffd88cb631a005555f6f18 (patch) | |
tree | e29f35048362fbe231995129f9c1c43a7f1e7717 /libsparse | |
parent | ecdbbbda825bb26715881037a6f3a5aaf2f95de7 (diff) | |
download | core-ef4507b6c885af7fb3ffd88cb631a005555f6f18.tar.gz |
libsparse: fix double free after block splitting
Due to https://r.android.com/1310496, sparse_file_write() splits all
blocks larger than 64 MiB. However, the code that splits file-backed
blocks copies the pointer to the filename without duplicating the
underlying memory, causing a double free in backed_block_destroy()
later. Fix this by using strdup(). Also, as long as that is being
fixed, also check for failure.
Test: SANITIZE_HOST=address mmm external/e2fsprogs
mkdir mnt
mkfs.ext4 img 1G
sudo mount img mnt
sudo cp /dev/urandom mnt/file
sudo umount mnt
ext2simg img simg
Before this fix it gave:
==2216498==ERROR: AddressSanitizer: attempting double-free on 0x602000000090 in thread T0:
#0 0x55a52454c9a2 in free out/stage2/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/out/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3
#1 0x7ffa247c82ee in backed_block_destroy(backed_block*) system/core/libsparse/backed_block.cpp:106:5
#2 0x7ffa247c82ee in backed_block_list_destroy(backed_block_list*) system/core/libsparse/backed_block.cpp:124:7
#3 0x7ffa247cd055 in sparse_file_destroy system/core/libsparse/sparse.cpp:49:3
#4 0x55a524587b75 in main external/e2fsprogs/contrib/android/ext2simg.c:239:2
Change-Id: I4607ef5adcf6512645342beaf91aff6033414e54
Diffstat (limited to 'libsparse')
-rw-r--r-- | libsparse/backed_block.cpp | 13 |
1 files changed, 11 insertions, 2 deletions
diff --git a/libsparse/backed_block.cpp b/libsparse/backed_block.cpp index 6229e7c6e..a0d1cde9c 100644 --- a/libsparse/backed_block.cpp +++ b/libsparse/backed_block.cpp @@ -315,6 +315,10 @@ int backed_block_add_file(struct backed_block_list* bbl, const char* filename, i bb->len = len; bb->type = BACKED_BLOCK_FILE; bb->file.filename = strdup(filename); + if (!bb->file.filename) { + free(bb); + return -ENOMEM; + } bb->file.offset = offset; bb->next = nullptr; @@ -359,14 +363,17 @@ int backed_block_split(struct backed_block_list* bbl, struct backed_block* bb, new_bb->len = bb->len - max_len; new_bb->block = bb->block + max_len / bbl->block_size; new_bb->next = bb->next; - bb->next = new_bb; - bb->len = max_len; switch (bb->type) { case BACKED_BLOCK_DATA: new_bb->data.data = (char*)bb->data.data + max_len; break; case BACKED_BLOCK_FILE: + new_bb->file.filename = strdup(bb->file.filename); + if (!new_bb->file.filename) { + free(new_bb); + return -ENOMEM; + } new_bb->file.offset += max_len; break; case BACKED_BLOCK_FD: @@ -376,5 +383,7 @@ int backed_block_split(struct backed_block_list* bbl, struct backed_block* bb, break; } + bb->next = new_bb; + bb->len = max_len; return 0; } |