lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250717140728.3221243-1-lilingfeng3@huawei.com>
Date: Thu, 17 Jul 2025 22:07:28 +0800
From: Li Lingfeng <lilingfeng3@...wei.com>
To: <dm-devel@...ts.linux.dev>
CC: <agk@...hat.com>, <snitzer@...nel.org>, <mpatocka@...hat.com>,
	<tusharsu@...ux.microsoft.com>, <colin.i.king@...il.com>,
	<linux-kernel@...r.kernel.org>, <houtao1@...wei.com>, <yi.zhang@...wei.com>,
	<yangerkun@...wei.com>, <yukuai3@...wei.com>, <chengzhihao1@...wei.com>,
	<lilingfeng3@...wei.com>, <lilingfeng@...weicloud.com>
Subject: [PATCH] dm: introduce spinlocks to prevent concurrent usage and release of device_metadata and hash

There is a window between freeing md->ima.active_table.hash and setting
md->ima.active_table.hash to NULL in dm_ima_measure_on_device_resume().
If dm_ima_measure_on_device_remove() accesses md->ima.active_table.hash
concurrently during this window, it could lead to a double free or UAF,
as shown below:

BUG: KASAN: slab-use-after-free in dm_ima_measure_on_device_remove+0x3fc/0x6c0
Read of size 71 at addr ffff88817bb9e220 by task dmsetup/2303

CPU: 2 UID: 0 PID: 2303 Comm: dmsetup Not tainted 6.16.0-rc6-dirty #7 PREEMPT(none)
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0x5b/0x80
 print_address_description.constprop.0+0x88/0x310
 print_report+0x12f/0x21d
 kasan_report+0xcc/0x190
 kasan_check_range+0x104/0x1b0
 __asan_memcpy+0x23/0x60
 dm_ima_measure_on_device_remove+0x3fc/0x6c0
 dev_remove+0x123/0x1e0
 ctl_ioctl+0x2a2/0x480
 dm_ctl_ioctl+0xe/0x20
 __x64_sys_ioctl+0xc7/0x110
 do_syscall_64+0x72/0x390
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

To reproduce this issue, add a delay between freeing
md->ima.active_table.hash and setting it to NULL, using the following
steps:
dmsetup create mydevice --table "0 2097152 linear /dev/sda 0"
dmsetup suspend mydevice
dmsetup reload mydevice --table "0 2097152 linear /dev/sdb 0"
dmsetup resume mydevice &
dmsetup remove mydevice

The device_metadata also appears to have a similar issue.
Add spinlocks for hash and device_metadata to prevent concurrent
usage and release.

Fixes: 91ccbbac1747 ("dm ima: measure data on table load")
Signed-off-by: Li Lingfeng <lilingfeng3@...wei.com>
---
 drivers/md/dm-ima.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/md/dm-ima.h |  2 ++
 2 files changed, 47 insertions(+)

diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index b90f34259fbb..ff2ac54addb8 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -170,6 +170,10 @@ void dm_ima_reset_data(struct mapped_device *md)
 {
 	memset(&(md->ima), 0, sizeof(md->ima));
 	md->ima.dm_version_str_len = strlen(DM_IMA_VERSION_STR);
+	spin_lock_init(&md->ima.active_table.device_metadata_lock);
+	spin_lock_init(&md->ima.inactive_table.device_metadata_lock);
+	spin_lock_init(&md->ima.active_table.hash_lock);
+	spin_lock_init(&md->ima.inactive_table.hash_lock);
 }
 
 /*
@@ -336,19 +340,24 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
 	for (i = 0; i < digest_size; i++)
 		snprintf((digest_buf + hash_alg_prefix_len + (i*2)), 3, "%02x", digest[i]);
 
+	spin_lock(&table->md->ima.inactive_table.hash_lock);
 	if (table->md->ima.active_table.hash != table->md->ima.inactive_table.hash)
 		kfree(table->md->ima.inactive_table.hash);
 
 	table->md->ima.inactive_table.hash = digest_buf;
 	table->md->ima.inactive_table.hash_len = strlen(digest_buf);
+	spin_unlock(&table->md->ima.inactive_table.hash_lock);
+
 	table->md->ima.inactive_table.num_targets = num_targets;
 
+	spin_lock(&table->md->ima.inactive_table.device_metadata_lock);
 	if (table->md->ima.active_table.device_metadata !=
 	    table->md->ima.inactive_table.device_metadata)
 		kfree(table->md->ima.inactive_table.device_metadata);
 
 	table->md->ima.inactive_table.device_metadata = device_data_buf;
 	table->md->ima.inactive_table.device_metadata_len = device_data_buf_len;
+	spin_unlock(&table->md->ima.inactive_table.device_metadata_lock);
 
 	goto exit;
 error:
@@ -388,18 +397,22 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
 	l += md->ima.dm_version_str_len;
 
 	if (swap) {
+		spin_lock(&md->ima.active_table.hash_lock);
 		if (md->ima.active_table.hash != md->ima.inactive_table.hash)
 			kfree(md->ima.active_table.hash);
 
 		md->ima.active_table.hash = NULL;
 		md->ima.active_table.hash_len = 0;
+		spin_unlock(&md->ima.active_table.hash_lock);
 
+		spin_lock(&md->ima.active_table.device_metadata_lock);
 		if (md->ima.active_table.device_metadata !=
 		    md->ima.inactive_table.device_metadata)
 			kfree(md->ima.active_table.device_metadata);
 
 		md->ima.active_table.device_metadata = NULL;
 		md->ima.active_table.device_metadata_len = 0;
+		spin_unlock(&md->ima.active_table.device_metadata_lock);
 		md->ima.active_table.num_targets = 0;
 
 		if (md->ima.inactive_table.hash) {
@@ -429,6 +442,7 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
 		nodata = false;
 	}
 
+	spin_lock(&md->ima.active_table.hash_lock);
 	if (md->ima.active_table.hash) {
 		memcpy(device_table_data + l, active, active_len);
 		l += active_len;
@@ -442,6 +456,7 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
 
 		nodata = false;
 	}
+	spin_unlock(&md->ima.active_table.hash_lock);
 
 	if (nodata) {
 		r = dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio);
@@ -503,6 +518,7 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
 	memcpy(device_table_data + l, DM_IMA_VERSION_STR, md->ima.dm_version_str_len);
 	l += md->ima.dm_version_str_len;
 
+	spin_lock(&md->ima.active_table.device_metadata_lock);
 	if (md->ima.active_table.device_metadata) {
 		memcpy(device_table_data + l, device_active_str, device_active_len);
 		l += device_active_len;
@@ -513,7 +529,9 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
 
 		nodata = false;
 	}
+	spin_unlock(&md->ima.active_table.device_metadata_lock);
 
+	spin_lock(&md->ima.inactive_table.device_metadata_lock);
 	if (md->ima.inactive_table.device_metadata) {
 		memcpy(device_table_data + l, device_inactive_str, device_inactive_len);
 		l += device_inactive_len;
@@ -524,7 +542,9 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
 
 		nodata = false;
 	}
+	spin_unlock(&md->ima.inactive_table.device_metadata_lock);
 
+	spin_lock(&md->ima.active_table.hash_lock);
 	if (md->ima.active_table.hash) {
 		memcpy(device_table_data + l, active_table_str, active_table_len);
 		l += active_table_len;
@@ -538,7 +558,9 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
 
 		nodata = false;
 	}
+	spin_unlock(&md->ima.active_table.hash_lock);
 
+	spin_lock(&md->ima.inactive_table.hash_lock);
 	if (md->ima.inactive_table.hash) {
 		memcpy(device_table_data + l, inactive_table_str, inactive_table_len);
 		l += inactive_table_len;
@@ -552,6 +574,7 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
 
 		nodata = false;
 	}
+	spin_unlock(&md->ima.inactive_table.hash_lock);
 	/*
 	 * In case both active and inactive tables, and corresponding
 	 * device metadata is cleared/missing - record the name and uuid
@@ -582,17 +605,31 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
 	kfree(device_table_data);
 	kfree(capacity_str);
 exit:
+	spin_lock(&md->ima.active_table.device_metadata_lock);
+	spin_lock(&md->ima.inactive_table.device_metadata_lock);
 	kfree(md->ima.active_table.device_metadata);
 
 	if (md->ima.active_table.device_metadata !=
 	    md->ima.inactive_table.device_metadata)
 		kfree(md->ima.inactive_table.device_metadata);
 
+	md->ima.active_table.device_metadata = NULL;
+	md->ima.inactive_table.device_metadata = NULL;
+	spin_unlock(&md->ima.inactive_table.device_metadata_lock);
+	spin_unlock(&md->ima.active_table.device_metadata_lock);
+
+	spin_lock(&md->ima.active_table.hash_lock);
+	spin_lock(&md->ima.inactive_table.hash_lock);
 	kfree(md->ima.active_table.hash);
 
 	if (md->ima.active_table.hash != md->ima.inactive_table.hash)
 		kfree(md->ima.inactive_table.hash);
 
+	md->ima.active_table.hash = NULL;
+	md->ima.inactive_table.hash = NULL;
+	spin_unlock(&md->ima.inactive_table.hash_lock);
+	spin_unlock(&md->ima.active_table.hash_lock);
+
 	dm_ima_reset_data(md);
 
 	kfree(dev_name);
@@ -623,6 +660,8 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map)
 	memcpy(device_table_data + l, DM_IMA_VERSION_STR, md->ima.dm_version_str_len);
 	l += md->ima.dm_version_str_len;
 
+	spin_lock(&md->ima.inactive_table.hash_lock);
+	spin_lock(&md->ima.inactive_table.device_metadata_lock);
 	if (md->ima.inactive_table.device_metadata_len &&
 	    md->ima.inactive_table.hash_len) {
 		memcpy(device_table_data + l, md->ima.inactive_table.device_metadata,
@@ -642,6 +681,8 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map)
 
 		nodata = false;
 	}
+	spin_unlock(&md->ima.inactive_table.device_metadata_lock);
+	spin_unlock(&md->ima.inactive_table.hash_lock);
 
 	if (nodata) {
 		if (dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio))
@@ -660,19 +701,23 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map)
 	dm_ima_measure_data("dm_table_clear", device_table_data, l, noio);
 
 	if (new_map) {
+		spin_lock(&md->ima.inactive_table.hash_lock);
 		if (md->ima.inactive_table.hash &&
 		    md->ima.inactive_table.hash != md->ima.active_table.hash)
 			kfree(md->ima.inactive_table.hash);
 
 		md->ima.inactive_table.hash = NULL;
 		md->ima.inactive_table.hash_len = 0;
+		spin_unlock(&md->ima.inactive_table.hash_lock);
 
+		spin_lock(&md->ima.inactive_table.device_metadata_lock);
 		if (md->ima.inactive_table.device_metadata &&
 		    md->ima.inactive_table.device_metadata != md->ima.active_table.device_metadata)
 			kfree(md->ima.inactive_table.device_metadata);
 
 		md->ima.inactive_table.device_metadata = NULL;
 		md->ima.inactive_table.device_metadata_len = 0;
+		spin_unlock(&md->ima.inactive_table.device_metadata_lock);
 		md->ima.inactive_table.num_targets = 0;
 
 		if (md->ima.active_table.hash) {
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index 568870a1a145..a021056f4ab5 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -35,6 +35,7 @@ struct dm_ima_device_table_metadata {
 	 * pairs delimited by a semicolon at the end of the list.
 	 */
 	char *device_metadata;
+	spinlock_t device_metadata_lock;
 	unsigned int device_metadata_len;
 	unsigned int num_targets;
 
@@ -43,6 +44,7 @@ struct dm_ima_device_table_metadata {
 	 * attributes' key-value pairs from the active/inactive tables.
 	 */
 	char *hash;
+	spinlock_t hash_lock;
 	unsigned int hash_len;
 };
 
-- 
2.46.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ