From 7ee06ddc4038f936b0d4459d37a7d4d844fb03db Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Fri, 7 May 2021 11:38:10 -0400 Subject: dm snapshot: fix a crash when an origin has no snapshots If an origin target has no snapshots, o->split_boundary is set to 0. This causes BUG_ON(sectors <= 0) in block/bio.c:bio_split(). Fix this by initializing chunk_size, and in turn split_boundary, to rounddown_pow_of_two(UINT_MAX) -- the largest power of two that fits into "unsigned" type. Reported-by: Michael Tokarev Tested-by: Michael Tokarev Cc: stable@vger.kernel.org Signed-off-by: Mikulas Patocka Signed-off-by: Mike Snitzer --- drivers/md/dm-snap.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c index a2acb014c13a..2a51ddd840b4 100644 --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c @@ -855,12 +855,11 @@ static int dm_add_exception(void *context, chunk_t old, chunk_t new) static uint32_t __minimum_chunk_size(struct origin *o) { struct dm_snapshot *snap; - unsigned chunk_size = 0; + unsigned chunk_size = rounddown_pow_of_two(UINT_MAX); if (o) list_for_each_entry(snap, &o->snapshots, list) - chunk_size = min_not_zero(chunk_size, - snap->store->chunk_size); + chunk_size = min(chunk_size, snap->store->chunk_size); return (uint32_t) chunk_size; } -- cgit v1.2.3 From c699a0db2d62e3bbb7f0bf35c87edbc8d23e3062 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Mon, 10 May 2021 14:49:05 -0400 Subject: dm snapshot: fix crash with transient storage and zero chunk size The following commands will crash the kernel: modprobe brd rd_size=1048576 dmsetup create o --table "0 `blockdev --getsize /dev/ram0` snapshot-origin /dev/ram0" dmsetup create s --table "0 `blockdev --getsize /dev/ram0` snapshot /dev/ram0 /dev/ram1 N 0" The reason is that when we test for zero chunk size, we jump to the label bad_read_metadata without setting the "r" variable. The function snapshot_ctr destroys all the structures and then exits with "r == 0". The kernel then crashes because it falsely believes that snapshot_ctr succeeded. In order to fix the bug, we set the variable "r" to -EINVAL. Signed-off-by: Mikulas Patocka Cc: stable@vger.kernel.org Signed-off-by: Mike Snitzer --- drivers/md/dm-snap.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c index 2a51ddd840b4..b8e4d31124ea 100644 --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c @@ -1408,6 +1408,7 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv) if (!s->store->chunk_size) { ti->error = "Chunk size not set"; + r = -EINVAL; goto bad_read_metadata; } -- cgit v1.2.3 From dbae70d452a0858d62915166d93650c98fe6639c Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Wed, 12 May 2021 08:28:43 -0400 Subject: dm integrity: revert to not using discard filler when recalulating Revert the commit 7a5b96b4784454ba258e83dc7469ddbacd3aaac3 ("dm integrity: use discard support when recalculating"). There's a bug that when we write some data beyond the current recalculate boundary, the checksum will be rewritten with the discard filler later. And the data will no longer have integrity protection. There's no easy fix for this case. Also, another problematic case is if dm-integrity is used to detect bitrot (random device errors, bit flips, etc); dm-integrity should detect that even for unused sectors. With commit 7a5b96b4784 it can happen that such change is undetected (because discard filler is not a valid checksum). Signed-off-by: Mikulas Patocka Acked-by: Milan Broz Signed-off-by: Mike Snitzer --- drivers/md/dm-integrity.c | 57 ++++++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 33 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c index 781942aeddd1..6d00e619a141 100644 --- a/drivers/md/dm-integrity.c +++ b/drivers/md/dm-integrity.c @@ -2689,30 +2689,26 @@ next_chunk: if (unlikely(dm_integrity_failed(ic))) goto err; - if (!ic->discard) { - io_req.bi_op = REQ_OP_READ; - io_req.bi_op_flags = 0; - io_req.mem.type = DM_IO_VMA; - io_req.mem.ptr.addr = ic->recalc_buffer; - io_req.notify.fn = NULL; - io_req.client = ic->io; - io_loc.bdev = ic->dev->bdev; - io_loc.sector = get_data_sector(ic, area, offset); - io_loc.count = n_sectors; + io_req.bi_op = REQ_OP_READ; + io_req.bi_op_flags = 0; + io_req.mem.type = DM_IO_VMA; + io_req.mem.ptr.addr = ic->recalc_buffer; + io_req.notify.fn = NULL; + io_req.client = ic->io; + io_loc.bdev = ic->dev->bdev; + io_loc.sector = get_data_sector(ic, area, offset); + io_loc.count = n_sectors; - r = dm_io(&io_req, 1, &io_loc, NULL); - if (unlikely(r)) { - dm_integrity_io_error(ic, "reading data", r); - goto err; - } + r = dm_io(&io_req, 1, &io_loc, NULL); + if (unlikely(r)) { + dm_integrity_io_error(ic, "reading data", r); + goto err; + } - t = ic->recalc_tags; - for (i = 0; i < n_sectors; i += ic->sectors_per_block) { - integrity_sector_checksum(ic, logical_sector + i, ic->recalc_buffer + (i << SECTOR_SHIFT), t); - t += ic->tag_size; - } - } else { - t = ic->recalc_tags + (n_sectors >> ic->sb->log2_sectors_per_block) * ic->tag_size; + t = ic->recalc_tags; + for (i = 0; i < n_sectors; i += ic->sectors_per_block) { + integrity_sector_checksum(ic, logical_sector + i, ic->recalc_buffer + (i << SECTOR_SHIFT), t); + t += ic->tag_size; } metadata_block = get_metadata_sector_and_offset(ic, area, offset, &metadata_offset); @@ -4368,13 +4364,11 @@ try_smaller_buffer: goto bad; } INIT_WORK(&ic->recalc_work, integrity_recalc); - if (!ic->discard) { - ic->recalc_buffer = vmalloc(RECALC_SECTORS << SECTOR_SHIFT); - if (!ic->recalc_buffer) { - ti->error = "Cannot allocate buffer for recalculating"; - r = -ENOMEM; - goto bad; - } + ic->recalc_buffer = vmalloc(RECALC_SECTORS << SECTOR_SHIFT); + if (!ic->recalc_buffer) { + ti->error = "Cannot allocate buffer for recalculating"; + r = -ENOMEM; + goto bad; } ic->recalc_tags = kvmalloc_array(RECALC_SECTORS >> ic->sb->log2_sectors_per_block, ic->tag_size, GFP_KERNEL); @@ -4383,9 +4377,6 @@ try_smaller_buffer: r = -ENOMEM; goto bad; } - if (ic->discard) - memset(ic->recalc_tags, DISCARD_FILLER, - (RECALC_SECTORS >> ic->sb->log2_sectors_per_block) * ic->tag_size); } else { if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) { ti->error = "Recalculate can only be specified with internal_hash"; @@ -4579,7 +4570,7 @@ static void dm_integrity_dtr(struct dm_target *ti) static struct target_type integrity_target = { .name = "integrity", - .version = {1, 9, 0}, + .version = {1, 10, 0}, .module = THIS_MODULE, .features = DM_TARGET_SINGLETON | DM_TARGET_INTEGRITY, .ctr = dm_integrity_ctr, -- cgit v1.2.3 From bc8f3d4647a99468d7733039b6bc9234b6e91df4 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Tue, 11 May 2021 11:41:00 -0400 Subject: dm integrity: fix sparse warnings Use the types __le* instead of __u* to fix sparse warnings. Signed-off-by: Mikulas Patocka Signed-off-by: Mike Snitzer --- drivers/md/dm-integrity.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'drivers') diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c index 6d00e619a141..20f2510db1f6 100644 --- a/drivers/md/dm-integrity.c +++ b/drivers/md/dm-integrity.c @@ -66,14 +66,14 @@ struct superblock { __u8 magic[8]; __u8 version; __u8 log2_interleave_sectors; - __u16 integrity_tag_size; - __u32 journal_sections; - __u64 provided_data_sectors; /* userspace uses this value */ - __u32 flags; + __le16 integrity_tag_size; + __le32 journal_sections; + __le64 provided_data_sectors; /* userspace uses this value */ + __le32 flags; __u8 log2_sectors_per_block; __u8 log2_blocks_per_bitmap_bit; __u8 pad[2]; - __u64 recalc_sector; + __le64 recalc_sector; __u8 pad2[8]; __u8 salt[SALT_SIZE]; }; @@ -86,16 +86,16 @@ struct superblock { #define JOURNAL_ENTRY_ROUNDUP 8 -typedef __u64 commit_id_t; +typedef __le64 commit_id_t; #define JOURNAL_MAC_PER_SECTOR 8 struct journal_entry { union { struct { - __u32 sector_lo; - __u32 sector_hi; + __le32 sector_lo; + __le32 sector_hi; } s; - __u64 sector; + __le64 sector; } u; commit_id_t last_bytes[]; /* __u8 tag[0]; */ @@ -806,7 +806,7 @@ static void section_mac(struct dm_integrity_c *ic, unsigned section, __u8 result } if (ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_HMAC)) { - uint64_t section_le; + __le64 section_le; r = crypto_shash_update(desc, (__u8 *)&ic->sb->salt, SALT_SIZE); if (unlikely(r < 0)) { @@ -1640,7 +1640,7 @@ static void integrity_end_io(struct bio *bio) static void integrity_sector_checksum(struct dm_integrity_c *ic, sector_t sector, const char *data, char *result) { - __u64 sector_le = cpu_to_le64(sector); + __le64 sector_le = cpu_to_le64(sector); SHASH_DESC_ON_STACK(req, ic->internal_hash); int r; unsigned digest_size; @@ -3822,7 +3822,7 @@ static int create_journal(struct dm_integrity_c *ic, char **error) for (i = 0; i < ic->journal_sections; i++) { struct scatterlist sg; struct skcipher_request *section_req; - __u32 section_le = cpu_to_le32(i); + __le32 section_le = cpu_to_le32(i); memset(crypt_iv, 0x00, ivsize); memset(crypt_data, 0x00, crypt_len); -- cgit v1.2.3