Skip to content

Commit 8bde103

Browse files
jovanbulckMikulas Patocka
authored andcommitted
dm-integrity: fix non-constant-time tag verification
When using dm-integrity in standalone mode with a keyed hmac algorithm, integrity tags are calculated and verified internally. Using plain memcmp to compare the stored and computed tags may leak the position of the first byte mismatch through side-channel analysis, allowing to brute-force expected tags in linear time (e.g., by counting single-stepping interrupts in confidential virtual machine environments). Co-developed-by: Luca Wilke <work@luca-wilke.com> Signed-off-by: Luca Wilke <work@luca-wilke.com> Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@vger.kernel.org
1 parent 5c5d0d7 commit 8bde103

File tree

1 file changed

+22
-23
lines changed

1 file changed

+22
-23
lines changed

drivers/md/dm-integrity.c

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <linux/reboot.h>
2222
#include <crypto/hash.h>
2323
#include <crypto/skcipher.h>
24+
#include <crypto/utils.h>
2425
#include <linux/async_tx.h>
2526
#include <linux/dm-bufio.h>
2627

@@ -516,7 +517,7 @@ static int sb_mac(struct dm_integrity_c *ic, bool wr)
516517
dm_integrity_io_error(ic, "crypto_shash_digest", r);
517518
return r;
518519
}
519-
if (memcmp(mac, actual_mac, mac_size)) {
520+
if (crypto_memneq(mac, actual_mac, mac_size)) {
520521
dm_integrity_io_error(ic, "superblock mac", -EILSEQ);
521522
dm_audit_log_target(DM_MSG_PREFIX, "mac-superblock", ic->ti, 0);
522523
return -EILSEQ;
@@ -859,7 +860,7 @@ static void rw_section_mac(struct dm_integrity_c *ic, unsigned int section, bool
859860
if (likely(wr))
860861
memcpy(&js->mac, result + (j * JOURNAL_MAC_PER_SECTOR), JOURNAL_MAC_PER_SECTOR);
861862
else {
862-
if (memcmp(&js->mac, result + (j * JOURNAL_MAC_PER_SECTOR), JOURNAL_MAC_PER_SECTOR)) {
863+
if (crypto_memneq(&js->mac, result + (j * JOURNAL_MAC_PER_SECTOR), JOURNAL_MAC_PER_SECTOR)) {
863864
dm_integrity_io_error(ic, "journal mac", -EILSEQ);
864865
dm_audit_log_target(DM_MSG_PREFIX, "mac-journal", ic->ti, 0);
865866
}
@@ -1401,10 +1402,9 @@ static bool find_newer_committed_node(struct dm_integrity_c *ic, struct journal_
14011402
static int dm_integrity_rw_tag(struct dm_integrity_c *ic, unsigned char *tag, sector_t *metadata_block,
14021403
unsigned int *metadata_offset, unsigned int total_size, int op)
14031404
{
1404-
#define MAY_BE_FILLER 1
1405-
#define MAY_BE_HASH 2
14061405
unsigned int hash_offset = 0;
1407-
unsigned int may_be = MAY_BE_HASH | (ic->discard ? MAY_BE_FILLER : 0);
1406+
unsigned char mismatch_hash = 0;
1407+
unsigned char mismatch_filler = !ic->discard;
14081408

14091409
do {
14101410
unsigned char *data, *dp;
@@ -1425,37 +1425,38 @@ static int dm_integrity_rw_tag(struct dm_integrity_c *ic, unsigned char *tag, se
14251425
if (op == TAG_READ) {
14261426
memcpy(tag, dp, to_copy);
14271427
} else if (op == TAG_WRITE) {
1428-
if (memcmp(dp, tag, to_copy)) {
1428+
if (crypto_memneq(dp, tag, to_copy)) {
14291429
memcpy(dp, tag, to_copy);
14301430
dm_bufio_mark_partial_buffer_dirty(b, *metadata_offset, *metadata_offset + to_copy);
14311431
}
14321432
} else {
14331433
/* e.g.: op == TAG_CMP */
14341434

14351435
if (likely(is_power_of_2(ic->tag_size))) {
1436-
if (unlikely(memcmp(dp, tag, to_copy)))
1437-
if (unlikely(!ic->discard) ||
1438-
unlikely(memchr_inv(dp, DISCARD_FILLER, to_copy) != NULL)) {
1439-
goto thorough_test;
1440-
}
1436+
if (unlikely(crypto_memneq(dp, tag, to_copy)))
1437+
goto thorough_test;
14411438
} else {
14421439
unsigned int i, ts;
14431440
thorough_test:
14441441
ts = total_size;
14451442

14461443
for (i = 0; i < to_copy; i++, ts--) {
1447-
if (unlikely(dp[i] != tag[i]))
1448-
may_be &= ~MAY_BE_HASH;
1449-
if (likely(dp[i] != DISCARD_FILLER))
1450-
may_be &= ~MAY_BE_FILLER;
1444+
/*
1445+
* Warning: the control flow must not be
1446+
* dependent on match/mismatch of
1447+
* individual bytes.
1448+
*/
1449+
mismatch_hash |= dp[i] ^ tag[i];
1450+
mismatch_filler |= dp[i] ^ DISCARD_FILLER;
14511451
hash_offset++;
14521452
if (unlikely(hash_offset == ic->tag_size)) {
1453-
if (unlikely(!may_be)) {
1453+
if (unlikely(mismatch_hash) && unlikely(mismatch_filler)) {
14541454
dm_bufio_release(b);
14551455
return ts;
14561456
}
14571457
hash_offset = 0;
1458-
may_be = MAY_BE_HASH | (ic->discard ? MAY_BE_FILLER : 0);
1458+
mismatch_hash = 0;
1459+
mismatch_filler = !ic->discard;
14591460
}
14601461
}
14611462
}
@@ -1476,8 +1477,6 @@ static int dm_integrity_rw_tag(struct dm_integrity_c *ic, unsigned char *tag, se
14761477
} while (unlikely(total_size));
14771478

14781479
return 0;
1479-
#undef MAY_BE_FILLER
1480-
#undef MAY_BE_HASH
14811480
}
14821481

14831482
struct flush_request {
@@ -2076,7 +2075,7 @@ static bool __journal_read_write(struct dm_integrity_io *dio, struct bio *bio,
20762075
char checksums_onstack[MAX_T(size_t, HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
20772076

20782077
integrity_sector_checksum(ic, logical_sector, mem + bv.bv_offset, checksums_onstack);
2079-
if (unlikely(memcmp(checksums_onstack, journal_entry_tag(ic, je), ic->tag_size))) {
2078+
if (unlikely(crypto_memneq(checksums_onstack, journal_entry_tag(ic, je), ic->tag_size))) {
20802079
DMERR_LIMIT("Checksum failed when reading from journal, at sector 0x%llx",
20812080
logical_sector);
20822081
dm_audit_log_bio(DM_MSG_PREFIX, "journal-checksum",
@@ -2595,7 +2594,7 @@ static void dm_integrity_inline_recheck(struct work_struct *w)
25952594
bio_put(outgoing_bio);
25962595

25972596
integrity_sector_checksum(ic, dio->bio_details.bi_iter.bi_sector, outgoing_data, digest);
2598-
if (unlikely(memcmp(digest, dio->integrity_payload, min(crypto_shash_digestsize(ic->internal_hash), ic->tag_size)))) {
2597+
if (unlikely(crypto_memneq(digest, dio->integrity_payload, min(crypto_shash_digestsize(ic->internal_hash), ic->tag_size)))) {
25992598
DMERR_LIMIT("%pg: Checksum failed at sector 0x%llx",
26002599
ic->dev->bdev, dio->bio_details.bi_iter.bi_sector);
26012600
atomic64_inc(&ic->number_of_mismatches);
@@ -2634,7 +2633,7 @@ static int dm_integrity_end_io(struct dm_target *ti, struct bio *bio, blk_status
26342633
char *mem = bvec_kmap_local(&bv);
26352634
//memset(mem, 0xff, ic->sectors_per_block << SECTOR_SHIFT);
26362635
integrity_sector_checksum(ic, dio->bio_details.bi_iter.bi_sector, mem, digest);
2637-
if (unlikely(memcmp(digest, dio->integrity_payload + pos,
2636+
if (unlikely(crypto_memneq(digest, dio->integrity_payload + pos,
26382637
min(crypto_shash_digestsize(ic->internal_hash), ic->tag_size)))) {
26392638
kunmap_local(mem);
26402639
dm_integrity_free_payload(dio);
@@ -2911,7 +2910,7 @@ static void do_journal_write(struct dm_integrity_c *ic, unsigned int write_start
29112910

29122911
integrity_sector_checksum(ic, sec + ((l - j) << ic->sb->log2_sectors_per_block),
29132912
(char *)access_journal_data(ic, i, l), test_tag);
2914-
if (unlikely(memcmp(test_tag, journal_entry_tag(ic, je2), ic->tag_size))) {
2913+
if (unlikely(crypto_memneq(test_tag, journal_entry_tag(ic, je2), ic->tag_size))) {
29152914
dm_integrity_io_error(ic, "tag mismatch when replaying journal", -EILSEQ);
29162915
dm_audit_log_target(DM_MSG_PREFIX, "integrity-replay-journal", ic->ti, 0);
29172916
}

0 commit comments

Comments
 (0)