Skip to content

Commit

Permalink
dm integrity: conditionally disable "recalculate" feature
Browse files Browse the repository at this point in the history
commit 5c02406 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>
  • Loading branch information
Mikulas Patocka authored and Greg Kroah-Hartman committed Jan 30, 2021
1 parent 440ed9a commit 26b30d3
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
7 changes: 7 additions & 0 deletions Documentation/device-mapper/dm-integrity.txt
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ block_size:number
Supported values are 512, 1024, 2048 and 4096 bytes. If not
specified the default block size is 512 bytes.

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
tables with suspend and resume). The other arguments should not be changed
Expand Down
24 changes: 23 additions & 1 deletion drivers/md/dm-integrity.c
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ struct dm_integrity_c {

bool journal_uptodate;
bool just_formatted;
bool legacy_recalculate;

struct alg_spec internal_hash_alg;
struct alg_spec journal_crypt_alg;
Expand Down Expand Up @@ -345,6 +346,14 @@ static int dm_integrity_failed(struct dm_integrity_c *ic)
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)
{
Expand Down Expand Up @@ -2503,6 +2512,7 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type,
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)
Expand All @@ -2516,6 +2526,8 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type,
DMEMIT(" buffer_sectors:%u", 1U << ic->log2_buffer_sectors);
DMEMIT(" journal_watermark:%u", (unsigned)watermark_percentage);
DMEMIT(" commit_time:%u", ic->autocommit_msec);
if (ic->legacy_recalculate)
DMEMIT(" legacy_recalculate");

#define EMIT_ALG(a, n) \
do { \
Expand Down Expand Up @@ -3118,7 +3130,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv)
unsigned extra_args;
struct dm_arg_set as;
static const struct dm_arg _args[] = {
{0, 15, "Invalid number of feature args"},
{0, 12, "Invalid number of feature args"},
};
unsigned journal_sectors, interleave_sectors, buffer_sectors, journal_watermark, sync_msec;
bool recalculate;
Expand Down Expand Up @@ -3248,6 +3260,8 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv)
goto bad;
} else if (!strcmp(opt_string, "recalculate")) {
recalculate = true;
} else if (!strcmp(opt_string, "legacy_recalculate")) {
ic->legacy_recalculate = true;
} else {
r = -EINVAL;
ti->error = "Invalid argument";
Expand Down Expand Up @@ -3523,6 +3537,14 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv)
}
}

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)) {
Expand Down

0 comments on commit 26b30d3

Please sign in to comment.