Skip to content

Commit

Permalink
libsparse: fix double free after block splitting
Browse files Browse the repository at this point in the history
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
        Spark-Rom#1 0x7ffa247c82ee in backed_block_destroy(backed_block*) system/core/libsparse/backed_block.cpp:106:5
        Spark-Rom#2 0x7ffa247c82ee in backed_block_list_destroy(backed_block_list*) system/core/libsparse/backed_block.cpp:124:7
        Spark-Rom#3 0x7ffa247cd055 in sparse_file_destroy system/core/libsparse/sparse.cpp:49:3
        Spark-Rom#4 0x55a524587b75 in main external/e2fsprogs/contrib/android/ext2simg.c:239:2

Google: 2506666
Change-Id: I4607ef5adcf6512645342beaf91aff6033414e54
  • Loading branch information
ebiggers authored and travarilo committed May 14, 2023
1 parent bb80638 commit 2366fb7
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions libsparse/backed_block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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:
Expand All @@ -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;
}

0 comments on commit 2366fb7

Please sign in to comment.