| From 5c02406428d5219c367c5f53457698c58bc5f917 Mon Sep 17 00:00:00 2001 |
| From: Mikulas Patocka <mpatocka@redhat.com> |
| Date: Wed, 20 Jan 2021 13:59:11 -0500 |
| Subject: dm integrity: conditionally disable "recalculate" feature |
| |
| From: Mikulas Patocka <mpatocka@redhat.com> |
| |
| commit 5c02406428d5219c367c5f53457698c58bc5f917 upstream. |
| |
| Otherwise a malicious user could (ab)use the "recalculate" feature |
| that makes dm-integrity calculate the checksums in the background |
| while the device is already usable. When the system restarts before all |
| checksums have been calculated, the calculation continues where it was |
| interrupted even if the recalculate feature is not requested the next |
| time the dm device is set up. |
| |
| Disable recalculating if we use internal_hash or journal_hash with a |
| key (e.g. HMAC) and we don't have the "legacy_recalculate" flag. |
| |
| This may break activation of a volume, created by an older kernel, |
| that is not yet fully recalculated -- if this happens, the user should |
| add the "legacy_recalculate" flag to constructor parameters. |
| |
| Cc: stable@vger.kernel.org |
| Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> |
| Reported-by: Daniel Glockner <dg@emlix.com> |
| Signed-off-by: Mike Snitzer <snitzer@redhat.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| Documentation/admin-guide/device-mapper/dm-integrity.rst | 6 +++ |
| drivers/md/dm-integrity.c | 24 ++++++++++++++- |
| 2 files changed, 29 insertions(+), 1 deletion(-) |
| |
| --- a/Documentation/admin-guide/device-mapper/dm-integrity.rst |
| +++ b/Documentation/admin-guide/device-mapper/dm-integrity.rst |
| @@ -177,6 +177,12 @@ bitmap_flush_interval:number |
| The bitmap flush interval in milliseconds. The metadata buffers |
| are synchronized when this interval expires. |
| |
| +legacy_recalculate |
| + Allow recalculating of volumes with HMAC keys. This is disabled by |
| + default for security reasons - an attacker could modify the volume, |
| + set recalc_sector to zero, and the kernel would not detect the |
| + modification. |
| + |
| |
| The journal mode (D/J), buffer_sectors, journal_watermark, commit_time can |
| be changed when reloading the target (load an inactive table and swap the |
| --- a/drivers/md/dm-integrity.c |
| +++ b/drivers/md/dm-integrity.c |
| @@ -254,6 +254,7 @@ struct dm_integrity_c { |
| bool journal_uptodate; |
| bool just_formatted; |
| bool recalculate_flag; |
| + bool legacy_recalculate; |
| |
| struct alg_spec internal_hash_alg; |
| struct alg_spec journal_crypt_alg; |
| @@ -381,6 +382,14 @@ static int dm_integrity_failed(struct dm |
| return READ_ONCE(ic->failed); |
| } |
| |
| +static bool dm_integrity_disable_recalculate(struct dm_integrity_c *ic) |
| +{ |
| + if ((ic->internal_hash_alg.key || ic->journal_mac_alg.key) && |
| + !ic->legacy_recalculate) |
| + return true; |
| + return false; |
| +} |
| + |
| static commit_id_t dm_integrity_commit_id(struct dm_integrity_c *ic, unsigned i, |
| unsigned j, unsigned char seq) |
| { |
| @@ -2998,6 +3007,7 @@ static void dm_integrity_status(struct d |
| arg_count += !!ic->internal_hash_alg.alg_string; |
| arg_count += !!ic->journal_crypt_alg.alg_string; |
| arg_count += !!ic->journal_mac_alg.alg_string; |
| + arg_count += ic->legacy_recalculate; |
| DMEMIT("%s %llu %u %c %u", ic->dev->name, (unsigned long long)ic->start, |
| ic->tag_size, ic->mode, arg_count); |
| if (ic->meta_dev) |
| @@ -3017,6 +3027,8 @@ static void dm_integrity_status(struct d |
| DMEMIT(" sectors_per_bit:%llu", (unsigned long long)ic->sectors_per_block << ic->log2_blocks_per_bitmap_bit); |
| DMEMIT(" bitmap_flush_interval:%u", jiffies_to_msecs(ic->bitmap_flush_interval)); |
| } |
| + if (ic->legacy_recalculate) |
| + DMEMIT(" legacy_recalculate"); |
| |
| #define EMIT_ALG(a, n) \ |
| do { \ |
| @@ -3625,7 +3637,7 @@ static int dm_integrity_ctr(struct dm_ta |
| unsigned extra_args; |
| struct dm_arg_set as; |
| static const struct dm_arg _args[] = { |
| - {0, 15, "Invalid number of feature args"}, |
| + {0, 14, "Invalid number of feature args"}, |
| }; |
| unsigned journal_sectors, interleave_sectors, buffer_sectors, journal_watermark, sync_msec; |
| bool should_write_sb; |
| @@ -3769,6 +3781,8 @@ static int dm_integrity_ctr(struct dm_ta |
| goto bad; |
| } else if (!strcmp(opt_string, "recalculate")) { |
| ic->recalculate_flag = true; |
| + } else if (!strcmp(opt_string, "legacy_recalculate")) { |
| + ic->legacy_recalculate = true; |
| } else { |
| r = -EINVAL; |
| ti->error = "Invalid argument"; |
| @@ -4067,6 +4081,14 @@ try_smaller_buffer: |
| } |
| } |
| |
| + if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING) && |
| + le64_to_cpu(ic->sb->recalc_sector) < ic->provided_data_sectors && |
| + dm_integrity_disable_recalculate(ic)) { |
| + ti->error = "Recalculating with HMAC is disabled for security reasons - if you really need it, use the argument \"legacy_recalculate\""; |
| + r = -EOPNOTSUPP; |
| + goto bad; |
| + } |
| + |
| ic->bufio = dm_bufio_client_create(ic->meta_dev ? ic->meta_dev->bdev : ic->dev->bdev, |
| 1U << (SECTOR_SHIFT + ic->log2_buffer_sectors), 1, 0, NULL, NULL); |
| if (IS_ERR(ic->bufio)) { |