[Ocfs2-tools-devel] [PATCH 1/2] ocfs2-tools: fix a double-free bug caused by ocfs2_bitmap_free
piaojun
piaojun at huawei.com
Tue Mar 1 00:48:41 PST 2016
When I am using fsck.ocfs2 to fix a gd problem in ocfs2, a double-free
bug happens. The detail of the bug is as below:
main()->
o2fsck_pass1()->
write_cluster_alloc()->
ocfs2_load_chain_allocator()->
ocfs2_bitmap_read()->ocfs2_bitmap_free()->
ocfs2_free_cached_inode()
In ocfs2_load_chain_allocator(), ocfs2_bitmap_read() fails and goes
into ocfs2_bitmap_free(). But in ocfs2_bitmap_free(), the
cinode->ci_chains is freed but not assigned to NULL. Then
ocfs2_load_chain_allocator() fails and goto out to
ocfs2_free_cached_inode. At last, cinode->ci_chains is freed twice.
So the argument passed into ocfs2_bitmap_free should be assigned to
NULL by ocfs2_free rather than the parameter bitmap.
Signed-off-by: Jun Piao <piaojun at huawei.com>
---
extras/find_dup_extents.c | 4 ++--
fsck.ocfs2/fsck.c | 8 ++++----
fsck.ocfs2/icount.c | 2 +-
fsck.ocfs2/journal.c | 2 +-
fsck.ocfs2/pass0.c | 4 ++--
include/ocfs2/ocfs2.h | 2 +-
libocfs2/bitmap.c | 20 ++++++++++----------
libocfs2/cached_inode.c | 4 ++--
libocfs2/chainalloc.c | 4 ++--
9 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/extras/find_dup_extents.c b/extras/find_dup_extents.c
index 1f8fdcc..f33d8ed 100644
--- a/extras/find_dup_extents.c
+++ b/extras/find_dup_extents.c
@@ -270,8 +270,8 @@ int main(int argc, char *argv[])
if (!ret && we.has_dups)
ret = run_scan(&we, 1);
- ocfs2_bitmap_free(we.extent_map);
- ocfs2_bitmap_free(we.dup_map);
+ ocfs2_bitmap_free(&we.extent_map);
+ ocfs2_bitmap_free(&we.dup_map);
out_close:
ret = ocfs2_close(fs);
diff --git a/fsck.ocfs2/fsck.c b/fsck.ocfs2/fsck.c
index 4b7d4b3..561787f 100644
--- a/fsck.ocfs2/fsck.c
+++ b/fsck.ocfs2/fsck.c
@@ -202,17 +202,17 @@ errcode_t o2fsck_state_reinit(ocfs2_filesys *fs,
o2fsck_state *ost)
{
errcode_t ret;
- ocfs2_bitmap_free(ost->ost_dir_inodes);
+ ocfs2_bitmap_free(&ost->ost_dir_inodes);
ost->ost_dir_inodes = NULL;
- ocfs2_bitmap_free(ost->ost_reg_inodes);
+ ocfs2_bitmap_free(&ost->ost_reg_inodes);
ost->ost_reg_inodes = NULL;
- ocfs2_bitmap_free(ost->ost_allocated_clusters);
+ ocfs2_bitmap_free(&ost->ost_allocated_clusters);
ost->ost_allocated_clusters = NULL;
if (ost->ost_duplicate_clusters) {
- ocfs2_bitmap_free(ost->ost_duplicate_clusters);
+ ocfs2_bitmap_free(&ost->ost_duplicate_clusters);
ost->ost_duplicate_clusters = NULL;
}
diff --git a/fsck.ocfs2/icount.c b/fsck.ocfs2/icount.c
index 1fa7aab..68781cd 100644
--- a/fsck.ocfs2/icount.c
+++ b/fsck.ocfs2/icount.c
@@ -232,7 +232,7 @@ void o2fsck_icount_free(o2fsck_icount *icount)
struct rb_node *node;
icount_node *in;
- ocfs2_bitmap_free(icount->ic_single_bm);
+ ocfs2_bitmap_free(&icount->ic_single_bm);
while((node = rb_first(&icount->ic_multiple_tree)) != NULL) {
in = rb_entry(node, icount_node, in_node);
rb_erase(node, &icount->ic_multiple_tree);
diff --git a/fsck.ocfs2/journal.c b/fsck.ocfs2/journal.c
index ff47633..bba8499 100644
--- a/fsck.ocfs2/journal.c
+++ b/fsck.ocfs2/journal.c
@@ -722,7 +722,7 @@ out:
if (buf)
ocfs2_free(&buf);
if (used_blocks)
- ocfs2_bitmap_free(used_blocks);
+ ocfs2_bitmap_free(&used_blocks);
return ret;
}
diff --git a/fsck.ocfs2/pass0.c b/fsck.ocfs2/pass0.c
index fd1848b..3534214 100644
--- a/fsck.ocfs2/pass0.c
+++ b/fsck.ocfs2/pass0.c
@@ -1293,9 +1293,9 @@ static errcode_t verify_bitmap_descs(o2fsck_state
*ost,
out:
if (allowed)
- ocfs2_bitmap_free(allowed);
+ ocfs2_bitmap_free(&allowed);
if (forbidden)
- ocfs2_bitmap_free(forbidden);
+ ocfs2_bitmap_free(&forbidden);
return ret;
}
diff --git a/include/ocfs2/ocfs2.h b/include/ocfs2/ocfs2.h
index d33a28d..5a05cc3 100644
--- a/include/ocfs2/ocfs2.h
+++ b/include/ocfs2/ocfs2.h
@@ -626,7 +626,7 @@ errcode_t ocfs2_cluster_bitmap_new(ocfs2_filesys *fs,
errcode_t ocfs2_block_bitmap_new(ocfs2_filesys *fs,
const char *description,
ocfs2_bitmap **ret_bitmap);
-void ocfs2_bitmap_free(ocfs2_bitmap *bitmap);
+void ocfs2_bitmap_free(ocfs2_bitmap **p_bitmap);
errcode_t ocfs2_bitmap_set(ocfs2_bitmap *bitmap, uint64_t bitno,
int *oldval);
errcode_t ocfs2_bitmap_clear(ocfs2_bitmap *bitmap, uint64_t bitno,
diff --git a/libocfs2/bitmap.c b/libocfs2/bitmap.c
index e26bebb..9c2104e 100644
--- a/libocfs2/bitmap.c
+++ b/libocfs2/bitmap.c
@@ -37,7 +37,7 @@
/* The public API */
-void ocfs2_bitmap_free(ocfs2_bitmap *bitmap)
+void ocfs2_bitmap_free(ocfs2_bitmap **bitmap)
{
struct rb_node *node;
struct ocfs2_bitmap_region *br;
@@ -47,18 +47,18 @@ void ocfs2_bitmap_free(ocfs2_bitmap *bitmap)
* it should have done it in destroy_notify. Same with the
* private pointers.
*/
- if (bitmap->b_ops->destroy_notify)
- (*bitmap->b_ops->destroy_notify)(bitmap);
+ if ((*bitmap)->b_ops->destroy_notify)
+ (*(*bitmap)->b_ops->destroy_notify)(*bitmap);
- while ((node = rb_first(&bitmap->b_regions)) != NULL) {
+ while ((node = rb_first(&(*bitmap)->b_regions)) != NULL) {
br = rb_entry(node, struct ocfs2_bitmap_region, br_node);
- rb_erase(&br->br_node, &bitmap->b_regions);
+ rb_erase(&br->br_node, &(*bitmap)->b_regions);
ocfs2_bitmap_free_region(br);
}
- ocfs2_free(&bitmap->b_description);
- ocfs2_free(&bitmap);
+ ocfs2_free(&(*bitmap)->b_description);
+ ocfs2_free(bitmap);
}
errcode_t ocfs2_bitmap_set(ocfs2_bitmap *bitmap, uint64_t bitno,
@@ -907,14 +907,14 @@ errcode_t ocfs2_cluster_bitmap_new(ocfs2_filesys *fs,
ret = ocfs2_bitmap_alloc_region(bitmap, bitoff, 0,
alloc_bits, &br);
if (ret) {
- ocfs2_bitmap_free(bitmap);
+ ocfs2_bitmap_free(&bitmap);
return ret;
}
ret = ocfs2_bitmap_insert_region(bitmap, br);
if (ret) {
ocfs2_bitmap_free_region(br);
- ocfs2_bitmap_free(bitmap);
+ ocfs2_bitmap_free(&bitmap);
return ret;
}
@@ -1241,7 +1241,7 @@ int main(int argc, char *argv[])
run_test(bitmap);
- ocfs2_bitmap_free(bitmap);
+ ocfs2_bitmap_free(&bitmap);
out_close:
ocfs2_close(fs);
diff --git a/libocfs2/cached_inode.c b/libocfs2/cached_inode.c
index 24ba324..8b7126b 100644
--- a/libocfs2/cached_inode.c
+++ b/libocfs2/cached_inode.c
@@ -74,7 +74,7 @@ errcode_t ocfs2_free_cached_inode(ocfs2_filesys *fs,
return OCFS2_ET_INVALID_ARGUMENT;
if (cinode->ci_chains)
- ocfs2_bitmap_free(cinode->ci_chains);
+ ocfs2_bitmap_free(&cinode->ci_chains);
if (cinode->ci_inode)
ocfs2_free(&cinode->ci_inode);
@@ -106,7 +106,7 @@ errcode_t ocfs2_refresh_cached_inode(ocfs2_filesys *fs,
ocfs2_cached_inode *cinode)
{
if (cinode->ci_chains) {
- ocfs2_bitmap_free(cinode->ci_chains);
+ ocfs2_bitmap_free(&cinode->ci_chains);
cinode->ci_chains = NULL;
}
diff --git a/libocfs2/chainalloc.c b/libocfs2/chainalloc.c
index f886725..a866945 100644
--- a/libocfs2/chainalloc.c
+++ b/libocfs2/chainalloc.c
@@ -433,7 +433,7 @@ errcode_t ocfs2_load_chain_allocator(ocfs2_filesys *fs,
char name[256];
if (cinode->ci_chains)
- ocfs2_bitmap_free(cinode->ci_chains);
+ ocfs2_bitmap_free(&cinode->ci_chains);
total_bits = (uint64_t)fs->fs_clusters *
cinode->ci_inode->id2.i_chain.cl_bpc;
@@ -454,7 +454,7 @@ errcode_t ocfs2_load_chain_allocator(ocfs2_filesys *fs,
cinode, gb_blkno);
ret = ocfs2_bitmap_read(cinode->ci_chains);
if (ret) {
- ocfs2_bitmap_free(cinode->ci_chains);
+ ocfs2_bitmap_free(&cinode->ci_chains);
return ret;
}
--
1.8.4.3
More information about the Ocfs2-tools-devel
mailing list