* [PATCH] tpm_tis: Check for an error after exhausting send retires
From: Jacqueline Wong @ 2026-04-10 21:13 UTC (permalink / raw)
To: peterhuewe, jarkko
Cc: jgg, Alexander.Steffen, linux-integrity, linux-kernel,
axelrasmussen, Jordan Hand
From: Jordan Hand <jhand@google.com>
tpm_tis_send_main() will attempt to retry sending data TPM_RETRY times.
Currently, if those retries are exhausted, the driver will attempt to
call execute. The TPM will be in the wrong state, leading to the
operation simply timing out.
Instead, if there is still an error after retries are exhausted, return
that error immediately.
Additionally, add logging to more easily determine reason for transmit
failure.
Fixes: 280db21e153d8 ("tpm_tis: Resend command to recover from data transfer errors")
Signed-off-by: Jordan Hand <jhand@google.com>
---
drivers/char/tpm/tpm_tis_core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index e2a1769081b1..b78937841879 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -471,6 +471,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
status = tpm_tis_status(chip);
if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
rc = -EIO;
+ dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be set. sts = 0x%08x\n",
+ status);
goto out_err;
}
}
@@ -491,6 +493,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
status = tpm_tis_status(chip);
if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) {
rc = -EIO;
+ dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be unset. sts = 0x%08x\n",
+ status);
goto out_err;
}
@@ -552,11 +556,16 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
break;
else if (rc != -EAGAIN && rc != -EIO)
/* Data transfer failed, not recoverable */
- return rc;
+ goto out_err;
usleep_range(priv->timeout_min, priv->timeout_max);
}
+ if (rc == -EAGAIN || rc == -EIO) {
+ dev_err(&chip->dev, "Exhausted tpm_tis_send_data retries\n");
+ goto out_err;
+ }
+
/* go and do it */
rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);
if (rc < 0)
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* Re: [PATCH] tpm_tis: Check for an error after exhausting send retires
From: Paul Menzel @ 2026-04-10 21:21 UTC (permalink / raw)
To: Jacqueline Wong
Cc: peterhuewe, jarkko, jgg, Alexander.Steffen, linux-integrity,
linux-kernel, axelrasmussen, Jordan Hand
In-Reply-To: <20260410211350.1132611-1-jacqwong@google.com>
Dear Jacqueline,
Thank you for your patch.
Am 10.04.26 um 23:13 schrieb Jacqueline Wong:
> From: Jordan Hand <jhand@google.com>
>
> tpm_tis_send_main() will attempt to retry sending data TPM_RETRY times.
> Currently, if those retries are exhausted, the driver will attempt to
> call execute. The TPM will be in the wrong state, leading to the
> operation simply timing out.
>
> Instead, if there is still an error after retries are exhausted, return
> that error immediately.
>
> Additionally, add logging to more easily determine reason for transmit
> failure.
Please paste the logs without and with your patch of an effected system.
*Additionally* is often a good indicator to split the patch into even
smaller pieces. ;-)
> Fixes: 280db21e153d8 ("tpm_tis: Resend command to recover from data transfer errors")
> Signed-off-by: Jordan Hand <jhand@google.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index e2a1769081b1..b78937841879 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -471,6 +471,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
> status = tpm_tis_status(chip);
> if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
> rc = -EIO;
> + dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be set. sts = 0x%08x\n",
> + status);
> goto out_err;
> }
> }
> @@ -491,6 +493,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
> status = tpm_tis_status(chip);
> if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) {
> rc = -EIO;
> + dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be unset. sts = 0x%08x\n",
> + status);
> goto out_err;
> }
>
> @@ -552,11 +556,16 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
> break;
> else if (rc != -EAGAIN && rc != -EIO)
> /* Data transfer failed, not recoverable */
> - return rc;
> + goto out_err;
>
> usleep_range(priv->timeout_min, priv->timeout_max);
> }
>
> + if (rc == -EAGAIN || rc == -EIO) {
> + dev_err(&chip->dev, "Exhausted tpm_tis_send_data retries\n");
Please also print the number of retries.
> + goto out_err;
> + }
> +
> /* go and do it */
> rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);
> if (rc < 0)
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* [PATCH v2 0/2] tpm_tis: fix retry exhaustion and add logging
From: Jacqueline Wong @ 2026-04-11 0:32 UTC (permalink / raw)
To: linux-integrity; +Cc: jarkko, peterhuewe, jgg, axelrasmussen, Jacqueline Wong
The Fix:
- Patch 1: Adds error logs to identify the specific hardware status mismatch.
- Patch 2: Stops execution immediately when retries are exhausted.
v2 changes:
- Split logging and logic into separate patches.
- Added retry count to the error message.
- Included dmesg traces below.
Testing:
Dmesg traces obtained using error injection to simulate status register mismatches.
Before:
[ 130.288751] tpm tpm0: Operation Timed out
[ 250.306070] tpm tpm0: Operation Timed out
[ 250.310173] tpm tpm0: A TPM error (-62) occurred attempting to determine the timeouts
After:
[ 10.271547] tpm tpm0: TPM_STS_DATA_EXPECT should be unset. sts = 0x00000080
...
[ 10.646283] tpm tpm0: TPM_STS_DATA_EXPECT should be unset. sts = 0x00000080
[ 10.653461] tpm tpm0: Exhausted 50 tpm_tis_send_data retries
[ 10.659304] tpm tpm0: tpm_try_transmit: send(): error -5
[ 10.665435] tpm tpm0: TPM_STS_DATA_EXPECT should be unset. sts = 0x00000080
...
[ 11.037198] tpm tpm0: TPM_STS_DATA_EXPECT should be unset. sts = 0x00000080
[ 11.044441] tpm tpm0: Exhausted 50 tpm_tis_send_data retries
[ 11.050288] tpm tpm0: tpm_try_transmit: send(): error -5
[ 11.055723] tpm tpm0: A TPM error (-5) occurred attempting to determine the timeouts
Jacqueline Wong (2):
tpm: tpm_tis: add error logging for data transfer
tpm: tpm_tis: stop transmit if retries are exhausted
drivers/char/tpm/tpm_tis_core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply
* [PATCH v2 1/2] tpm: tpm_tis: add error logging for data transfer
From: Jacqueline Wong @ 2026-04-11 0:32 UTC (permalink / raw)
To: linux-integrity
Cc: jarkko, peterhuewe, jgg, axelrasmussen, Jacqueline Wong,
Jordan Hand
In-Reply-To: <20260411003300.1823020-1-jacqwong@google.com>
Add logging to more easily determine reason for transmit failure
Fixes: 280db21e153d8 ("tpm_tis: Resend command to recover from data transfer errors")
Signed-off-by: Jacqueline Wong <jacqwong@google.com>
Signed-off-by: Jordan Hand <jhand@google.com>
---
drivers/char/tpm/tpm_tis_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index e2a1769081b1..c79b696086fe 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -471,6 +471,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
status = tpm_tis_status(chip);
if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
rc = -EIO;
+ dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be set. sts = 0x%08x\n",
+ status);
goto out_err;
}
}
@@ -491,6 +493,8 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
status = tpm_tis_status(chip);
if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) {
rc = -EIO;
+ dev_err(&chip->dev, "TPM_STS_DATA_EXPECT should be unset. sts = 0x%08x\n",
+ status);
goto out_err;
}
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH v2 2/2] tpm: tpm_tis: stop transmit if retries are exhausted
From: Jacqueline Wong @ 2026-04-11 0:33 UTC (permalink / raw)
To: linux-integrity
Cc: jarkko, peterhuewe, jgg, axelrasmussen, Jacqueline Wong,
Jordan Hand
In-Reply-To: <20260411003300.1823020-1-jacqwong@google.com>
tpm_tis_send_main() will attempt to retry sending data TPM_RETRY times.
Currently, if those retries are exhausted, the driver will attempt to
call execute. The TPM will be in the wrong state, leading to the
operation simply timing out.
Instead, if there is still an error after retries are exhausted, return
that error immediately.
Fixes: 280db21e153d8 ("tpm_tis: Resend command to recover from data transfer errors")
Signed-off-by: Jacqueline Wong <jacqwong@google.com>
Signed-off-by: Jordan Hand <jhand@google.com>
---
drivers/char/tpm/tpm_tis_core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c79b696086fe..ecda3c1085c7 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -556,11 +556,16 @@ static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
break;
else if (rc != -EAGAIN && rc != -EIO)
/* Data transfer failed, not recoverable */
- return rc;
+ goto out_err;
usleep_range(priv->timeout_min, priv->timeout_max);
}
+ if (rc == -EAGAIN || rc == -EIO) {
+ dev_err(&chip->dev, "Exhausted %d tpm_tis_send_data retries\n", TPM_RETRY);
+ goto out_err;
+ }
+
/* go and do it */
rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);
if (rc < 0)
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH] trusted-keys: move pr_fmt out of trusted-type.h
From: Josh Snyder @ 2026-04-11 20:12 UTC (permalink / raw)
To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Ahmad Fatoum, Pengutronix Kernel Team, Paul Moore, James Morris,
Serge E. Hallyn, David Gstir, sigma star Kernel Team,
Srish Srinivasan, Nayna Jain, Sumit Garg
Cc: linux-integrity, keyrings, linux-kernel, linux-security-module,
Josh Snyder
Defining pr_fmt in a widely-included header leaks the "trusted_key: "
prefix into every translation unit that transitively includes
<keys/trusted-type.h>. dm-crypt, for example, ends up printing
trusted_key: device-mapper: crypt: dm-10: INTEGRITY AEAD ERROR ...
dm-crypt began including <keys/trusted-type.h> in commit 363880c4eb36
("dm crypt: support using trusted keys"), which predates the pr_fmt
addition, so the regression has been live from the moment the header
gained its own pr_fmt definition.
Move the pr_fmt definition into the trusted-keys source files that
actually want the prefix.
Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Josh Snyder <josh@code406.com>
---
| 6 ------
| 2 ++
| 2 ++
| 2 ++
| 2 ++
| 2 ++
| 2 ++
7 files changed, 12 insertions(+), 6 deletions(-)
--git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 03527162613f7..54da1f174aeab 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -11,12 +11,6 @@
#include <linux/rcupdate.h>
#include <linux/tpm.h>
-#ifdef pr_fmt
-#undef pr_fmt
-#endif
-
-#define pr_fmt(fmt) "trusted_key: " fmt
-
#define MIN_KEY_SIZE 32
#define MAX_KEY_SIZE 128
#if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
--git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
index 601943ce0d60f..a31fd89c0e5c5 100644
--- a/security/keys/trusted-keys/trusted_caam.c
+++ b/security/keys/trusted-keys/trusted_caam.c
@@ -4,6 +4,8 @@
* Copyright 2025 NXP
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <keys/trusted_caam.h>
#include <keys/trusted-type.h>
#include <linux/build_bug.h>
--git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index 0b142d941cd2e..159af9dcfc774 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -6,6 +6,8 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <keys/user-type.h>
#include <keys/trusted-type.h>
#include <keys/trusted_tee.h>
--git a/security/keys/trusted-keys/trusted_dcp.c b/security/keys/trusted-keys/trusted_dcp.c
index 7b6eb655df0cb..f15ec400848ce 100644
--- a/security/keys/trusted-keys/trusted_dcp.c
+++ b/security/keys/trusted-keys/trusted_dcp.c
@@ -3,6 +3,8 @@
* Copyright (C) 2021 sigma star gmbh
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <crypto/aead.h>
#include <crypto/aes.h>
#include <crypto/algapi.h>
--git a/security/keys/trusted-keys/trusted_pkwm.c b/security/keys/trusted-keys/trusted_pkwm.c
index bf42c6679245a..94c92b90d88da 100644
--- a/security/keys/trusted-keys/trusted_pkwm.c
+++ b/security/keys/trusted-keys/trusted_pkwm.c
@@ -3,6 +3,8 @@
* Copyright (C) 2025 IBM Corporation, Srish Srinivasan <ssrish@linux.ibm.com>
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <keys/trusted_pkwm.h>
#include <keys/trusted-type.h>
#include <linux/build_bug.h>
--git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 6ea728f1eae6f..69dac20e4bf23 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -6,6 +6,8 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <crypto/hash_info.h>
#include <crypto/sha1.h>
#include <crypto/utils.h>
--git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 6340823f8b53c..f47ae952a0e7c 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -4,6 +4,8 @@
* Copyright (C) 2014 Intel Corporation
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <linux/asn1_encoder.h>
#include <linux/oid_registry.h>
#include <linux/string.h>
---
base-commit: cc13002a9f984d37906e9476f3e532a8cdd126f5
change-id: 20260411-trusted-key-header-a544a4f149d2
Best regards,
--
Josh
^ permalink raw reply related
* Re: [PATCH v3] KEYS: trusted: Debugging as a feature
From: Nayna Jain @ 2026-04-12 18:47 UTC (permalink / raw)
To: Jarkko Sakinen, linux-integrity, keyrings
Cc: Srish Srinivasan, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ahmad Fatoum,
Pengutronix Kernel Team, linux-kernel, linux-security-module
In-Reply-To: <20260409160752.988713-1-jarkko@kernel.org>
On 4/9/26 12:07 PM, Jarkko Sakinen wrote:
> From: Jarkko Sakkinen <jarkko@kernel.org>
>
> TPM_DEBUG, and other similar flags, are a non-standard way to specify a
> feature in Linux kernel. Introduce CONFIG_TRUSTED_KEYS_DEBUG for trusted
> keys, and use it to replace these ad-hoc feature flags.
>
> Given that trusted keys debug dumps can contain sensitive data, harden the
> feature as follows:
>
> 1. In the Kconfig description postulate that pr_debug() statements must be
> used.
> 2. Use pr_debug() statements in TPM 1.x driver to print the protocol dump.
> 3. Require trusted.debug=1 on the kernel command line (default: 0) to
> activate dumps at runtime, even when CONFIG_TRUSTED_KEYS_DEBUG=y.
>
> Traces, when actually needed, can be easily enabled by providing
> trusted.dyndbg='+p' and trusted.debug=1 in the kernel command-line.
Thanks Jarkko. Additional changes looks good to me. I just realized that
the kernel command-line parameters document may need to be updated to
include these parameters.
Apart from that, feel free to add my
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Thanks & Regards,
- Nayna
^ permalink raw reply
* Re: [PATCH] tpm: aovid -Wunused-but-set-variable
From: Thorsten Blum @ 2026-04-12 23:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jarkko Sakkinen, Matthew Garrett, Bartosz Szczepanek,
Ard Biesheuvel, Arnd Bergmann, Peter Huewe, Jason Gunthorpe,
linux-integrity, linux-kernel
In-Reply-To: <20240322132307.907203-1-arnd@kernel.org>
On Fri, Mar 22, 2024 at 02:22:48PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Outside of the EFI tpm code, the TPM_MEMREMAP()/TPM_MEMUNMAP functions are
> defined as trivial macros, leading to the mapping_size variable ending
> up unused:
>
> In file included from drivers/char/tpm/tpm-sysfs.c:16:
> In file included from drivers/char/tpm/tpm.h:28:
> include/linux/tpm_eventlog.h:167:6: error: variable 'mapping_size' set but not used [-Werror,-Wunused-but-set-variable]
> 167 | int mapping_size;
>
> Turn the stubs into inline functions to avoid this warning.
>
> Fixes: c46f3405692d ("tpm: Reserve the TPM final events table")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> include/linux/tpm_eventlog.h | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
> index 7d68a5cc5881..6e5be15029fb 100644
> --- a/include/linux/tpm_eventlog.h
> +++ b/include/linux/tpm_eventlog.h
> @@ -131,11 +131,16 @@ struct tcg_algorithm_info {
> };
>
> #ifndef TPM_MEMREMAP
> -#define TPM_MEMREMAP(start, size) NULL
> +static inline void *TPM_MEMREMAP(unsigned long start, size_t size)
> +{
> + return NULL;
> +}
> #endif
>
> #ifndef TPM_MEMUNMAP
> -#define TPM_MEMUNMAP(start, size) do{} while(0)
> +static inline void TPM_MEMUNMAP(void *mapping, size_t size)
> +{
> +}
> #endif
>
> /**
I just stumbled upon the same problem and found this patch from 2024,
which still applies. I cc'ed the current maintainers - maybe someone can
pick this up? Thanks!
Reviewed-by: Thorsten Blum <thorsten.blum@linux.dev>
^ permalink raw reply
* Re: [PATCH v1] tpm: restore timeout for key creation commands
From: Zhang, Baoli @ 2026-04-13 1:31 UTC (permalink / raw)
To: Paul Menzel
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, Serge Hallyn,
Lili Li, linux-integrity, linux-kernel
In-Reply-To: <2f9f7dcf-7cea-46f8-8887-ac2495e12863@molgen.mpg.de>
On 4/10/2026 2:49 PM, Paul Menzel wrote:
> Dear Baoli,
>
>
> Thank you for your patch. Some formalities:
>
> Am 10.04.26 um 03:49 schrieb Baoli.Zhang:
>> After the per-command duration map was introduced, TPM2 key creation
>> commands (`CREATE_PRIMARY`, `CREATE`, `CREATE_LOADED`) were limited to
>> 30 seconds.
>>
>> On some platforms this is not sufficient and key creation can time out.
>> Commit 207696b17f38 ("tpm: use a map for tpm2_calc_ordinal_duration()")
>> inadvertently reduced these command timeouts from 300 seconds to 30
>> seconds. Restore them to 300 seconds to avoid spurious failures.
>
> Please document such a platform.
Thanks for your comments, I will add the platform after the internal
alignment.
>
>> Fixes: 207696b17f38 ("tpm: use a map for tpm2_calc_ordinal_duration()")
>>
>> Signed-off-by: Baoli.Zhang <baoli.zhang@linux.intel.com>
>
> It’d be great if you remove the dot from your name:
>
> git config --global user.name "Baoli Zhang"
>
Yes, will remove it in v2.
>> Co-developed-by: lili.li <lili.li@intel.com>
>
> Same here. Maybe spell it Lili Li?
>
> git commit --amend --author="BaoliZhang
> <baoli.zhang@linux.intel.com>" -s
>
Yes, her name is Lili Li. Will also remove dot from her name.
>> ---
>> drivers/char/tpm/tpm2-cmd.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 3a77be7ebf4aa..430022f695f24 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -71,9 +71,9 @@ static const struct {
>> {TPM2_CC_HIERARCHY_CHANGE_AUTH, 2000},
>> {TPM2_CC_GET_CAPABILITY, 750},
>> {TPM2_CC_NV_READ, 2000},
>> - {TPM2_CC_CREATE_PRIMARY, 30000},
>> - {TPM2_CC_CREATE, 30000},
>> - {TPM2_CC_CREATE_LOADED, 30000},
>> + {TPM2_CC_CREATE_PRIMARY, 300000},
>> + {TPM2_CC_CREATE, 300000},
>> + {TPM2_CC_CREATE_LOADED, 300000},
>> };
>> /**
>
>
^ permalink raw reply
* Re: [PATCH] trusted-keys: move pr_fmt out of trusted-type.h
From: Marco Felsch @ 2026-04-13 11:01 UTC (permalink / raw)
To: Josh Snyder
Cc: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Ahmad Fatoum, Pengutronix Kernel Team, Paul Moore, James Morris,
Serge E. Hallyn, David Gstir, sigma star Kernel Team,
Srish Srinivasan, Nayna Jain, Sumit Garg, linux-security-module,
linux-integrity, keyrings, linux-kernel
In-Reply-To: <20260411-trusted-key-header-v1-1-407c2cd954db@code406.com>
Hi Josh,
On 26-04-11, Josh Snyder wrote:
> Defining pr_fmt in a widely-included header leaks the "trusted_key: "
> prefix into every translation unit that transitively includes
> <keys/trusted-type.h>. dm-crypt, for example, ends up printing
>
> trusted_key: device-mapper: crypt: dm-10: INTEGRITY AEAD ERROR ...
>
> dm-crypt began including <keys/trusted-type.h> in commit 363880c4eb36
> ("dm crypt: support using trusted keys"), which predates the pr_fmt
> addition, so the regression has been live from the moment the header
> gained its own pr_fmt definition.
>
> Move the pr_fmt definition into the trusted-keys source files that
> actually want the prefix.
>
> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Josh Snyder <josh@code406.com>
> ---
> include/keys/trusted-type.h | 6 ------
> security/keys/trusted-keys/trusted_caam.c | 2 ++
> security/keys/trusted-keys/trusted_core.c | 2 ++
> security/keys/trusted-keys/trusted_dcp.c | 2 ++
> security/keys/trusted-keys/trusted_pkwm.c | 2 ++
> security/keys/trusted-keys/trusted_tpm1.c | 2 ++
> security/keys/trusted-keys/trusted_tpm2.c | 2 ++
> 7 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 03527162613f7..54da1f174aeab 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -11,12 +11,6 @@
> #include <linux/rcupdate.h>
> #include <linux/tpm.h>
>
> -#ifdef pr_fmt
> -#undef pr_fmt
> -#endif
> -
> -#define pr_fmt(fmt) "trusted_key: " fmt
> -
> #define MIN_KEY_SIZE 32
> #define MAX_KEY_SIZE 128
> #if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
> index 601943ce0d60f..a31fd89c0e5c5 100644
> --- a/security/keys/trusted-keys/trusted_caam.c
> +++ b/security/keys/trusted-keys/trusted_caam.c
> @@ -4,6 +4,8 @@
> * Copyright 2025 NXP
> */
>
> +#define pr_fmt(fmt) "trusted_key: " fmt
Can we adapt this patch further to include the trusted-key type as well?
E.g. 'trusted_key-caam'.
Regards,
Marco
^ permalink raw reply
* Re: [PATCH] trusted-keys: move pr_fmt out of trusted-type.h
From: Ahmad Fatoum @ 2026-04-13 11:03 UTC (permalink / raw)
To: Marco Felsch, Josh Snyder
Cc: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Pengutronix Kernel Team, Paul Moore, James Morris,
Serge E. Hallyn, David Gstir, sigma star Kernel Team,
Srish Srinivasan, Nayna Jain, Sumit Garg, linux-security-module,
linux-integrity, keyrings, linux-kernel
In-Reply-To: <cie3zqy5phlopdrxsxpniujwr6i3cpdkfrwjvth3a7ypwjx3ee@hqjl67jnfdch>
Hi,
On 4/13/26 1:01 PM, Marco Felsch wrote:
> Hi Josh,
>
> On 26-04-11, Josh Snyder wrote:
>> Defining pr_fmt in a widely-included header leaks the "trusted_key: "
>> prefix into every translation unit that transitively includes
>> <keys/trusted-type.h>. dm-crypt, for example, ends up printing
>>
>> trusted_key: device-mapper: crypt: dm-10: INTEGRITY AEAD ERROR ...
>>
>> dm-crypt began including <keys/trusted-type.h> in commit 363880c4eb36
>> ("dm crypt: support using trusted keys"), which predates the pr_fmt
>> addition, so the regression has been live from the moment the header
>> gained its own pr_fmt definition.
>>
>> Move the pr_fmt definition into the trusted-keys source files that
>> actually want the prefix.
>>
>> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
>> Assisted-by: Claude:claude-opus-4-6
>> Signed-off-by: Josh Snyder <josh@code406.com>
>> ---
>> include/keys/trusted-type.h | 6 ------
>> security/keys/trusted-keys/trusted_caam.c | 2 ++
>> security/keys/trusted-keys/trusted_core.c | 2 ++
>> security/keys/trusted-keys/trusted_dcp.c | 2 ++
>> security/keys/trusted-keys/trusted_pkwm.c | 2 ++
>> security/keys/trusted-keys/trusted_tpm1.c | 2 ++
>> security/keys/trusted-keys/trusted_tpm2.c | 2 ++
>> 7 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
>> index 03527162613f7..54da1f174aeab 100644
>> --- a/include/keys/trusted-type.h
>> +++ b/include/keys/trusted-type.h
>> @@ -11,12 +11,6 @@
>> #include <linux/rcupdate.h>
>> #include <linux/tpm.h>
>>
>> -#ifdef pr_fmt
>> -#undef pr_fmt
>> -#endif
>> -
>> -#define pr_fmt(fmt) "trusted_key: " fmt
>> -
>> #define MIN_KEY_SIZE 32
>> #define MAX_KEY_SIZE 128
>> #if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
>> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
>> index 601943ce0d60f..a31fd89c0e5c5 100644
>> --- a/security/keys/trusted-keys/trusted_caam.c
>> +++ b/security/keys/trusted-keys/trusted_caam.c
>> @@ -4,6 +4,8 @@
>> * Copyright 2025 NXP
>> */
>>
>> +#define pr_fmt(fmt) "trusted_key: " fmt
>
> Can we adapt this patch further to include the trusted-key type as well?
> E.g. 'trusted_key-caam'.
Agreed, if we move it into the individual files, we can use the occasion
to make it a bit more descriptive.
I would suggest "trusted_key: caam: ", so the prefix stays the same.
Cheers,
Ahmad
>
> Regards,
> Marco
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH] evm: zero-initialize the evm_xattrs read buffer
From: Roberto Sassu @ 2026-04-13 15:20 UTC (permalink / raw)
To: Pengpeng Hou, Mimi Zohar, Roberto Sassu
Cc: Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
Serge Hallyn, linux-integrity, linux-security-module,
linux-kernel
In-Reply-To: <20260407153002.2-evm-xattrs-pengpeng@iscas.ac.cn>
On Tue, 2026-04-07 at 14:09 +0800, Pengpeng Hou wrote:
> evm_read_xattrs() allocates size + 1 bytes, fills them from the list of
> enabled xattrs and then passes strlen(temp) to simple_read_from_buffer().
> When no configured xattrs are enabled, the fill loop stores nothing and
> temp[0] remains uninitialized, so strlen() reads beyond initialized
> memory.
>
> Use kzalloc() so the empty-list case stays a valid empty C string.
Please also add the Fixes: tag with the relevant commit.
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> security/integrity/evm/evm_secfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> index acd840461902..03d376fa36c2 100644
> --- a/security/integrity/evm/evm_secfs.c
> +++ b/security/integrity/evm/evm_secfs.c
> @@ -145,7 +145,7 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
> size += strlen(xattr->name) + 1;
> }
>
> - temp = kmalloc(size + 1, GFP_KERNEL);
> + temp = kzalloc(size + 1, GFP_KERNEL);
Yes, or just set temp[size] to the terminator so that we don't waste
computation. Can you also change sprintf() to snprintf()?
Thanks
Roberto
> if (!temp) {
> mutex_unlock(&xattr_list_mutex);
> return -ENOMEM;
^ permalink raw reply
* [PATCH 05/10] dm-ima: Fix UAF errors and measuring incorrect context
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
the dm-ima code did not keep the dm_ima_measure_on_* functions from
running at the same time. This could lead to various errors. If two
processes were updating the device state, one could update the state
first, but the other could measure the state first, causing the the
current device state to appear incorrect. If a table load happened while
a device was resuming, the IMA measurement could report the wrong table
being active. And if two dm_ima_measure_on_* functions ran at the same
time, one of them could free data that the other was accessing, causing
a crash.
All the core dm functions that call a dm_ima_measure_on_* function
update the device state they want to measure under the _hash_lock,
except for do_resume(). But holding the _hash_lock is not a good way to
synchronize these functions. It's a global mutex, that is needed in may
dm operations, and the dm_ima_measure_* functions can sleep, blocking
any dm operation on any device that needs the _hash_lock.
To serialize and order the IMA measurement functions, the
dm_ima_measurements now has two counters. One is updated while holding
the _hash_lock and stored, along with the device name and uuid, in a
dm_ima_context struct. Once the _hash_lock is dropped, the
dm_ima_measure_* function is called. It waits unit the other counter
matches the value it stored. Then it measures the device, updates second
counter, and wakes up any other functions waiting on that counter. This
makes sure that the measurements are serialized and done in the order
that the _hash_lock was acquired in. But they only block other
dm_ima_measure_* functions for the same device, which should happen only
rarely.
do_resume() is trickier, because it removes the inactive table while
holding the _hash_lock, but doesn't hold it while updating md->map. To
make sure it is also ordered, the IMA code grabs the _hash_lock after
md->map is updated. Then it makes sure that the device isn't being
removed and that another do_resume() hasn't already changed the active
table again, and serializes like the other functions do.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 271 +++++++++++++++++++++++++-----------------
drivers/md/dm-ima.h | 64 ++++++++--
drivers/md/dm-ioctl.c | 144 ++++++++++++++++++++--
drivers/md/dm.c | 2 +
4 files changed, 350 insertions(+), 131 deletions(-)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index c141068bc6b4..e8fa049c6317 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -21,25 +21,32 @@
* character, so that they don't interfere with the construction of key-value pairs,
* and clients can split the key1=val1,key2=val2,key3=val3; pairs properly.
*/
-static void fix_separator_chars(char **buf)
+static void fix_separator_chars(char *buf)
{
- int l = strlen(*buf);
+ int l = strlen(buf);
int i, j, sp = 0;
for (i = 0; i < l; i++)
- if ((*buf)[i] == '\\' || (*buf)[i] == ';' || (*buf)[i] == '=' || (*buf)[i] == ',')
+ if (buf[i] == '\\' || buf[i] == ';' || buf[i] == '=' || buf[i] == ',')
sp++;
if (!sp)
return;
+ buf[l + sp] = '\0';
for (i = l-1, j = i+sp; i >= 0; i--) {
- (*buf)[j--] = (*buf)[i];
- if ((*buf)[i] == '\\' || (*buf)[i] == ';' || (*buf)[i] == '=' || (*buf)[i] == ',')
- (*buf)[j--] = '\\';
+ buf[j--] = buf[i];
+ if (buf[i] == '\\' || buf[i] == ';' || buf[i] == '=' || buf[i] == ',')
+ buf[j--] = '\\';
}
}
+static void fix_context_strings(struct dm_ima_context *context)
+{
+ fix_separator_chars(context->dev_name);
+ fix_separator_chars(context->dev_uuid);
+}
+
/*
* Internal function to allocate memory for IMA measurements.
*/
@@ -59,68 +66,75 @@ static void *dm_ima_alloc(size_t len, bool noio)
return ptr;
}
-/*
- * Internal function to allocate and copy name and uuid for IMA measurements.
- */
-static int dm_ima_alloc_and_copy_name_uuid(struct mapped_device *md, char **dev_name,
- char **dev_uuid, bool noio)
+void dm_ima_init(struct mapped_device *md)
{
- int r;
- *dev_name = dm_ima_alloc(DM_NAME_LEN*2, noio);
- if (!(*dev_name)) {
- r = -ENOMEM;
- goto error;
- }
+ md->ima.update_idx = 0;
+ atomic_set(&md->ima.measure_idx, 0);
+ init_waitqueue_head(&md->ima.ima_wq);
+}
- *dev_uuid = dm_ima_alloc(DM_UUID_LEN*2, noio);
- if (!(*dev_uuid)) {
- r = -ENOMEM;
- goto error;
+void dm_ima_alloc_context(struct dm_ima_context **context, bool noio)
+{
+ *context = dm_ima_alloc(sizeof(struct dm_ima_context), noio);
+}
+
+void dm_ima_free_context(struct dm_ima_context *context)
+{
+ if (likely(context)) {
+ kfree(context->table.device_metadata);
+ kfree(context->table.hash);
+ kfree(context);
}
+}
- r = dm_copy_name_and_uuid(md, *dev_name, *dev_uuid);
- if (r)
- goto error;
+/*
+ * Helper function for swapping the table, to make sure that the
+ * correct table metadata is saved and restored.
+ */
+void dm_ima_context_table_op(struct mapped_device *md,
+ struct dm_ima_context *context,
+ enum dm_ima_table_op op)
+{
+ struct dm_ima_measurements *ima = &md->ima;
- fix_separator_chars(dev_name);
- fix_separator_chars(dev_uuid);
+ if (unlikely(!context))
+ return;
- return 0;
-error:
- kfree(*dev_name);
- kfree(*dev_uuid);
- *dev_name = NULL;
- *dev_uuid = NULL;
- return r;
+ wait_event(ima->ima_wq,
+ ((unsigned int)(atomic_read(&ima->measure_idx)) ==
+ context->update_idx));
+ smp_mb();
+
+ if (op == DM_IMA_TABLE_SAVE) {
+ context->table = ima->inactive_table;
+ memset(&ima->inactive_table, 0, sizeof(ima->inactive_table));
+ } else {
+ ima->inactive_table = context->table;
+ memset(&context->table, 0, sizeof(context->table));
+ }
+
+ smp_mb__before_atomic();
+ atomic_inc(&ima->measure_idx);
+ wake_up_all(&ima->ima_wq);
}
/*
* Internal function to allocate and copy device data for IMA measurements.
*/
static int dm_ima_alloc_and_copy_device_data(struct mapped_device *md, char **device_data,
+ struct dm_ima_context *context,
unsigned int num_targets, bool noio)
{
- char *dev_name = NULL, *dev_uuid = NULL;
- int r;
-
- r = dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio);
- if (r)
- return r;
-
*device_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, noio);
- if (!(*device_data)) {
- r = -ENOMEM;
- goto error;
- }
+ if (!(*device_data))
+ return -ENOMEM;
scnprintf(*device_data, DM_IMA_DEVICE_BUF_LEN,
"name=%s,uuid=%s,major=%d,minor=%d,minor_count=%d,num_targets=%u;",
- dev_name, dev_uuid, md->disk->major, md->disk->first_minor,
- md->disk->minors, num_targets);
-error:
- kfree(dev_name);
- kfree(dev_uuid);
- return r;
+ context->dev_name, context->dev_uuid, md->disk->major,
+ md->disk->first_minor, md->disk->minors, num_targets);
+
+ return 0;
}
/*
@@ -162,7 +176,8 @@ static int dm_ima_alloc_and_copy_capacity_str(struct mapped_device *md, char **c
/*
* Build up the IMA data for each target, and finally measure.
*/
-void dm_ima_measure_on_table_load(struct dm_table *table)
+void dm_ima_measure_on_table_load(struct dm_table *table,
+ struct dm_ima_context *context)
{
size_t device_data_buf_len, target_metadata_buf_len, target_data_buf_len, l = 0;
char *target_metadata_buf = NULL, *target_data_buf = NULL, *digest_buf = NULL;
@@ -175,9 +190,17 @@ void dm_ima_measure_on_table_load(struct dm_table *table)
bool noio = false;
char table_load_event_name[] = "dm_table_load";
+ if (unlikely(!context))
+ return;
+
+ wait_event(table->md->ima.ima_wq,
+ ((unsigned int)(atomic_read(&table->md->ima.measure_idx)) ==
+ context->update_idx));
+ smp_mb();
+
ima_buf = dm_ima_alloc(DM_IMA_MEASUREMENT_BUF_LEN, noio);
if (!ima_buf)
- return;
+ goto error;
target_metadata_buf = dm_ima_alloc(DM_IMA_TARGET_METADATA_BUF_LEN, noio);
if (!target_metadata_buf)
@@ -189,7 +212,9 @@ void dm_ima_measure_on_table_load(struct dm_table *table)
num_targets = table->num_targets;
- if (dm_ima_alloc_and_copy_device_data(table->md, &device_data_buf, num_targets, noio))
+ fix_context_strings(context);
+ if (dm_ima_alloc_and_copy_device_data(table->md, &device_data_buf,
+ context, num_targets, noio))
goto error;
sha256_init(&hash_ctx);
@@ -299,14 +324,19 @@ void dm_ima_measure_on_table_load(struct dm_table *table)
kfree(ima_buf);
kfree(target_metadata_buf);
kfree(target_data_buf);
+
+ smp_mb__before_atomic();
+ atomic_inc(&table->md->ima.measure_idx);
+ wake_up_all(&table->md->ima.ima_wq);
}
/*
* Measure IMA data on device resume.
*/
-void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
+void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap,
+ struct dm_ima_context *context)
{
- char *device_table_data, *dev_name = NULL, *dev_uuid = NULL, *capacity_str = NULL;
+ char *device_table_data = NULL, *capacity_str = NULL;
char active[] = "active_table_hash=";
unsigned int active_len = strlen(active);
unsigned int l = 0;
@@ -314,9 +344,17 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
bool nodata = true;
int capacity_len;
+ if (unlikely(!context))
+ return;
+
+ wait_event(md->ima.ima_wq,
+ ((unsigned int)(atomic_read(&md->ima.measure_idx)) ==
+ context->update_idx));
+ smp_mb();
+
device_table_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, noio);
if (!device_table_data)
- return;
+ goto error;
capacity_len = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
if (capacity_len < 0)
@@ -328,25 +366,8 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
if (swap) {
kfree(md->ima.active_table.hash);
kfree(md->ima.active_table.device_metadata);
- memset(&md->ima.active_table, 0, sizeof(md->ima.active_table));
-
- if (md->ima.inactive_table.hash) {
- md->ima.active_table.hash = md->ima.inactive_table.hash;
- md->ima.active_table.hash_len = md->ima.inactive_table.hash_len;
- md->ima.inactive_table.hash = NULL;
- md->ima.inactive_table.hash_len = 0;
- }
-
- if (md->ima.inactive_table.device_metadata) {
- md->ima.active_table.device_metadata =
- md->ima.inactive_table.device_metadata;
- md->ima.active_table.device_metadata_len =
- md->ima.inactive_table.device_metadata_len;
- md->ima.active_table.num_targets = md->ima.inactive_table.num_targets;
- md->ima.inactive_table.device_metadata = NULL;
- md->ima.inactive_table.device_metadata_len = 0;
- md->ima.inactive_table.num_targets = 0;
- }
+ md->ima.active_table = context->table;
+ memset(&context->table, 0, sizeof(context->table));
}
if (md->ima.active_table.device_metadata) {
@@ -372,12 +393,11 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
}
if (nodata) {
- if (dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio))
- goto error;
-
+ fix_context_strings(context);
l = scnprintf(device_table_data, DM_IMA_DEVICE_BUF_LEN,
"%sname=%s,uuid=%s;device_resume=no_data;",
- DM_IMA_VERSION_STR, dev_name, dev_uuid);
+ DM_IMA_VERSION_STR, context->dev_name,
+ context->dev_uuid);
}
memcpy(device_table_data + l, capacity_str, capacity_len);
@@ -385,19 +405,23 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
dm_ima_measure_data("dm_device_resume", device_table_data, l, noio);
- kfree(dev_name);
- kfree(dev_uuid);
error:
kfree(capacity_str);
kfree(device_table_data);
+
+ smp_mb__before_atomic();
+ atomic_inc(&md->ima.measure_idx);
+ wake_up_all(&md->ima.ima_wq);
}
/*
* Measure IMA data on remove.
*/
-void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
+void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all,
+ struct dm_ima_context *context,
+ unsigned int idx)
{
- char *device_table_data, *dev_name = NULL, *dev_uuid = NULL, *capacity_str = NULL;
+ char *device_table_data, *capacity_str = NULL;
char active_table_str[] = "active_table_hash=";
char inactive_table_str[] = "inactive_table_hash=";
char device_active_str[] = "device_active_metadata=";
@@ -413,6 +437,13 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
bool nodata = true;
int capacity_len;
+ wait_event(md->ima.ima_wq,
+ ((unsigned int)(atomic_read(&md->ima.measure_idx)) == idx));
+ smp_mb();
+
+ if (unlikely(!context))
+ goto exit;
+
device_table_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN*2, noio);
if (!device_table_data)
goto exit;
@@ -481,12 +512,11 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
* in IMA measurements.
*/
if (nodata) {
- if (dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio))
- goto error;
-
+ fix_context_strings(context);
l = scnprintf(device_table_data, DM_IMA_DEVICE_BUF_LEN,
"%sname=%s,uuid=%s;device_remove=no_data;",
- DM_IMA_VERSION_STR, dev_name, dev_uuid);
+ DM_IMA_VERSION_STR, context->dev_name,
+ context->dev_uuid);
}
memcpy(device_table_data + l, remove_all_str, remove_all_len);
@@ -499,7 +529,6 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
dm_ima_measure_data("dm_device_remove", device_table_data, l, noio);
-error:
kfree(device_table_data);
kfree(capacity_str);
exit:
@@ -512,30 +541,40 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
memset(&md->ima.active_table, 0, sizeof(md->ima.active_table));
memset(&md->ima.inactive_table, 0, sizeof(md->ima.inactive_table));
- kfree(dev_name);
- kfree(dev_uuid);
+ smp_mb__before_atomic();
+ atomic_inc(&md->ima.measure_idx);
+ wake_up_all(&md->ima.ima_wq);
}
/*
* Measure ima data on table clear.
*/
-void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map)
+void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map,
+ struct dm_ima_context *context)
{
unsigned int l = 0;
- char *device_table_data = NULL, *dev_name = NULL, *dev_uuid = NULL, *capacity_str = NULL;
+ char *device_table_data = NULL, *capacity_str = NULL;
char inactive_str[] = "inactive_table_hash=";
unsigned int inactive_len = strlen(inactive_str);
bool noio = true;
bool nodata = true;
int capacity_len;
+ if (unlikely(!context))
+ return;
+
+ wait_event(md->ima.ima_wq,
+ ((unsigned int)(atomic_read(&md->ima.measure_idx)) ==
+ context->update_idx));
+ smp_mb();
+
device_table_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, noio);
if (!device_table_data)
- return;
+ goto error;
capacity_len = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
if (capacity_len < 0)
- goto error1;
+ goto error;
memcpy(device_table_data + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
l += strlen(DM_IMA_VERSION_STR);
@@ -561,12 +600,11 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map)
}
if (nodata) {
- if (dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio))
- goto error2;
-
+ fix_context_strings(context);
l = scnprintf(device_table_data, DM_IMA_DEVICE_BUF_LEN,
"%sname=%s,uuid=%s;table_clear=no_data;",
- DM_IMA_VERSION_STR, dev_name, dev_uuid);
+ DM_IMA_VERSION_STR, context->dev_name,
+ context->dev_uuid);
}
memcpy(device_table_data + l, capacity_str, capacity_len);
@@ -580,29 +618,38 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map)
memset(&md->ima.inactive_table, 0, sizeof(md->ima.inactive_table));
}
- kfree(dev_name);
- kfree(dev_uuid);
-error2:
+error:
kfree(capacity_str);
-error1:
kfree(device_table_data);
+
+ smp_mb__before_atomic();
+ atomic_inc(&md->ima.measure_idx);
+ wake_up_all(&md->ima.ima_wq);
}
/*
* Measure IMA data on device rename.
*/
-void dm_ima_measure_on_device_rename(struct mapped_device *md)
+void dm_ima_measure_on_device_rename(struct mapped_device *md,
+ struct dm_ima_context *context)
{
- char *old_device_data = NULL, *new_device_data = NULL, *combined_device_data = NULL;
- char *new_dev_name = NULL, *new_dev_uuid = NULL, *capacity_str = NULL;
+ char *old_device_data = NULL, *new_device_data = NULL;
+ char *combined_device_data = NULL, *capacity_str = NULL;
bool noio = true;
int len;
- if (dm_ima_alloc_and_copy_device_data(md, &new_device_data,
- md->ima.active_table.num_targets, noio))
+ if (unlikely(!context))
return;
- if (dm_ima_alloc_and_copy_name_uuid(md, &new_dev_name, &new_dev_uuid, noio))
+ wait_event(md->ima.ima_wq,
+ ((unsigned int)(atomic_read(&md->ima.measure_idx)) ==
+ context->update_idx));
+ smp_mb();
+
+ fix_context_strings(context);
+ if (dm_ima_alloc_and_copy_device_data(md, &new_device_data, context,
+ md->ima.active_table.num_targets,
+ noio))
goto error;
combined_device_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN * 2, noio);
@@ -619,7 +666,7 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md)
len = scnprintf(combined_device_data, DM_IMA_DEVICE_BUF_LEN * 2,
"%s%snew_name=%s,new_uuid=%s;%s", DM_IMA_VERSION_STR, old_device_data,
- new_dev_name, new_dev_uuid, capacity_str);
+ context->dev_name, context->dev_uuid, capacity_str);
dm_ima_measure_data("dm_device_rename", combined_device_data, len, noio);
@@ -631,6 +678,8 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md)
kfree(capacity_str);
kfree(combined_device_data);
kfree(old_device_data);
- kfree(new_dev_name);
- kfree(new_dev_uuid);
+
+ smp_mb__before_atomic();
+ atomic_inc(&md->ima.measure_idx);
+ wake_up_all(&md->ima.ima_wq);
}
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index c0548492bef0..9e1b654867d9 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -24,6 +24,11 @@
__dm_ima_str(DM_VERSION_MINOR) "." \
__dm_ima_str(DM_VERSION_PATCHLEVEL) ";"
+enum dm_ima_table_op {
+ DM_IMA_TABLE_SAVE,
+ DM_IMA_TABLE_RESTORE,
+};
+
#ifdef CONFIG_IMA
struct dm_ima_device_table_metadata {
@@ -45,28 +50,67 @@ struct dm_ima_device_table_metadata {
unsigned int hash_len;
};
+struct dm_ima_context {
+ struct dm_ima_device_table_metadata table;
+ unsigned int update_idx;
+ char dev_name[DM_NAME_LEN*2];
+ char dev_uuid[DM_UUID_LEN*2];
+};
+
/*
* This structure contains device metadata, and table hash for
* active and inactive tables for ima measurements.
*/
struct dm_ima_measurements {
+ unsigned int update_idx;
+ atomic_t measure_idx;
+ struct wait_queue_head ima_wq;
struct dm_ima_device_table_metadata active_table;
struct dm_ima_device_table_metadata inactive_table;
};
-void dm_ima_measure_on_table_load(struct dm_table *table);
-void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap);
-void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all);
-void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map);
-void dm_ima_measure_on_device_rename(struct mapped_device *md);
+void dm_ima_init(struct mapped_device *md);
+void dm_ima_alloc_context(struct dm_ima_context **context, bool noio);
+void dm_ima_free_context(struct dm_ima_context *context);
+void dm_ima_context_table_op(struct mapped_device *md,
+ struct dm_ima_context *context,
+ enum dm_ima_table_op op);
+void dm_ima_measure_on_table_load(struct dm_table *table,
+ struct dm_ima_context *context);
+void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap,
+ struct dm_ima_context *context);
+void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all,
+ struct dm_ima_context *context,
+ unsigned int idx);
+void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map,
+ struct dm_ima_context *context);
+void dm_ima_measure_on_device_rename(struct mapped_device *md,
+ struct dm_ima_context *context);
#else
-static inline void dm_ima_measure_on_table_load(struct dm_table *table) {}
-static inline void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap) {}
-static inline void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all) {}
-static inline void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map) {}
-static inline void dm_ima_measure_on_device_rename(struct mapped_device *md) {}
+struct dm_ima_context;
+
+static inline void dm_ima_init(struct mapped_device *md) {}
+static inline void dm_ima_alloc_context(struct dm_ima_context **context, bool noio) {}
+static inline void dm_ima_free_context(struct dm_ima_context *context) {}
+static inline void dm_ima_context_table_op(struct mapped_device *md,
+ struct dm_ima_context *context,
+ enum dm_ima_table_op op) {}
+static inline void dm_ima_measure_on_table_load(struct dm_table *table,
+ struct dm_ima_context *context) {}
+static inline void dm_ima_measure_on_device_resume(struct mapped_device *md,
+ bool swap,
+ struct dm_ima_context *context) {}
+static inline void dm_ima_measure_on_device_remove(struct mapped_device *md,
+ bool remove_all,
+ struct dm_ima_context *context,
+ unsigned int idx) {}
+static inline void dm_ima_measure_on_table_clear(struct mapped_device *md,
+ bool new_map,
+ struct dm_ima_context *context) {}
+static inline void dm_ima_measure_on_device_rename(struct mapped_device *md,
+ struct dm_ima_context *context) {}
#endif /* CONFIG_IMA */
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 1a0dd4981d03..16609882aa92 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -259,6 +259,80 @@ static void free_cell(struct hash_cell *hc)
}
}
+#ifdef CONFIG_IMA
+
+/*
+ * Called while holding to _hash_lock, to guarantee the ordering of the
+ * following dm_ima_measure_on_* functions, which should be called
+ * right after dropping the _hash_lock
+ */
+static unsigned int dm_ima_init_context(struct hash_cell *hc,
+ struct dm_ima_context *context,
+ bool need_idx)
+{
+ lockdep_assert_held(&_hash_lock);
+
+ if (unlikely(!context))
+ return need_idx ? hc->md->ima.update_idx++ : 0;
+
+ context->update_idx = hc->md->ima.update_idx++;
+ strcpy(context->dev_name, hc->name);
+ strcpy(context->dev_uuid, hc->uuid ? : "");
+
+ return context->update_idx;
+}
+
+/*
+ * Called by do_resume() to guarantee that correct ordering, since
+ * it do_resume() does not grab the _hash_lock, either when the table
+ * is not getting swapped, or when swapping the active table
+ */
+static bool dm_ima_need_measure(struct mapped_device *md,
+ struct dm_table *table,
+ struct dm_ima_context *context)
+{
+ int srcu_idx;
+ struct hash_cell *hc;
+ bool need_measure = false;
+
+ if (unlikely(!context))
+ return false;
+
+ down_write(&_hash_lock);
+ /* Check if the device has been removed */
+ hc = dm_get_mdptr(md);
+ if (hc) {
+ /*
+ * If we have a table, we need to make sure that it's the
+ * active table. Otherwise we raced with another process
+ * setting the active table and it will do the measurement
+ */
+ if (!table || dm_get_live_table(md, &srcu_idx) == table) {
+ dm_ima_init_context(hc, context, false);
+ need_measure = true;
+ }
+ if (table)
+ dm_put_live_table(md, srcu_idx);
+ }
+ up_write(&_hash_lock);
+
+ return need_measure;
+}
+#else
+static inline unsigned int dm_ima_init_context(struct hash_cell *hc,
+ struct dm_ima_context *context,
+ bool neex_idx)
+{
+ return 0;
+}
+static inline bool dm_ima_need_measure(struct mapped_device *md,
+ struct dm_table *table,
+ struct dm_ima_context *context)
+{
+ return false;
+}
+#endif
+
/*
* The kdev_t and uuid of a device can never change once it is
* initially inserted.
@@ -344,7 +418,10 @@ static int dm_hash_remove_all(unsigned flags)
struct hash_cell *hc;
struct mapped_device *md;
struct dm_table *t;
+ struct dm_ima_context *ima_context = NULL;
+ unsigned int ima_idx;
+ dm_ima_alloc_context(&ima_context, true);
retry:
dev_skipped = 0;
@@ -353,6 +430,7 @@ static int dm_hash_remove_all(unsigned flags)
for (n = rb_first(&name_rb_tree); n; n = rb_next(n)) {
if (flags & DM_REMOVE_INTERRUPTIBLE && fatal_signal_pending(current)) {
up_write(&_hash_lock);
+ dm_ima_free_context(ima_context);
return -EINTR;
}
@@ -367,6 +445,7 @@ static int dm_hash_remove_all(unsigned flags)
continue;
}
+ ima_idx = dm_ima_init_context(hc, ima_context, true);
t = __hash_remove(hc);
up_write(&_hash_lock);
@@ -375,7 +454,7 @@ static int dm_hash_remove_all(unsigned flags)
dm_sync_table(md);
dm_table_destroy(t);
}
- dm_ima_measure_on_device_remove(md, true);
+ dm_ima_measure_on_device_remove(md, true, ima_context, ima_idx);
dm_put(md);
if (likely(flags & DM_REMOVE_KEEP_OPEN_DEVICES))
dm_destroy(md);
@@ -396,6 +475,7 @@ static int dm_hash_remove_all(unsigned flags)
if (dev_skipped && !(flags & DM_REMOVE_ONLY_DEFERRED))
DMWARN("remove_all left %d open device(s)", dev_skipped);
+ dm_ima_free_context(ima_context);
return 0;
}
@@ -443,6 +523,7 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
struct mapped_device *md;
unsigned int change_uuid = (param->flags & DM_UUID_FLAG) ? 1 : 0;
int srcu_idx;
+ struct dm_ima_context *ima_context = NULL;
/*
* duplicate new.
@@ -451,6 +532,7 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
if (!new_data)
return ERR_PTR(-ENOMEM);
+ dm_ima_alloc_context(&ima_context, true);
down_write(&_hash_lock);
/*
@@ -467,6 +549,7 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
param->name, new);
dm_put(hc->md);
up_write(&_hash_lock);
+ dm_ima_free_context(ima_context);
kfree(new_data);
return ERR_PTR(-EBUSY);
}
@@ -479,6 +562,7 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
DMERR("Unable to rename non-existent device, %s to %s%s",
param->name, change_uuid ? "uuid " : "", new);
up_write(&_hash_lock);
+ dm_ima_free_context(ima_context);
kfree(new_data);
return ERR_PTR(-ENXIO);
}
@@ -492,6 +576,7 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
param->name, new, hc->uuid);
dm_put(hc->md);
up_write(&_hash_lock);
+ dm_ima_free_context(ima_context);
kfree(new_data);
return ERR_PTR(-EINVAL);
}
@@ -514,9 +599,11 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
md = hc->md;
- dm_ima_measure_on_device_rename(md);
+ dm_ima_init_context(hc, ima_context, false);
up_write(&_hash_lock);
+ dm_ima_measure_on_device_rename(md, ima_context);
+ dm_ima_free_context(ima_context);
kfree(old_name);
return md;
@@ -995,13 +1082,17 @@ static int dev_remove(struct file *filp, struct dm_ioctl *param, size_t param_si
struct mapped_device *md;
int r;
struct dm_table *t;
+ struct dm_ima_context *ima_context = NULL;
+ unsigned int ima_idx;
+ dm_ima_alloc_context(&ima_context, true);
down_write(&_hash_lock);
hc = __find_device_hash_cell(param);
if (!hc) {
DMDEBUG_LIMIT("device doesn't appear to be in the dev hash table.");
up_write(&_hash_lock);
+ dm_ima_free_context(ima_context);
return -ENXIO;
}
@@ -1015,14 +1106,17 @@ static int dev_remove(struct file *filp, struct dm_ioctl *param, size_t param_si
if (r == -EBUSY && param->flags & DM_DEFERRED_REMOVE) {
up_write(&_hash_lock);
dm_put(md);
+ dm_ima_free_context(ima_context);
return 0;
}
DMDEBUG_LIMIT("unable to remove open device %s", hc->name);
up_write(&_hash_lock);
dm_put(md);
+ dm_ima_free_context(ima_context);
return r;
}
+ ima_idx = dm_ima_init_context(hc, ima_context, true);
t = __hash_remove(hc);
up_write(&_hash_lock);
@@ -1033,7 +1127,8 @@ static int dev_remove(struct file *filp, struct dm_ioctl *param, size_t param_si
param->flags &= ~DM_DEFERRED_REMOVE;
- dm_ima_measure_on_device_remove(md, false);
+ dm_ima_measure_on_device_remove(md, false, ima_context, ima_idx);
+ dm_ima_free_context(ima_context);
if (!dm_kobject_uevent(md, KOBJ_REMOVE, param->event_nr, false))
param->flags |= DM_UEVENT_GENERATED_FLAG;
@@ -1169,13 +1264,16 @@ static int do_resume(struct dm_ioctl *param)
struct mapped_device *md;
struct dm_table *new_map, *old_map = NULL;
bool need_resize_uevent = false;
+ struct dm_ima_context *ima_context = NULL;
+ dm_ima_alloc_context(&ima_context, true);
down_write(&_hash_lock);
hc = __find_device_hash_cell(param);
if (!hc) {
DMDEBUG_LIMIT("device doesn't appear to be in the dev hash table.");
up_write(&_hash_lock);
+ dm_ima_free_context(ima_context);
return -ENXIO;
}
@@ -1184,13 +1282,15 @@ static int do_resume(struct dm_ioctl *param)
new_map = hc->new_map;
hc->new_map = NULL;
param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
-
+ if (new_map)
+ dm_ima_init_context(hc, ima_context, false);
up_write(&_hash_lock);
/* Do we need to load a new map ? */
if (new_map) {
sector_t old_size, new_size;
+ dm_ima_context_table_op(md, ima_context, DM_IMA_TABLE_SAVE);
/* Suspend if it isn't already suspended */
if (param->flags & DM_SKIP_LOCKFS_FLAG)
suspend_flags &= ~DM_SUSPEND_LOCKFS_FLAG;
@@ -1204,6 +1304,8 @@ static int do_resume(struct dm_ioctl *param)
if (hc && !hc->new_map) {
hc->new_map = new_map;
new_map = NULL;
+ dm_ima_init_context(hc, ima_context,
+ false);
} else {
r = -ENXIO;
}
@@ -1211,7 +1313,9 @@ static int do_resume(struct dm_ioctl *param)
if (new_map) {
dm_sync_table(md);
dm_table_destroy(new_map);
- }
+ } else
+ dm_ima_context_table_op(md, ima_context, DM_IMA_TABLE_RESTORE);
+ dm_ima_free_context(ima_context);
dm_put(md);
return r;
}
@@ -1222,9 +1326,12 @@ static int do_resume(struct dm_ioctl *param)
if (IS_ERR(old_map)) {
dm_sync_table(md);
dm_table_destroy(new_map);
+ dm_ima_free_context(ima_context);
dm_put(md);
return PTR_ERR(old_map);
}
+ if (dm_ima_need_measure(md, new_map, ima_context))
+ dm_ima_measure_on_device_resume(md, true, ima_context);
new_size = dm_get_size(md);
if (old_size && new_size && old_size != new_size)
need_resize_uevent = true;
@@ -1238,7 +1345,10 @@ static int do_resume(struct dm_ioctl *param)
if (dm_suspended_md(md)) {
r = dm_resume(md);
if (!r) {
- dm_ima_measure_on_device_resume(md, new_map ? true : false);
+ if (!new_map && dm_ima_need_measure(md, NULL,
+ ima_context))
+ dm_ima_measure_on_device_resume(md, false,
+ ima_context);
if (!dm_kobject_uevent(md, KOBJ_CHANGE, param->event_nr, need_resize_uevent))
param->flags |= DM_UEVENT_GENERATED_FLAG;
@@ -1255,6 +1365,7 @@ static int do_resume(struct dm_ioctl *param)
if (!r)
__dev_status(md, param);
+ dm_ima_free_context(ima_context);
dm_put(md);
return r;
}
@@ -1528,11 +1639,12 @@ static bool is_valid_type(enum dm_queue_mode cur, enum dm_queue_mode new)
static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_size)
{
- int r;
+ int r, srcu_idx;
struct hash_cell *hc;
struct dm_table *t, *old_map = NULL;
struct mapped_device *md;
struct target_type *immutable_target_type;
+ struct dm_ima_context *ima_context = NULL;
md = find_device(param);
if (!md)
@@ -1548,8 +1660,6 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si
if (r)
goto err_unlock_md_type;
- dm_ima_measure_on_table_load(t);
-
immutable_target_type = dm_get_immutable_target_type(md);
if (immutable_target_type &&
(immutable_target_type != dm_table_get_immutable_target_type(t)) &&
@@ -1576,12 +1686,14 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si
dm_unlock_md_type(md);
+ dm_ima_alloc_context(&ima_context, false);
/* stage inactive table */
down_write(&_hash_lock);
hc = dm_get_mdptr(md);
if (!hc) {
DMERR("device has been removed from the dev hash table.");
up_write(&_hash_lock);
+ dm_ima_free_context(ima_context);
r = -ENXIO;
goto err_destroy_table;
}
@@ -1589,8 +1701,15 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si
if (hc->new_map)
old_map = hc->new_map;
hc->new_map = t;
+ dm_ima_init_context(hc, ima_context, false);
+ /* Make sure new_map doesn't get freed before we measure it*/
+ dm_get_live_table(md, &srcu_idx);
up_write(&_hash_lock);
+ dm_ima_measure_on_table_load(t, ima_context);
+ dm_ima_free_context(ima_context);
+ dm_put_live_table(md, srcu_idx);
+
param->flags |= DM_INACTIVE_PRESENT_FLAG;
__dev_status(md, param);
@@ -1619,13 +1738,16 @@ static int table_clear(struct file *filp, struct dm_ioctl *param, size_t param_s
struct mapped_device *md;
struct dm_table *old_map = NULL;
bool has_new_map = false;
+ struct dm_ima_context *ima_context = NULL;
+ dm_ima_alloc_context(&ima_context, true);
down_write(&_hash_lock);
hc = __find_device_hash_cell(param);
if (!hc) {
DMDEBUG_LIMIT("device doesn't appear to be in the dev hash table.");
up_write(&_hash_lock);
+ dm_ima_free_context(ima_context);
return -ENXIO;
}
@@ -1635,8 +1757,11 @@ static int table_clear(struct file *filp, struct dm_ioctl *param, size_t param_s
has_new_map = true;
}
+ dm_ima_init_context(hc, ima_context, false);
md = hc->md;
up_write(&_hash_lock);
+ dm_ima_measure_on_table_clear(md, has_new_map, ima_context);
+ dm_ima_free_context(ima_context);
param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
__dev_status(md, param);
@@ -1645,7 +1770,6 @@ static int table_clear(struct file *filp, struct dm_ioctl *param, size_t param_s
dm_sync_table(md);
dm_table_destroy(old_map);
}
- dm_ima_measure_on_table_clear(md, has_new_map);
dm_put(md);
return 0;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8b60c9804f5b..87011c41ef7b 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2546,6 +2546,8 @@ int dm_create(int minor, struct mapped_device **result)
if (!md)
return -ENXIO;
+ dm_ima_init(md);
+
*result = md;
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 02/10] dm-ima: remove broken last_target_measured logic
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
When it ran out of space for adding more targets to the ima_buf,
dm_ima_measure_on_table_load() would measure the dm device early, and
then add the rest of the targets and measure it again.
last_target_measured was intended to flag the last target measured so
that the device wouldn't get remeasured, if no new targets were added
after the early measurement. But the way to code works, the dm device
will never be measured early unless there is another target to add to
the ima_buf. Instead, if there is only one more target to add, that
target was getting added to the ima_buf, but it wasn't getting
remeasured, because last_target_measured was set. Since
dm_ima_measure_on_table_load() only measures a device early when there
are more targets to add, the final measurement must always happen, and
last_target_measured is unneeded.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index a639bb0fe6c3..209221fa8bc5 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -167,7 +167,6 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
size_t device_data_buf_len, target_metadata_buf_len, target_data_buf_len, l = 0;
char *target_metadata_buf = NULL, *target_data_buf = NULL, *digest_buf = NULL;
char *ima_buf = NULL, *device_data_buf = NULL;
- int last_target_measured = -1;
status_type_t type = STATUSTYPE_IMA;
size_t cur_total_buf_len = 0;
unsigned int num_targets, i;
@@ -205,8 +204,6 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
for (i = 0; i < num_targets; i++) {
struct dm_target *ti = dm_table_get_target(table, i);
- last_target_measured = 0;
-
/*
* First retrieve the target metadata.
*/
@@ -256,14 +253,6 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
memcpy(ima_buf + l, device_data_buf, device_data_buf_len);
l += device_data_buf_len;
-
- /*
- * If this iteration of the for loop turns out to be the last target
- * in the table, dm_ima_measure_data("dm_table_load", ...) doesn't need
- * to be called again, just the hash needs to be finalized.
- * "last_target_measured" tracks this state.
- */
- last_target_measured = 1;
}
/*
@@ -277,11 +266,8 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
l += target_data_buf_len;
}
- if (!last_target_measured) {
- dm_ima_measure_data(table_load_event_name, ima_buf, l, noio);
-
- sha256_update(&hash_ctx, (const u8 *)ima_buf, l);
- }
+ dm_ima_measure_data(table_load_event_name, ima_buf, l, noio);
+ sha256_update(&hash_ctx, (const u8 *)ima_buf, l);
/*
* Finalize the table hash, and store it in table->md->ima.inactive_table.hash,
--
2.53.0
^ permalink raw reply related
* [PATCH 08/10] dm-ima: Handle race between rename and table swap
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
a device rename could happen after do_resume() removed the inactive
table that it was swapping to out of the hash cell, but before it was
made the active table. In this case, the table metadata would still
have the old name. Update the swapped table's metadata to avoid this.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 5c6f3f8761a8..4631dc2a6d4d 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -367,6 +367,19 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap,
kfree(md->ima.active_table.device_metadata);
md->ima.active_table = context->table;
memset(&context->table, 0, sizeof(context->table));
+ if (md->ima.active_table.device_metadata) {
+ /*
+ * A rename could have happened while the swap was
+ * going on. In that case, the saved table info would
+ * still have the old name. Update the metadata to be
+ * sure that it has the current name
+ */
+ struct dm_ima_device_table_metadata *table = &md->ima.active_table;
+ fix_context_strings(context);
+ dm_ima_copy_device_data(md, table->device_metadata,
+ context, table->num_targets);
+ table->device_metadata_len = strlen(table->device_metadata);
+ }
}
if (md->ima.active_table.device_metadata) {
--
2.53.0
^ permalink raw reply related
* [PATCH 01/10] dm-ima: remove dm_ima_reset_data()
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
There's no point in saving the string length of DM_IMA_VERSION_STR. It's
a constant, so the compiler will precompute it. dm_create() will already
zero out the rest of dm->ima.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 32 ++++++++++++--------------------
drivers/md/dm-ima.h | 3 ---
drivers/md/dm.c | 2 --
3 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 9495ca035056..a639bb0fe6c3 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -159,15 +159,6 @@ static int dm_ima_alloc_and_copy_capacity_str(struct mapped_device *md, char **c
capacity);
}
-/*
- * Initialize/reset the dm ima related data structure variables.
- */
-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);
-}
-
/*
* Build up the IMA data for each target, and finally measure.
*/
@@ -204,8 +195,8 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
sha256_init(&hash_ctx);
- memcpy(ima_buf + l, DM_IMA_VERSION_STR, table->md->ima.dm_version_str_len);
- l += table->md->ima.dm_version_str_len;
+ memcpy(ima_buf + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
+ l += strlen(DM_IMA_VERSION_STR);
device_data_buf_len = strlen(device_data_buf);
memcpy(ima_buf + l, device_data_buf, device_data_buf_len);
@@ -260,8 +251,8 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
* prefix, so that multiple records from the same "dm_table_load" for
* a given device can be linked together.
*/
- memcpy(ima_buf + l, DM_IMA_VERSION_STR, table->md->ima.dm_version_str_len);
- l += table->md->ima.dm_version_str_len;
+ memcpy(ima_buf + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
+ l += strlen(DM_IMA_VERSION_STR);
memcpy(ima_buf + l, device_data_buf, device_data_buf_len);
l += device_data_buf_len;
@@ -349,8 +340,8 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
if (capacity_len < 0)
goto error;
- memcpy(device_table_data + l, DM_IMA_VERSION_STR, md->ima.dm_version_str_len);
- l += md->ima.dm_version_str_len;
+ memcpy(device_table_data + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
+ l += strlen(DM_IMA_VERSION_STR);
if (swap) {
if (md->ima.active_table.hash != md->ima.inactive_table.hash)
@@ -460,8 +451,8 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
goto exit;
}
- memcpy(device_table_data + l, DM_IMA_VERSION_STR, md->ima.dm_version_str_len);
- l += md->ima.dm_version_str_len;
+ memcpy(device_table_data + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
+ l += strlen(DM_IMA_VERSION_STR);
if (md->ima.active_table.device_metadata) {
memcpy(device_table_data + l, device_active_str, device_active_len);
@@ -551,7 +542,8 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
if (md->ima.active_table.hash != md->ima.inactive_table.hash)
kfree(md->ima.inactive_table.hash);
- dm_ima_reset_data(md);
+ memset(&md->ima.active_table, 0, sizeof(md->ima.active_table));
+ memset(&md->ima.inactive_table, 0, sizeof(md->ima.inactive_table));
kfree(dev_name);
kfree(dev_uuid);
@@ -578,8 +570,8 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map)
if (capacity_len < 0)
goto error1;
- memcpy(device_table_data + l, DM_IMA_VERSION_STR, md->ima.dm_version_str_len);
- l += md->ima.dm_version_str_len;
+ memcpy(device_table_data + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
+ l += strlen(DM_IMA_VERSION_STR);
if (md->ima.inactive_table.device_metadata_len &&
md->ima.inactive_table.hash_len) {
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index a403deca6093..b0b166aa2283 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -52,10 +52,8 @@ struct dm_ima_device_table_metadata {
struct dm_ima_measurements {
struct dm_ima_device_table_metadata active_table;
struct dm_ima_device_table_metadata inactive_table;
- unsigned int dm_version_str_len;
};
-void dm_ima_reset_data(struct mapped_device *md);
void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags);
void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap);
void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all);
@@ -64,7 +62,6 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md);
#else
-static inline void dm_ima_reset_data(struct mapped_device *md) {}
static inline void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags) {}
static inline void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap) {}
static inline void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all) {}
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index e178fe19973e..8b60c9804f5b 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2546,8 +2546,6 @@ int dm_create(int minor, struct mapped_device **result)
if (!md)
return -ENXIO;
- dm_ima_reset_data(md);
-
*result = md;
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 03/10] dm-ima: Remove status_flags from dm_ima_measure_on_table_load()
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
There is no status flag that is is used for STATUSTYPE_IMA type
status() calls, and STATUSTYPE_IMA itself is not a valid
status flag.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 4 ++--
drivers/md/dm-ima.h | 4 ++--
drivers/md/dm-ioctl.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 209221fa8bc5..8b84b676cad4 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -162,7 +162,7 @@ static int dm_ima_alloc_and_copy_capacity_str(struct mapped_device *md, char **c
/*
* Build up the IMA data for each target, and finally measure.
*/
-void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags)
+void dm_ima_measure_on_table_load(struct dm_table *table)
{
size_t device_data_buf_len, target_metadata_buf_len, target_data_buf_len, l = 0;
char *target_metadata_buf = NULL, *target_data_buf = NULL, *digest_buf = NULL;
@@ -217,7 +217,7 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
* Then retrieve the actual target data.
*/
if (ti->type->status)
- ti->type->status(ti, type, status_flags, target_data_buf,
+ ti->type->status(ti, type, 0, target_data_buf,
DM_IMA_TARGET_DATA_BUF_LEN);
else
target_data_buf[0] = '\0';
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index b0b166aa2283..c0548492bef0 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -54,7 +54,7 @@ struct dm_ima_measurements {
struct dm_ima_device_table_metadata inactive_table;
};
-void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags);
+void dm_ima_measure_on_table_load(struct dm_table *table);
void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap);
void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all);
void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map);
@@ -62,7 +62,7 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md);
#else
-static inline void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags) {}
+static inline void dm_ima_measure_on_table_load(struct dm_table *table) {}
static inline void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap) {}
static inline void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all) {}
static inline void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map) {}
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 405acc14d718..1a0dd4981d03 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1548,7 +1548,7 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si
if (r)
goto err_unlock_md_type;
- dm_ima_measure_on_table_load(t, STATUSTYPE_IMA);
+ dm_ima_measure_on_table_load(t);
immutable_target_type = dm_get_immutable_target_type(md);
if (immutable_target_type &&
--
2.53.0
^ permalink raw reply related
* [PATCH 06/10] dm-ima: remove new_map from dm_ima_measure_on_device_clear
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
Now that two processes can't modify md->ima in
dm_ima_measure_on_device_clear() at the same time, there's no need to
track if an inactive table was actually removed. We might as well
clean it up unconditionally, on the off chance that a previous
ima measurement failed and left md->ima.inactive_table behind.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 10 ++++------
drivers/md/dm-ima.h | 3 +--
drivers/md/dm-ioctl.c | 4 +---
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index e8fa049c6317..317f6d7e0e5e 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -549,7 +549,7 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all,
/*
* Measure ima data on table clear.
*/
-void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map,
+void dm_ima_measure_on_table_clear(struct mapped_device *md,
struct dm_ima_context *context)
{
unsigned int l = 0;
@@ -612,11 +612,9 @@ 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) {
- kfree(md->ima.inactive_table.hash);
- kfree(md->ima.inactive_table.device_metadata);
- memset(&md->ima.inactive_table, 0, sizeof(md->ima.inactive_table));
- }
+ kfree(md->ima.inactive_table.hash);
+ kfree(md->ima.inactive_table.device_metadata);
+ memset(&md->ima.inactive_table, 0, sizeof(md->ima.inactive_table));
error:
kfree(capacity_str);
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index 9e1b654867d9..2f2ac69042f2 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -82,7 +82,7 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap,
void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all,
struct dm_ima_context *context,
unsigned int idx);
-void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map,
+void dm_ima_measure_on_table_clear(struct mapped_device *md,
struct dm_ima_context *context);
void dm_ima_measure_on_device_rename(struct mapped_device *md,
struct dm_ima_context *context);
@@ -107,7 +107,6 @@ static inline void dm_ima_measure_on_device_remove(struct mapped_device *md,
struct dm_ima_context *context,
unsigned int idx) {}
static inline void dm_ima_measure_on_table_clear(struct mapped_device *md,
- bool new_map,
struct dm_ima_context *context) {}
static inline void dm_ima_measure_on_device_rename(struct mapped_device *md,
struct dm_ima_context *context) {}
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 16609882aa92..2288f3d58ce9 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1737,7 +1737,6 @@ static int table_clear(struct file *filp, struct dm_ioctl *param, size_t param_s
struct hash_cell *hc;
struct mapped_device *md;
struct dm_table *old_map = NULL;
- bool has_new_map = false;
struct dm_ima_context *ima_context = NULL;
dm_ima_alloc_context(&ima_context, true);
@@ -1754,13 +1753,12 @@ static int table_clear(struct file *filp, struct dm_ioctl *param, size_t param_s
if (hc->new_map) {
old_map = hc->new_map;
hc->new_map = NULL;
- has_new_map = true;
}
dm_ima_init_context(hc, ima_context, false);
md = hc->md;
up_write(&_hash_lock);
- dm_ima_measure_on_table_clear(md, has_new_map, ima_context);
+ dm_ima_measure_on_table_clear(md, ima_context);
dm_ima_free_context(ima_context);
param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
--
2.53.0
^ permalink raw reply related
* [RFC PATCH 00/10] Fix dm-ima bugs
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
The dm-ima code does not guarantee that the dm_ima_measure_on_*
functions will not be called at the same time. Since they modify and
free shared memory, this can lead to Use-After-Free errors or garbage
measurements. Further, they don't make sure that the state they measure
corresponds to the actual device state. For instance if table_load()
runs at the same time as do_resume() on a table swap,
dm_ima_measure_on_device_resume() can end up thinking the wrong table is
active. Or a concurrent dm_hash_rename() and a table swap, can end up
with a the new active table still using the old name. This patchset
makes sure the the dm-ima function are serialized and report the correct
device state.
However, the code is still messier that in could be. This is because
it duplicates the current measurement events and format. I would really
like to know if that is necessary. Specifically, it currently measures
the following dm device and table actions:
load
clear
rename
resume
remove
I don't see the benefit of reporting changes to the inactive table, or
resumes where the device does not change state. From the user's point of
view, the device is still the same after these events. At the same
time, it doesn't measure device creates if no table was loaded, so you
can have situations where the the first measurement for a device is a
rename or a remove. A more sensible set of actions to measure would be:
create
table_swap
rename
remove
Also, the measurement format doesn't map well to how dm device's are
actually set up, in a way that makes it harder for the code and records
extraneous information. First, like I mentioned before, I don't see the
benefit of measuring the inactive table. Second, the name, uuid, and
major/minor numbers are properties of the device, not it's table (and dm
devices can't have partitions, so the minor count will always be 1). I
don't see a reason to store and occasinally log this information twice,
if there is an active and incative table, and it forces extra
coordination between the dm_ima_measure_on_* functions.
I'm wondering it we are stuck with the current events and format, now
that this has been released? Or could we bump the version, and change
what events we measure, and how we format the output?
Benjamin Marzinski (10):
dm-ima: remove dm_ima_reset_data()
dm-ima: remove broken last_target_measured logic
dm-ima: Remove status_flags from dm_ima_measure_on_table_load()
dm-ima: don't copy the active table to the inactive table
dm-ima: Fix UAF errors and measuring incorrect context
dm-ima: remove new_map from dm_ima_measure_on_device_clear
dm-ima: Fix issues with dm_ima_measure_on_device_rename
dm-ima: Handle race between rename and table swap
dm-ima: Fail more gracefully in dm_ima_measure_on_*
dm-ima: use active table's size if available
drivers/md/dm-ima.c | 506 +++++++++++++++++++-----------------------
drivers/md/dm-ima.h | 67 ++++--
drivers/md/dm-ioctl.c | 146 +++++++++++-
drivers/md/dm.c | 2 +-
4 files changed, 421 insertions(+), 300 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH 07/10] dm-ima: Fix issues with dm_ima_measure_on_device_rename
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
dm_ima_measure_on_device_rename() can be called on a device before it
ever loads a table, so it needs to handle the case where there is no
table metadata. Also, it was only updating the table_metadata on the
active table. If there was an inactive table when the device was
and that table was later swapped in as the active table, it would
still have the old name. dm_ima_measure_on_device_rename() was also
needlessly allocating new memory for the updated table metadata, instead
of just reusing the existing memory.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 69 ++++++++++++++++++++++++---------------------
1 file changed, 37 insertions(+), 32 deletions(-)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 317f6d7e0e5e..5c6f3f8761a8 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -119,22 +119,18 @@ void dm_ima_context_table_op(struct mapped_device *md,
}
/*
- * Internal function to allocate and copy device data for IMA measurements.
+ * Internal function to copy device data for IMA measurements.
*/
-static int dm_ima_alloc_and_copy_device_data(struct mapped_device *md, char **device_data,
- struct dm_ima_context *context,
- unsigned int num_targets, bool noio)
+static void dm_ima_copy_device_data(struct mapped_device *md, char *device_data,
+ struct dm_ima_context *context,
+ unsigned int num_targets)
{
- *device_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, noio);
- if (!(*device_data))
- return -ENOMEM;
-
- scnprintf(*device_data, DM_IMA_DEVICE_BUF_LEN,
+ memset(device_data, 0, DM_IMA_DEVICE_BUF_LEN);
+ scnprintf(device_data, DM_IMA_DEVICE_BUF_LEN,
"name=%s,uuid=%s,major=%d,minor=%d,minor_count=%d,num_targets=%u;",
context->dev_name, context->dev_uuid, md->disk->major,
md->disk->first_minor, md->disk->minors, num_targets);
- return 0;
}
/*
@@ -212,11 +208,14 @@ void dm_ima_measure_on_table_load(struct dm_table *table,
num_targets = table->num_targets;
- fix_context_strings(context);
- if (dm_ima_alloc_and_copy_device_data(table->md, &device_data_buf,
- context, num_targets, noio))
+ device_data_buf = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, noio);
+ if (!device_data_buf)
goto error;
+ fix_context_strings(context);
+ dm_ima_copy_device_data(table->md, device_data_buf, context,
+ num_targets);
+
sha256_init(&hash_ctx);
memcpy(ima_buf + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
@@ -631,10 +630,11 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md,
void dm_ima_measure_on_device_rename(struct mapped_device *md,
struct dm_ima_context *context)
{
- char *old_device_data = NULL, *new_device_data = NULL;
+ char *old_device_data = NULL;
char *combined_device_data = NULL, *capacity_str = NULL;
bool noio = true;
int len;
+ struct dm_ima_device_table_metadata *table;
if (unlikely(!context))
return;
@@ -644,38 +644,43 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md,
context->update_idx));
smp_mb();
- fix_context_strings(context);
- if (dm_ima_alloc_and_copy_device_data(md, &new_device_data, context,
- md->ima.active_table.num_targets,
- noio))
- goto error;
-
combined_device_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN * 2, noio);
if (!combined_device_data)
- goto error;
+ goto exit;
if (dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio) < 0)
- goto error;
-
- old_device_data = md->ima.active_table.device_metadata;
-
- md->ima.active_table.device_metadata = new_device_data;
- md->ima.active_table.device_metadata_len = strlen(new_device_data);
+ goto exit;
+ if (md->ima.active_table.device_metadata)
+ old_device_data = md->ima.active_table.device_metadata;
+ else if (md->ima.inactive_table.device_metadata)
+ old_device_data = md->ima.inactive_table.device_metadata;
+ else
+ old_device_data = "table_rename=no_data;";
+ fix_context_strings(context);
len = scnprintf(combined_device_data, DM_IMA_DEVICE_BUF_LEN * 2,
"%s%snew_name=%s,new_uuid=%s;%s", DM_IMA_VERSION_STR, old_device_data,
context->dev_name, context->dev_uuid, capacity_str);
- dm_ima_measure_data("dm_device_rename", combined_device_data, len, noio);
+ if (md->ima.active_table.device_metadata) {
+ table = &md->ima.active_table;
+ dm_ima_copy_device_data(md, table->device_metadata, context,
+ table->num_targets);
+ table->device_metadata_len = strlen(table->device_metadata);
+ }
- goto exit;
+ if (md->ima.inactive_table.device_metadata) {
+ table = &md->ima.inactive_table;
+ dm_ima_copy_device_data(md, table->device_metadata, context,
+ table->num_targets);
+ table->device_metadata_len = strlen(table->device_metadata);
+ }
+
+ dm_ima_measure_data("dm_device_rename", combined_device_data, len, noio);
-error:
- kfree(new_device_data);
exit:
kfree(capacity_str);
kfree(combined_device_data);
- kfree(old_device_data);
smp_mb__before_atomic();
atomic_inc(&md->ima.measure_idx);
--
2.53.0
^ permalink raw reply related
* [PATCH 04/10] dm-ima: don't copy the active table to the inactive table
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
If an inactive table was cleared, dm_ima_measure_on_table_clear() was
copying the ima.active_table to ima.inactive_table. This is not what
device-mapper does, and it makes the IMA measurements show an inactive
table when there isn't one. Also, once this is removed, the code no
longer needs to keep checking if the active and the inactive table point
to the same memory.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 64 +++++++--------------------------------------
1 file changed, 10 insertions(+), 54 deletions(-)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 8b84b676cad4..c141068bc6b4 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -281,17 +281,13 @@ void dm_ima_measure_on_table_load(struct dm_table *table)
if (!digest_buf)
goto error;
- if (table->md->ima.active_table.hash != table->md->ima.inactive_table.hash)
- kfree(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);
table->md->ima.inactive_table.num_targets = num_targets;
- if (table->md->ima.active_table.device_metadata !=
- table->md->ima.inactive_table.device_metadata)
- kfree(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;
@@ -330,19 +326,9 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
l += strlen(DM_IMA_VERSION_STR);
if (swap) {
- 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;
-
- 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;
- md->ima.active_table.num_targets = 0;
+ kfree(md->ima.active_table.hash);
+ kfree(md->ima.active_table.device_metadata);
+ memset(&md->ima.active_table, 0, sizeof(md->ima.active_table));
if (md->ima.inactive_table.hash) {
md->ima.active_table.hash = md->ima.inactive_table.hash;
@@ -518,15 +504,10 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
kfree(capacity_str);
exit:
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);
+ kfree(md->ima.inactive_table.device_metadata);
kfree(md->ima.active_table.hash);
-
- if (md->ima.active_table.hash != md->ima.inactive_table.hash)
- kfree(md->ima.inactive_table.hash);
+ kfree(md->ima.inactive_table.hash);
memset(&md->ima.active_table, 0, sizeof(md->ima.active_table));
memset(&md->ima.inactive_table, 0, sizeof(md->ima.inactive_table));
@@ -594,34 +575,9 @@ 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) {
- 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;
-
- 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;
- md->ima.inactive_table.num_targets = 0;
-
- if (md->ima.active_table.hash) {
- md->ima.inactive_table.hash = md->ima.active_table.hash;
- md->ima.inactive_table.hash_len = md->ima.active_table.hash_len;
- }
-
- if (md->ima.active_table.device_metadata) {
- md->ima.inactive_table.device_metadata =
- md->ima.active_table.device_metadata;
- md->ima.inactive_table.device_metadata_len =
- md->ima.active_table.device_metadata_len;
- md->ima.inactive_table.num_targets =
- md->ima.active_table.num_targets;
- }
+ kfree(md->ima.inactive_table.hash);
+ kfree(md->ima.inactive_table.device_metadata);
+ memset(&md->ima.inactive_table, 0, sizeof(md->ima.inactive_table));
}
kfree(dev_name);
--
2.53.0
^ permalink raw reply related
* [PATCH 09/10] dm-ima: Fail more gracefully in dm_ima_measure_on_*
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
In all the dm_ima_measure_on_* functions besides
dm_ima_measure_on_table_load(), even if measuring the event fails, it's
still possible to update dm->ima, so that it continues to correctly
track the device state. This means that one measurement failure won't
cause future measurements to record the wrong data.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 4631dc2a6d4d..45038bd65f7b 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -351,17 +351,6 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap,
context->update_idx));
smp_mb();
- device_table_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, noio);
- if (!device_table_data)
- goto error;
-
- capacity_len = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
- if (capacity_len < 0)
- goto error;
-
- memcpy(device_table_data + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
- l += strlen(DM_IMA_VERSION_STR);
-
if (swap) {
kfree(md->ima.active_table.hash);
kfree(md->ima.active_table.device_metadata);
@@ -382,6 +371,17 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap,
}
}
+ device_table_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, noio);
+ if (!device_table_data)
+ goto error;
+
+ capacity_len = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
+ if (capacity_len < 0)
+ goto error;
+
+ memcpy(device_table_data + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
+ l += strlen(DM_IMA_VERSION_STR);
+
if (md->ima.active_table.device_metadata) {
memcpy(device_table_data + l, md->ima.active_table.device_metadata,
md->ima.active_table.device_metadata_len);
@@ -624,11 +624,11 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md,
dm_ima_measure_data("dm_table_clear", device_table_data, l, noio);
+error:
kfree(md->ima.inactive_table.hash);
kfree(md->ima.inactive_table.device_metadata);
memset(&md->ima.inactive_table, 0, sizeof(md->ima.inactive_table));
-error:
kfree(capacity_str);
kfree(device_table_data);
@@ -657,6 +657,8 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md,
context->update_idx));
smp_mb();
+ fix_context_strings(context);
+
combined_device_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN * 2, noio);
if (!combined_device_data)
goto exit;
@@ -670,11 +672,15 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md,
old_device_data = md->ima.inactive_table.device_metadata;
else
old_device_data = "table_rename=no_data;";
- fix_context_strings(context);
len = scnprintf(combined_device_data, DM_IMA_DEVICE_BUF_LEN * 2,
"%s%snew_name=%s,new_uuid=%s;%s", DM_IMA_VERSION_STR, old_device_data,
context->dev_name, context->dev_uuid, capacity_str);
+ dm_ima_measure_data("dm_device_rename", combined_device_data, len, noio);
+exit:
+ kfree(capacity_str);
+ kfree(combined_device_data);
+
if (md->ima.active_table.device_metadata) {
table = &md->ima.active_table;
dm_ima_copy_device_data(md, table->device_metadata, context,
@@ -689,12 +695,6 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md,
table->device_metadata_len = strlen(table->device_metadata);
}
- dm_ima_measure_data("dm_device_rename", combined_device_data, len, noio);
-
-exit:
- kfree(capacity_str);
- kfree(combined_device_data);
-
smp_mb__before_atomic();
atomic_inc(&md->ima.measure_idx);
wake_up_all(&md->ima.ima_wq);
--
2.53.0
^ permalink raw reply related
* [PATCH 10/10] dm-ima: use active table's size if available
From: Benjamin Marzinski @ 2026-04-14 0:22 UTC (permalink / raw)
To: Mikulas Patocka, Mike Snitzer
Cc: dm-devel, linux-integrity, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
It is possible that the dm_ima_measure_on_* functions run at the same
time as a table is getting swapped, but before the md->ima.active_table
is updated by dm_ima_measure_on_device_resume(). Instead of using the
current device size, use the size of the active table that is being
measured (assuming there is one), so the information is consistent.
Also, don't allocate a separate string to hold the capactiy. Just
print it directly to the measurement buffer.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
drivers/md/dm-ima.c | 79 +++++++++++----------------------------------
drivers/md/dm-ima.h | 1 +
2 files changed, 20 insertions(+), 60 deletions(-)
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 45038bd65f7b..ca6a33a0275e 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -151,22 +151,10 @@ static void dm_ima_measure_data(const char *event_name, const void *buf, size_t
memalloc_noio_restore(noio_flag);
}
-/*
- * Internal function to allocate and copy current device capacity for IMA measurements.
- */
-static int dm_ima_alloc_and_copy_capacity_str(struct mapped_device *md, char **capacity_str,
- bool noio)
+static sector_t dm_ima_capacity(struct mapped_device *md)
{
- sector_t capacity;
-
- capacity = get_capacity(md->disk);
-
- *capacity_str = dm_ima_alloc(DM_IMA_DEVICE_CAPACITY_BUF_LEN, noio);
- if (!(*capacity_str))
- return -ENOMEM;
-
- return scnprintf(*capacity_str, DM_IMA_DEVICE_BUF_LEN, "current_device_capacity=%llu;",
- capacity);
+ return (md->ima.active_table.device_metadata) ?
+ md->ima.active_table.capacity : get_capacity(md->disk);
}
/*
@@ -309,6 +297,7 @@ void dm_ima_measure_on_table_load(struct dm_table *table,
table->md->ima.inactive_table.hash = digest_buf;
table->md->ima.inactive_table.hash_len = strlen(digest_buf);
table->md->ima.inactive_table.num_targets = num_targets;
+ table->md->ima.inactive_table.capacity = dm_table_get_size(table);
kfree(table->md->ima.inactive_table.device_metadata);
@@ -335,13 +324,12 @@ void dm_ima_measure_on_table_load(struct dm_table *table,
void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap,
struct dm_ima_context *context)
{
- char *device_table_data = NULL, *capacity_str = NULL;
+ char *device_table_data = NULL;
char active[] = "active_table_hash=";
unsigned int active_len = strlen(active);
unsigned int l = 0;
bool noio = true;
bool nodata = true;
- int capacity_len;
if (unlikely(!context))
return;
@@ -375,10 +363,6 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap,
if (!device_table_data)
goto error;
- capacity_len = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
- if (capacity_len < 0)
- goto error;
-
memcpy(device_table_data + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
l += strlen(DM_IMA_VERSION_STR);
@@ -411,14 +395,12 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap,
DM_IMA_VERSION_STR, context->dev_name,
context->dev_uuid);
}
-
- memcpy(device_table_data + l, capacity_str, capacity_len);
- l += capacity_len;
+ l += scnprintf(device_table_data + l, DM_IMA_DEVICE_BUF_LEN - l,
+ "current_device_capacity=%llu;", dm_ima_capacity(md));
dm_ima_measure_data("dm_device_resume", device_table_data, l, noio);
error:
- kfree(capacity_str);
kfree(device_table_data);
smp_mb__before_atomic();
@@ -433,21 +415,18 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all,
struct dm_ima_context *context,
unsigned int idx)
{
- char *device_table_data, *capacity_str = NULL;
+ char *device_table_data;
char active_table_str[] = "active_table_hash=";
char inactive_table_str[] = "inactive_table_hash=";
char device_active_str[] = "device_active_metadata=";
char device_inactive_str[] = "device_inactive_metadata=";
- char remove_all_str[] = "remove_all=";
unsigned int active_table_len = strlen(active_table_str);
unsigned int inactive_table_len = strlen(inactive_table_str);
unsigned int device_active_len = strlen(device_active_str);
unsigned int device_inactive_len = strlen(device_inactive_str);
- unsigned int remove_all_len = strlen(remove_all_str);
unsigned int l = 0;
bool noio = true;
bool nodata = true;
- int capacity_len;
wait_event(md->ima.ima_wq,
((unsigned int)(atomic_read(&md->ima.measure_idx)) == idx));
@@ -460,12 +439,6 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all,
if (!device_table_data)
goto exit;
- capacity_len = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
- if (capacity_len < 0) {
- kfree(device_table_data);
- goto exit;
- }
-
memcpy(device_table_data + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
l += strlen(DM_IMA_VERSION_STR);
@@ -531,18 +504,13 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all,
context->dev_uuid);
}
- memcpy(device_table_data + l, remove_all_str, remove_all_len);
- l += remove_all_len;
- memcpy(device_table_data + l, remove_all ? "y;" : "n;", 2);
- l += 2;
-
- memcpy(device_table_data + l, capacity_str, capacity_len);
- l += capacity_len;
+ l += scnprintf(device_table_data + l, (DM_IMA_DEVICE_BUF_LEN * 2) - l,
+ "remove_all=%c;current_device_capacity=%llu;",
+ remove_all ? 'y' : 'n', dm_ima_capacity(md));
dm_ima_measure_data("dm_device_remove", device_table_data, l, noio);
kfree(device_table_data);
- kfree(capacity_str);
exit:
kfree(md->ima.active_table.device_metadata);
kfree(md->ima.inactive_table.device_metadata);
@@ -565,12 +533,11 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md,
struct dm_ima_context *context)
{
unsigned int l = 0;
- char *device_table_data = NULL, *capacity_str = NULL;
+ char *device_table_data = NULL;
char inactive_str[] = "inactive_table_hash=";
unsigned int inactive_len = strlen(inactive_str);
bool noio = true;
bool nodata = true;
- int capacity_len;
if (unlikely(!context))
return;
@@ -584,10 +551,6 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md,
if (!device_table_data)
goto error;
- capacity_len = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
- if (capacity_len < 0)
- goto error;
-
memcpy(device_table_data + l, DM_IMA_VERSION_STR, strlen(DM_IMA_VERSION_STR));
l += strlen(DM_IMA_VERSION_STR);
@@ -619,8 +582,8 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md,
context->dev_uuid);
}
- memcpy(device_table_data + l, capacity_str, capacity_len);
- l += capacity_len;
+ l += scnprintf(device_table_data + l, DM_IMA_DEVICE_BUF_LEN - l,
+ "current_device_capacity=%llu;", dm_ima_capacity(md));
dm_ima_measure_data("dm_table_clear", device_table_data, l, noio);
@@ -629,7 +592,6 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md,
kfree(md->ima.inactive_table.device_metadata);
memset(&md->ima.inactive_table, 0, sizeof(md->ima.inactive_table));
- kfree(capacity_str);
kfree(device_table_data);
smp_mb__before_atomic();
@@ -644,7 +606,7 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md,
struct dm_ima_context *context)
{
char *old_device_data = NULL;
- char *combined_device_data = NULL, *capacity_str = NULL;
+ char *combined_device_data = NULL;
bool noio = true;
int len;
struct dm_ima_device_table_metadata *table;
@@ -663,9 +625,6 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md,
if (!combined_device_data)
goto exit;
- if (dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio) < 0)
- goto exit;
-
if (md->ima.active_table.device_metadata)
old_device_data = md->ima.active_table.device_metadata;
else if (md->ima.inactive_table.device_metadata)
@@ -673,14 +632,14 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md,
else
old_device_data = "table_rename=no_data;";
len = scnprintf(combined_device_data, DM_IMA_DEVICE_BUF_LEN * 2,
- "%s%snew_name=%s,new_uuid=%s;%s", DM_IMA_VERSION_STR, old_device_data,
- context->dev_name, context->dev_uuid, capacity_str);
+ "%s%snew_name=%s,new_uuid=%s;current_device_capacity=%llu;",
+ DM_IMA_VERSION_STR, old_device_data, context->dev_name,
+ context->dev_uuid, dm_ima_capacity(md));
dm_ima_measure_data("dm_device_rename", combined_device_data, len, noio);
-exit:
- kfree(capacity_str);
kfree(combined_device_data);
+exit:
if (md->ima.active_table.device_metadata) {
table = &md->ima.active_table;
dm_ima_copy_device_data(md, table->device_metadata, context,
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index 2f2ac69042f2..d8b2438f1571 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -41,6 +41,7 @@ struct dm_ima_device_table_metadata {
char *device_metadata;
unsigned int device_metadata_len;
unsigned int num_targets;
+ sector_t capacity;
/*
* Contains the sha256 hashes of the IMA measurements of the target
--
2.53.0
^ permalink raw reply related
* Re: [RFC PATCH 00/10] Fix dm-ima bugs
From: Mike Snitzer @ 2026-04-14 17:12 UTC (permalink / raw)
To: Benjamin Marzinski
Cc: Mikulas Patocka, dm-devel, linux-integrity, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin
In-Reply-To: <20260414002244.1917447-1-bmarzins@redhat.com>
On Mon, Apr 13, 2026 at 08:22:34PM -0400, Benjamin Marzinski wrote:
> The dm-ima code does not guarantee that the dm_ima_measure_on_*
> functions will not be called at the same time. Since they modify and
> free shared memory, this can lead to Use-After-Free errors or garbage
> measurements. Further, they don't make sure that the state they measure
> corresponds to the actual device state. For instance if table_load()
> runs at the same time as do_resume() on a table swap,
> dm_ima_measure_on_device_resume() can end up thinking the wrong table is
> active. Or a concurrent dm_hash_rename() and a table swap, can end up
> with a the new active table still using the old name. This patchset
> makes sure the the dm-ima function are serialized and report the correct
> device state.
>
> However, the code is still messier that in could be. This is because
> it duplicates the current measurement events and format. I would really
> like to know if that is necessary. Specifically, it currently measures
> the following dm device and table actions:
>
> load
> clear
> rename
> resume
> remove
>
> I don't see the benefit of reporting changes to the inactive table, or
> resumes where the device does not change state. From the user's point of
> view, the device is still the same after these events. At the same
> time, it doesn't measure device creates if no table was loaded, so you
> can have situations where the the first measurement for a device is a
> rename or a remove. A more sensible set of actions to measure would be:
>
> create
> table_swap
> rename
> remove
>
> Also, the measurement format doesn't map well to how dm device's are
> actually set up, in a way that makes it harder for the code and records
> extraneous information. First, like I mentioned before, I don't see the
> benefit of measuring the inactive table. Second, the name, uuid, and
> major/minor numbers are properties of the device, not it's table (and dm
> devices can't have partitions, so the minor count will always be 1). I
> don't see a reason to store and occasinally log this information twice,
> if there is an active and incative table, and it forces extra
> coordination between the dm_ima_measure_on_* functions.
>
> I'm wondering it we are stuck with the current events and format, now
> that this has been released? Or could we bump the version, and change
> what events we measure, and how we format the output?
>
> Benjamin Marzinski (10):
> dm-ima: remove dm_ima_reset_data()
> dm-ima: remove broken last_target_measured logic
> dm-ima: Remove status_flags from dm_ima_measure_on_table_load()
> dm-ima: don't copy the active table to the inactive table
> dm-ima: Fix UAF errors and measuring incorrect context
> dm-ima: remove new_map from dm_ima_measure_on_device_clear
> dm-ima: Fix issues with dm_ima_measure_on_device_rename
> dm-ima: Handle race between rename and table swap
> dm-ima: Fail more gracefully in dm_ima_measure_on_*
> dm-ima: use active table's size if available
>
> drivers/md/dm-ima.c | 506 +++++++++++++++++++-----------------------
> drivers/md/dm-ima.h | 67 ++++--
> drivers/md/dm-ioctl.c | 146 +++++++++++-
> drivers/md/dm.c | 2 +-
> 4 files changed, 421 insertions(+), 300 deletions(-)
Pretty extensive changes needed here all things considered.
SO I'm aware, who is using dm-ima? I see that Tushar Sugandhi is no
longer at Microsoft and so he isn't cc'd on these changes. I can
infer from Cc some potential users, but I just want to make sure this
code isn't just technical debt that we're having to carry in DM now?
Thanks,
Mike
^ permalink raw reply
* Re: [RFC PATCH 00/10] Fix dm-ima bugs
From: Benjamin Marzinski @ 2026-04-14 18:35 UTC (permalink / raw)
To: Mike Snitzer
Cc: Mikulas Patocka, dm-devel, linux-integrity, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin
In-Reply-To: <ad51kuxJuU84Amep@kernel.org>
On Tue, Apr 14, 2026 at 01:12:50PM -0400, Mike Snitzer wrote:
> On Mon, Apr 13, 2026 at 08:22:34PM -0400, Benjamin Marzinski wrote:
> > The dm-ima code does not guarantee that the dm_ima_measure_on_*
> > functions will not be called at the same time. Since they modify and
> > free shared memory, this can lead to Use-After-Free errors or garbage
> > measurements. Further, they don't make sure that the state they measure
> > corresponds to the actual device state. For instance if table_load()
> > runs at the same time as do_resume() on a table swap,
> > dm_ima_measure_on_device_resume() can end up thinking the wrong table is
> > active. Or a concurrent dm_hash_rename() and a table swap, can end up
> > with a the new active table still using the old name. This patchset
> > makes sure the the dm-ima function are serialized and report the correct
> > device state.
> >
> > However, the code is still messier that in could be. This is because
> > it duplicates the current measurement events and format. I would really
> > like to know if that is necessary. Specifically, it currently measures
> > the following dm device and table actions:
> >
> > load
> > clear
> > rename
> > resume
> > remove
> >
> > I don't see the benefit of reporting changes to the inactive table, or
> > resumes where the device does not change state. From the user's point of
> > view, the device is still the same after these events. At the same
> > time, it doesn't measure device creates if no table was loaded, so you
> > can have situations where the the first measurement for a device is a
> > rename or a remove. A more sensible set of actions to measure would be:
> >
> > create
> > table_swap
> > rename
> > remove
> >
> > Also, the measurement format doesn't map well to how dm device's are
> > actually set up, in a way that makes it harder for the code and records
> > extraneous information. First, like I mentioned before, I don't see the
> > benefit of measuring the inactive table. Second, the name, uuid, and
> > major/minor numbers are properties of the device, not it's table (and dm
> > devices can't have partitions, so the minor count will always be 1). I
> > don't see a reason to store and occasinally log this information twice,
> > if there is an active and incative table, and it forces extra
> > coordination between the dm_ima_measure_on_* functions.
> >
> > I'm wondering it we are stuck with the current events and format, now
> > that this has been released? Or could we bump the version, and change
> > what events we measure, and how we format the output?
> >
> > Benjamin Marzinski (10):
> > dm-ima: remove dm_ima_reset_data()
> > dm-ima: remove broken last_target_measured logic
> > dm-ima: Remove status_flags from dm_ima_measure_on_table_load()
> > dm-ima: don't copy the active table to the inactive table
> > dm-ima: Fix UAF errors and measuring incorrect context
> > dm-ima: remove new_map from dm_ima_measure_on_device_clear
> > dm-ima: Fix issues with dm_ima_measure_on_device_rename
> > dm-ima: Handle race between rename and table swap
> > dm-ima: Fail more gracefully in dm_ima_measure_on_*
> > dm-ima: use active table's size if available
> >
> > drivers/md/dm-ima.c | 506 +++++++++++++++++++-----------------------
> > drivers/md/dm-ima.h | 67 ++++--
> > drivers/md/dm-ioctl.c | 146 +++++++++++-
> > drivers/md/dm.c | 2 +-
> > 4 files changed, 421 insertions(+), 300 deletions(-)
>
> Pretty extensive changes needed here all things considered.
>
> SO I'm aware, who is using dm-ima? I see that Tushar Sugandhi is no
> longer at Microsoft and so he isn't cc'd on these changes. I can
> infer from Cc some potential users, but I just want to make sure this
> code isn't just technical debt that we're having to carry in DM now?
I don't actually know if anyone is. The issue is that the Use After Free
crashes can happen as long as CONFIG_IMA is set, regardless of whether
or not you have any IMA policy set.
If we don't need to care, we could just serialize the
dm_ima_measure_on_* functions to keep them from having concurrent access
to shared memory, and let them keep reporting bogus data if there are
races (or simply unfortunate orderings. load then rename then resume
currently forgets the rename, even without a race) That's a much smaller
change, and if they're currently good enough for any users, it won't
make them any worse.
-Ben
>
> Thanks,
> Mike
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox