LU-8465 e2fsck: fix race in ext2fs_read_bitmaps()

During corruption testing hiting following segfault:

Multiple threads triggered to read bitmaps
Signal (11) SIGSEGV si_code=SEGV_MAPERR fault addr=0x200
./e2fsck[0x4382ae]
/lib64/libpthread.so.0(+0x14b20)[0x7f5854d2fb20]
./e2fsck(ext2fs_rb_insert_color+0xc)[0x46ac0c]
./e2fsck[0x467bb4]
./e2fsck[0x467e6d]
./e2fsck[0x45ba95]
./e2fsck[0x45c124]
/lib64/libpthread.so.0(+0x94e2)[0x7f5854d244e2]
/lib64/libc.so.6(clone+0x43)[0x7f5854beb6c3]

Problem is @block_map might be set NULL if one of
thread exit, move such kind of cleanup operation
to main thread after all threads exit.

Another potential problem is e2fsck_read_bitmap()
could be called during pass1, this need be serialized,
serialize it in the pass1.

Signed-off-by: Wang Shilong <wshilong@ddn.com>
Change-Id: I6cf2f4a11f3cf04bfa70d1e1f8b97fbcc33b79dc
Reviewed-on: https://review.whamcloud.com/40065
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 1402b5d..aa40218 100644 (file)
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -4483,9 +4483,9 @@ report_problem:
                                        }
                                        continue;
                                }
+                               e2fsck_pass1_fix_lock(ctx);
                                e2fsck_read_bitmaps(ctx);
                                pb->inode_modified = 1;
-                               e2fsck_pass1_fix_lock(ctx);
                                pctx->errcode =
                                        ext2fs_extent_delete(ehandle, 0);
                                e2fsck_pass1_fix_unlock(ctx);

diff --git a/lib/ext2fs/rw_bitmaps.c b/lib/ext2fs/rw_bitmaps.c
index 95de9b1..eb79120 100644 (file)
--- a/lib/ext2fs/rw_bitmaps.c
+++ b/lib/ext2fs/rw_bitmaps.c
@@ -445,14 +445,6 @@ success_cleanup:
        return 0;
 
 cleanup:
-       if (do_block) {
-               ext2fs_free_block_bitmap(fs->block_map);
-               fs->block_map = 0;
-       }
-       if (do_inode) {
-               ext2fs_free_inode_bitmap(fs->inode_map);
-               fs->inode_map = 0;
-       }
        if (inode_bitmap)
                ext2fs_free_mem(&inode_bitmap);
        if (block_bitmap)
@@ -463,9 +455,12 @@ cleanup:
 
 }
 
-static errcode_t read_bitmaps_range_end(ext2_filsys fs, int do_inode, int do_block)
+static errcode_t read_bitmaps_range_end(ext2_filsys fs, int do_inode, int do_block,
+                                       errcode_t retval)
 {
-       errcode_t retval = 0;
+
+       if (retval)
+               goto cleanup;
 
        /* Mark group blocks for any BLOCK_UNINIT groups */
        if (do_block) {
@@ -474,7 +469,7 @@ static errcode_t read_bitmaps_range_end(ext2_filsys fs, int do_inode, int do_blo
                        goto cleanup;
        }
 
-       return retval;
+       return 0;
 cleanup:
        if (do_block) {
                ext2fs_free_block_bitmap(fs->block_map);
@@ -497,10 +492,8 @@ static errcode_t read_bitmaps_range(ext2_filsys fs, int do_inode, int do_block,
                return retval;
 
        retval = read_bitmaps_range_start(fs, do_inode, do_block, start, end, NULL, NULL);
-       if (retval)
-               return retval;
 
-       return read_bitmaps_range_end(fs, do_inode, do_block);
+       return read_bitmaps_range_end(fs, do_inode, do_block, retval);
 }
 
 #ifdef CONFIG_PFSCK
@@ -636,9 +629,7 @@ out:
        free(thread_infos);
        free(thread_ids);
 
-       if (!retval)
-               retval = read_bitmaps_range_end(fs, do_inode, do_block);
-
+       retval = read_bitmaps_range_end(fs, do_inode, do_block, retval);
        if (!retval) {
                if (do_inode)
                        fs->flags &= ~EXT2_FLAG_IBITMAP_TAIL_PROBLEM;