Skip to content

Commit f2893c0

Browse files
mingnusMike Snitzer
authored andcommitted
dm array: fix releasing a faulty array block twice in dm_array_cursor_end
When dm_bm_read_lock() fails due to locking or checksum errors, it releases the faulty block implicitly while leaving an invalid output pointer behind. The caller of dm_bm_read_lock() should not operate on this invalid dm_block pointer, or it will lead to undefined result. For example, the dm_array_cursor incorrectly caches the invalid pointer on reading a faulty array block, causing a double release in dm_array_cursor_end(), then hitting the BUG_ON in dm-bufio cache_put(). Reproduce steps: 1. initialize a cache device dmsetup create cmeta --table "0 8192 linear /dev/sdc 0" dmsetup create cdata --table "0 65536 linear /dev/sdc 8192" dmsetup create corig --table "0 524288 linear /dev/sdc $262144" dd if=/dev/zero of=/dev/mapper/cmeta bs=4k count=1 dmsetup create cache --table "0 524288 cache /dev/mapper/cmeta \ /dev/mapper/cdata /dev/mapper/corig 128 2 metadata2 writethrough smq 0" 2. wipe the second array block offline dmsteup remove cache cmeta cdata corig mapping_root=$(dd if=/dev/sdc bs=1c count=8 skip=192 \ 2>/dev/null | hexdump -e '1/8 "%u\n"') ablock=$(dd if=/dev/sdc bs=1c count=8 skip=$((4096*mapping_root+2056)) \ 2>/dev/null | hexdump -e '1/8 "%u\n"') dd if=/dev/zero of=/dev/sdc bs=4k count=1 seek=$ablock 3. try reopen the cache device dmsetup create cmeta --table "0 8192 linear /dev/sdc 0" dmsetup create cdata --table "0 65536 linear /dev/sdc 8192" dmsetup create corig --table "0 524288 linear /dev/sdc $262144" dmsetup create cache --table "0 524288 cache /dev/mapper/cmeta \ /dev/mapper/cdata /dev/mapper/corig 128 2 metadata2 writethrough smq 0" Kernel logs: (snip) device-mapper: array: array_block_check failed: blocknr 0 != wanted 10 device-mapper: block manager: array validator check failed for block 10 device-mapper: array: get_ablock failed device-mapper: cache metadata: dm_array_cursor_next for mapping failed ------------[ cut here ]------------ kernel BUG at drivers/md/dm-bufio.c:638! Fix by setting the cached block pointer to NULL on errors. In addition to the reproducer described above, this fix can be verified using the "array_cursor/damaged" test in dm-unit: dm-unit run /pdata/array_cursor/damaged --kernel-dir <KERNEL_DIR> Signed-off-by: Ming-Hung Tsai <mtsai@redhat.com> Fixes: fdd1315 ("dm array: introduce cursor api") Reviewed-by: Joe Thornber <thornber@redhat.com> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
1 parent a573e40 commit f2893c0

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

drivers/md/persistent-data/dm-array.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -917,23 +917,27 @@ static int load_ablock(struct dm_array_cursor *c)
917917
if (c->block)
918918
unlock_ablock(c->info, c->block);
919919

920-
c->block = NULL;
921-
c->ab = NULL;
922920
c->index = 0;
923921

924922
r = dm_btree_cursor_get_value(&c->cursor, &key, &value_le);
925923
if (r) {
926924
DMERR("dm_btree_cursor_get_value failed");
927-
dm_btree_cursor_end(&c->cursor);
925+
goto out;
928926

929927
} else {
930928
r = get_ablock(c->info, le64_to_cpu(value_le), &c->block, &c->ab);
931929
if (r) {
932930
DMERR("get_ablock failed");
933-
dm_btree_cursor_end(&c->cursor);
931+
goto out;
934932
}
935933
}
936934

935+
return 0;
936+
937+
out:
938+
dm_btree_cursor_end(&c->cursor);
939+
c->block = NULL;
940+
c->ab = NULL;
937941
return r;
938942
}
939943

0 commit comments

Comments
 (0)