From 56b67a4f292f14548f4046979d46d07bcf8ba971 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Tue, 18 Apr 2017 16:51:50 -0400 Subject: dm integrity: various small changes and cleanups Some coding style changes. Fix a bug that the array test_tag has insufficient size if the digest size of internal has is bigger than the tag size. The function __fls is undefined for zero argument, this patch fixes undefined behavior if the user sets zero interleave_sectors. Fix the limit of optional arguments to 8. Don't allocate crypt_data on the stack to avoid a BUG with debug kernel. Rename all optional argument names to have underscores rather than dashes. Signed-off-by: Mikulas Patocka Signed-off-by: Mike Snitzer --- drivers/md/dm-integrity.c | 116 +++++++++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 54 deletions(-) (limited to 'drivers/md/dm-integrity.c') diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c index e26a079b41ea..95cdffbb206c 100644 --- a/drivers/md/dm-integrity.c +++ b/drivers/md/dm-integrity.c @@ -27,8 +27,8 @@ #define DEFAULT_JOURNAL_WATERMARK 50 #define DEFAULT_SYNC_MSEC 10000 #define DEFAULT_MAX_JOURNAL_SECTORS 131072 -#define MIN_INTERLEAVE_SECTORS 3 -#define MAX_INTERLEAVE_SECTORS 31 +#define MIN_LOG2_INTERLEAVE_SECTORS 3 +#define MAX_LOG2_INTERLEAVE_SECTORS 31 #define METADATA_WORKQUEUE_MAX_ACTIVE 16 /* @@ -414,7 +414,7 @@ static void page_list_location(struct dm_integrity_c *ic, unsigned section, unsi { unsigned sector; - access_journal_check(ic, section, offset, false, "access_journal"); + access_journal_check(ic, section, offset, false, "page_list_location"); sector = section * ic->journal_section_sectors + offset; @@ -1211,7 +1211,7 @@ static void integrity_metadata(struct work_struct *w) unsigned digest_size = crypto_shash_digestsize(ic->internal_hash); struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io)); char *checksums; - unsigned extra_space = digest_size > ic->tag_size ? digest_size - ic->tag_size : 0; + unsigned extra_space = unlikely(digest_size > ic->tag_size) ? digest_size - ic->tag_size : 0; char checksums_onstack[ic->tag_size + extra_space]; unsigned sectors_to_process = dio->range.n_sectors; sector_t sector = dio->range.logical_sector; @@ -1820,7 +1820,7 @@ static void do_journal_write(struct dm_integrity_c *ic, unsigned write_start, unlikely(from_replay) && #endif ic->internal_hash) { - unsigned char test_tag[ic->tag_size]; + char test_tag[max(crypto_shash_digestsize(ic->internal_hash), ic->tag_size)]; integrity_sector_checksum(ic, sec + (l - j), (char *)access_journal_data(ic, i, l), test_tag); @@ -2135,11 +2135,11 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type, arg_count += !!ic->journal_mac_alg.alg_string; DMEMIT("%s %llu %u %c %u", ic->dev->name, (unsigned long long)ic->start, ic->tag_size, ic->mode, arg_count); - DMEMIT(" journal-sectors:%u", ic->initial_sectors - SB_SECTORS); - DMEMIT(" interleave-sectors:%u", 1U << ic->sb->log2_interleave_sectors); - DMEMIT(" buffer-sectors:%u", 1U << ic->log2_buffer_sectors); - DMEMIT(" journal-watermark:%u", (unsigned)watermark_percentage); - DMEMIT(" commit-time:%u", ic->autocommit_msec); + DMEMIT(" journal_sectors:%u", ic->initial_sectors - SB_SECTORS); + DMEMIT(" interleave_sectors:%u", 1U << ic->sb->log2_interleave_sectors); + DMEMIT(" buffer_sectors:%u", 1U << ic->log2_buffer_sectors); + DMEMIT(" journal_watermark:%u", (unsigned)watermark_percentage); + DMEMIT(" commit_time:%u", ic->autocommit_msec); #define EMIT_ALG(a, n) \ do { \ @@ -2149,9 +2149,9 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type, DMEMIT(":%s", ic->a.key_string);\ } \ } while (0) - EMIT_ALG(internal_hash_alg, "internal-hash"); - EMIT_ALG(journal_crypt_alg, "journal-crypt"); - EMIT_ALG(journal_mac_alg, "journal-mac"); + EMIT_ALG(internal_hash_alg, "internal_hash"); + EMIT_ALG(journal_crypt_alg, "journal_crypt"); + EMIT_ALG(journal_mac_alg, "journal_mac"); break; } } @@ -2213,6 +2213,7 @@ static int initialize_superblock(struct dm_integrity_c *ic, unsigned journal_sec unsigned journal_sections; int test_bit; + memset(ic->sb, 0, SB_SECTORS << SECTOR_SHIFT); memcpy(ic->sb->magic, SB_MAGIC, 8); ic->sb->version = SB_VERSION; ic->sb->integrity_tag_size = cpu_to_le16(ic->tag_size); @@ -2225,9 +2226,11 @@ static int initialize_superblock(struct dm_integrity_c *ic, unsigned journal_sec journal_sections = 1; ic->sb->journal_sections = cpu_to_le32(journal_sections); + if (!interleave_sectors) + interleave_sectors = DEFAULT_INTERLEAVE_SECTORS; ic->sb->log2_interleave_sectors = __fls(interleave_sectors); - ic->sb->log2_interleave_sectors = max((__u8)MIN_INTERLEAVE_SECTORS, ic->sb->log2_interleave_sectors); - ic->sb->log2_interleave_sectors = min((__u8)MAX_INTERLEAVE_SECTORS, ic->sb->log2_interleave_sectors); + ic->sb->log2_interleave_sectors = max((__u8)MIN_LOG2_INTERLEAVE_SECTORS, ic->sb->log2_interleave_sectors); + ic->sb->log2_interleave_sectors = min((__u8)MAX_LOG2_INTERLEAVE_SECTORS, ic->sb->log2_interleave_sectors); ic->provided_data_sectors = 0; for (test_bit = fls64(ic->device_sectors) - 1; test_bit >= 3; test_bit--) { @@ -2238,7 +2241,7 @@ static int initialize_superblock(struct dm_integrity_c *ic, unsigned journal_sec ic->provided_data_sectors = prev_data_sectors; } - if (!le64_to_cpu(ic->provided_data_sectors)) + if (!ic->provided_data_sectors) return -EINVAL; ic->sb->provided_data_sectors = cpu_to_le64(ic->provided_data_sectors); @@ -2444,6 +2447,12 @@ static int create_journal(struct dm_integrity_c *ic, char **error) int r = 0; unsigned i; __u64 journal_pages, journal_desc_size, journal_tree_size; + unsigned char *crypt_data = NULL; + + ic->commit_ids[0] = cpu_to_le64(0x1111111111111111ULL); + ic->commit_ids[1] = cpu_to_le64(0x2222222222222222ULL); + ic->commit_ids[2] = cpu_to_le64(0x3333333333333333ULL); + ic->commit_ids[3] = cpu_to_le64(0x4444444444444444ULL); journal_pages = roundup((__u64)ic->journal_sections * ic->journal_section_sectors, PAGE_SIZE >> SECTOR_SHIFT) >> (PAGE_SHIFT - SECTOR_SHIFT); @@ -2541,7 +2550,13 @@ static int create_journal(struct dm_integrity_c *ic, char **error) SKCIPHER_REQUEST_ON_STACK(req, ic->journal_crypt); unsigned char iv[ivsize]; unsigned crypt_len = roundup(ivsize, blocksize); - unsigned char crypt_data[crypt_len]; + + crypt_data = kmalloc(crypt_len, GFP_KERNEL); + if (!crypt_data) { + *error = "Unable to allocate crypt data"; + r = -ENOMEM; + goto bad; + } skcipher_request_set_tfm(req, ic->journal_crypt); @@ -2630,38 +2645,38 @@ retest_commit_id: r = -ENOMEM; } bad: + kfree(crypt_data); return r; } /* - * Construct a integrity mapping: + * Construct a integrity mapping * * Arguments: * device * offset from the start of the device * tag size - * D - direct writes, J - journal writes + * D - direct writes, J - journal writes, R - recovery mode * number of optional arguments * optional arguments: - * journal-sectors - * interleave-sectors - * buffer-sectors - * journal-watermark - * commit-time - * internal-hash - * journal-crypt - * journal-mac + * journal_sectors + * interleave_sectors + * buffer_sectors + * journal_watermark + * commit_time + * internal_hash + * journal_crypt + * journal_mac */ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv) { struct dm_integrity_c *ic; char dummy; int r; - unsigned i; unsigned extra_args; struct dm_arg_set as; static struct dm_arg _args[] = { - {0, 7, "Invalid number of feature args"}, + {0, 8, "Invalid number of feature args"}, }; unsigned journal_sectors, interleave_sectors, buffer_sectors, journal_watermark, sync_msec; bool should_write_sb; @@ -2683,11 +2698,6 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv) ti->private = ic; ti->per_io_data_size = sizeof(struct dm_integrity_io); - ic->commit_ids[0] = cpu_to_le64(0x1111111111111111ULL); - ic->commit_ids[1] = cpu_to_le64(0x2222222222222222ULL); - ic->commit_ids[2] = cpu_to_le64(0x3333333333333333ULL); - ic->commit_ids[3] = cpu_to_le64(0x4444444444444444ULL); - ic->in_progress = RB_ROOT; init_waitqueue_head(&ic->endio_wait); bio_list_init(&ic->flush_bio_list); @@ -2718,7 +2728,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv) if (!strcmp(argv[3], "J") || !strcmp(argv[3], "D") || !strcmp(argv[3], "R")) ic->mode = argv[3][0]; else { - ti->error = "Invalid mode (expecting J or D)"; + ti->error = "Invalid mode (expecting J, D, R)"; r = -EINVAL; goto bad; } @@ -2746,29 +2756,29 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv) ti->error = "Not enough feature arguments"; goto bad; } - if (sscanf(opt_string, "journal-sectors:%u%c", &val, &dummy) == 1) + if (sscanf(opt_string, "journal_sectors:%u%c", &val, &dummy) == 1) journal_sectors = val; - else if (sscanf(opt_string, "interleave-sectors:%u%c", &val, &dummy) == 1) + else if (sscanf(opt_string, "interleave_sectors:%u%c", &val, &dummy) == 1) interleave_sectors = val; - else if (sscanf(opt_string, "buffer-sectors:%u%c", &val, &dummy) == 1) + else if (sscanf(opt_string, "buffer_sectors:%u%c", &val, &dummy) == 1) buffer_sectors = val; - else if (sscanf(opt_string, "journal-watermark:%u%c", &val, &dummy) == 1 && val <= 100) + else if (sscanf(opt_string, "journal_watermark:%u%c", &val, &dummy) == 1 && val <= 100) journal_watermark = val; - else if (sscanf(opt_string, "commit-time:%u%c", &val, &dummy) == 1) + else if (sscanf(opt_string, "commit_time:%u%c", &val, &dummy) == 1) sync_msec = val; - else if (!memcmp(opt_string, "internal-hash:", strlen("internal-hash:"))) { + else if (!memcmp(opt_string, "internal_hash:", strlen("internal_hash:"))) { r = get_alg_and_key(opt_string, &ic->internal_hash_alg, &ti->error, - "Invalid internal-hash argument"); + "Invalid internal_hash argument"); if (r) goto bad; - } else if (!memcmp(opt_string, "journal-crypt:", strlen("journal-crypt:"))) { + } else if (!memcmp(opt_string, "journal_crypt:", strlen("journal_crypt:"))) { r = get_alg_and_key(opt_string, &ic->journal_crypt_alg, &ti->error, - "Invalid journal-crypt argument"); + "Invalid journal_crypt argument"); if (r) goto bad; - } else if (!memcmp(opt_string, "journal-mac:", strlen("journal-mac:"))) { + } else if (!memcmp(opt_string, "journal_mac:", strlen("journal_mac:"))) { r = get_alg_and_key(opt_string, &ic->journal_mac_alg, &ti->error, - "Invalid journal-mac argument"); + "Invalid journal_mac argument"); if (r) goto bad; } else { @@ -2877,12 +2887,10 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv) should_write_sb = false; if (memcmp(ic->sb->magic, SB_MAGIC, 8)) { if (ic->mode != 'R') { - for (i = 0; i < 512; i += 8) { - if (*(__u64 *)((__u8 *)ic->sb + i)) { - r = -EINVAL; - ti->error = "The device is not initialized"; - goto bad; - } + if (memchr_inv(ic->sb, 0, SB_SECTORS << SECTOR_SHIFT)) { + r = -EINVAL; + ti->error = "The device is not initialized"; + goto bad; } } @@ -2906,8 +2914,8 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv) goto bad; } /* make sure that ti->max_io_len doesn't overflow */ - if (ic->sb->log2_interleave_sectors < MIN_INTERLEAVE_SECTORS || - ic->sb->log2_interleave_sectors > MAX_INTERLEAVE_SECTORS) { + if (ic->sb->log2_interleave_sectors < MIN_LOG2_INTERLEAVE_SECTORS || + ic->sb->log2_interleave_sectors > MAX_LOG2_INTERLEAVE_SECTORS) { r = -EINVAL; ti->error = "Invalid interleave_sectors in the superblock"; goto bad; -- cgit v1.2.3-59-g8ed1b