* Re: [PATCH -next] ima: Handle error code returned by ima_filter_rule_match()
From: Roberto Sassu @ 2025-11-20 9:17 UTC (permalink / raw)
To: Zhao Yipeng, zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg
Cc: lujialin4, linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <20251120071805.1604864-1-zhaoyipeng5@huawei.com>
On Thu, 2025-11-20 at 15:18 +0800, Zhao Yipeng wrote:
> In ima_match_rules(), if ima_filter_rule_match() returns -ENOENT due to
> the rule being NULL, the function incorrectly skips the 'if (!rc)' check
> and sets 'result = true'. The LSM rule is considered a match, causing
> extra files to be measured by IMA.
>
> This issue can be reproduced in the following scenario:
> After unloading the SELinux policy module via 'semodule -d', if an IMA
> measurement is triggered before ima_lsm_rules is updated,
> in ima_match_rules(), the first call to ima_filter_rule_match() returns
> -ESTALE. This causes the code to enter the 'if (rc == -ESTALE &&
> !rule_reinitialized)' block, perform ima_lsm_copy_rule() and retry. In
> ima_lsm_copy_rule(), since the SELinux module has been removed, the rule
> becomes NULL, and the second call to ima_filter_rule_match() returns
> -ENOENT. This bypasses the 'if (!rc)' check and results in a false match.
>
> Call trace:
> selinux_audit_rule_match+0x310/0x3b8
> security_audit_rule_match+0x60/0xa0
> ima_match_rules+0x2e4/0x4a0
> ima_match_policy+0x9c/0x1e8
> ima_get_action+0x48/0x60
> process_measurement+0xf8/0xa98
> ima_bprm_check+0x98/0xd8
> security_bprm_check+0x5c/0x78
> search_binary_handler+0x6c/0x318
> exec_binprm+0x58/0x1b8
> bprm_execve+0xb8/0x130
> do_execveat_common.isra.0+0x1a8/0x258
> __arm64_sys_execve+0x48/0x68
> invoke_syscall+0x50/0x128
> el0_svc_common.constprop.0+0xc8/0xf0
> do_el0_svc+0x24/0x38
> el0_svc+0x44/0x200
> el0t_64_sync_handler+0x100/0x130
> el0t_64_sync+0x3c8/0x3d0
>
> Fix this by changing 'if (!rc)' to 'if (rc <= 0)' to ensure that error
> codes like -ENOENT do not bypass the check and accidentally result in a
> successful match.
Thanks, it makes sense.
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Roberto
> Fixes: 4af4662fa4a9d ("integrity: IMA policy")
> Signed-off-by: Zhao Yipeng <zhaoyipeng5@huawei.com>
> ---
> security/integrity/ima/ima_policy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 128fab897930..db6d55af5a80 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -674,7 +674,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
> goto retry;
> }
> }
> - if (!rc) {
> + if (rc <= 0) {
> result = false;
> goto out;
> }
^ permalink raw reply
* [PATCH -next] ima: Handle error code returned by ima_filter_rule_match()
From: Zhao Yipeng @ 2025-11-20 7:18 UTC (permalink / raw)
To: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg
Cc: lujialin4, linux-integrity, linux-security-module, linux-kernel
In ima_match_rules(), if ima_filter_rule_match() returns -ENOENT due to
the rule being NULL, the function incorrectly skips the 'if (!rc)' check
and sets 'result = true'. The LSM rule is considered a match, causing
extra files to be measured by IMA.
This issue can be reproduced in the following scenario:
After unloading the SELinux policy module via 'semodule -d', if an IMA
measurement is triggered before ima_lsm_rules is updated,
in ima_match_rules(), the first call to ima_filter_rule_match() returns
-ESTALE. This causes the code to enter the 'if (rc == -ESTALE &&
!rule_reinitialized)' block, perform ima_lsm_copy_rule() and retry. In
ima_lsm_copy_rule(), since the SELinux module has been removed, the rule
becomes NULL, and the second call to ima_filter_rule_match() returns
-ENOENT. This bypasses the 'if (!rc)' check and results in a false match.
Call trace:
selinux_audit_rule_match+0x310/0x3b8
security_audit_rule_match+0x60/0xa0
ima_match_rules+0x2e4/0x4a0
ima_match_policy+0x9c/0x1e8
ima_get_action+0x48/0x60
process_measurement+0xf8/0xa98
ima_bprm_check+0x98/0xd8
security_bprm_check+0x5c/0x78
search_binary_handler+0x6c/0x318
exec_binprm+0x58/0x1b8
bprm_execve+0xb8/0x130
do_execveat_common.isra.0+0x1a8/0x258
__arm64_sys_execve+0x48/0x68
invoke_syscall+0x50/0x128
el0_svc_common.constprop.0+0xc8/0xf0
do_el0_svc+0x24/0x38
el0_svc+0x44/0x200
el0t_64_sync_handler+0x100/0x130
el0t_64_sync+0x3c8/0x3d0
Fix this by changing 'if (!rc)' to 'if (rc <= 0)' to ensure that error
codes like -ENOENT do not bypass the check and accidentally result in a
successful match.
Fixes: 4af4662fa4a9d ("integrity: IMA policy")
Signed-off-by: Zhao Yipeng <zhaoyipeng5@huawei.com>
---
security/integrity/ima/ima_policy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 128fab897930..db6d55af5a80 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -674,7 +674,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
goto retry;
}
}
- if (!rc) {
+ if (rc <= 0) {
result = false;
goto out;
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits
From: Jakub Kicinski @ 2025-11-20 1:47 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, Alan Stern, Alexander Viro, Alexei Starovoitov,
Andi Shyti, Andreas Dilger, Andrew Lunn, Andrew Morton,
Andrii Nakryiko, Andy Shevchenko, Ard Biesheuvel,
Arnaldo Carvalho de Melo, Bjorn Helgaas, Borislav Petkov,
Christian Brauner, Christian König, Christoph Hellwig,
Daniel Borkmann, Dan Williams, Dave Hansen, Dave Jiang,
David Ahern, David Hildenbrand, Davidlohr Bueso, David S. Miller,
Dennis Zhou, Eric Dumazet, Greg Kroah-Hartman, Herbert Xu,
Ingo Molnar, Jakub Sitnicki, James E.J. Bottomley,
Jarkko Sakkinen, Jason A. Donenfeld, Jens Axboe, Jiri Slaby,
Johannes Weiner, John Allen, Jonathan Cameron, Juergen Gross,
Kees Cook, KP Singh, Linus Walleij, Martin K. Petersen,
Matthew Wilcox (Oracle), Mika Westerberg, Mike Rapoport,
Miklos Szeredi, Namhyung Kim, Neal Cardwell, nic_swsd,
OGAWA Hirofumi, Olivia Mackall, Paolo Abeni, Paolo Bonzini,
Peter Huewe, Peter Zijlstra, Rafael J. Wysocki,
Sean Christopherson, Srinivas Kandagatla, Stefano Stabellini,
Steven Rostedt, Tejun Heo, Theodore Ts'o, Thomas Gleixner,
Tom Lendacky, Willem de Bruijn, x86, Yury Norov, amd-gfx, bpf,
cgroups, dri-devel, io-uring, kvm, linux-acpi, linux-block,
linux-crypto, linux-cxl, linux-efi, linux-ext4, linux-fsdevel,
linux-gpio, linux-i2c, linux-integrity, linux-mm, linux-nvme,
linux-pci, linux-perf-users, linux-scsi, linux-serial,
linux-trace-kernel, linux-usb, mptcp, netdev, usb-storage
In-Reply-To: <20251119224140.8616-1-david.laight.linux@gmail.com>
On Wed, 19 Nov 2025 22:40:56 +0000 david.laight.linux@gmail.com wrote:
> I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
> from 124 to under 100 to be able to send the cover letter.
> The individual patches only go to the addresses found for the associated files.
> That reduces the number of emails to a less unsane number.
Please split the networking (9?) patches out to a separate series.
It will help you with the CC list, and help us to get this applied..
^ permalink raw reply
* [PATCH 00/44] Change a lot of min_t() that might mask high bits
From: david.laight.linux @ 2025-11-19 22:40 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Alexander Viro, Alexei Starovoitov, Andi Shyti,
Andreas Dilger, Andrew Lunn, Andrew Morton, Andrii Nakryiko,
Andy Shevchenko, Ard Biesheuvel, Arnaldo Carvalho de Melo,
Bjorn Helgaas, Borislav Petkov, Christian Brauner,
Christian König, Christoph Hellwig, Daniel Borkmann,
Dan Williams, Dave Hansen, Dave Jiang, David Ahern,
David Hildenbrand, Davidlohr Bueso, David S. Miller, Dennis Zhou,
Eric Dumazet, Greg Kroah-Hartman, Herbert Xu, Ingo Molnar,
Jakub Kicinski, Jakub Sitnicki, James E.J. Bottomley,
Jarkko Sakkinen, Jason A. Donenfeld, Jens Axboe, Jiri Slaby,
Johannes Weiner, John Allen, Jonathan Cameron, Juergen Gross,
Kees Cook, KP Singh, Linus Walleij, Martin K. Petersen,
Matthew Wilcox (Oracle), Mika Westerberg, Mike Rapoport,
Miklos Szeredi, Namhyung Kim, Neal Cardwell, nic_swsd,
OGAWA Hirofumi, Olivia Mackall, Paolo Abeni, Paolo Bonzini,
Peter Huewe, Peter Zijlstra, Rafael J. Wysocki,
Sean Christopherson, Srinivas Kandagatla, Stefano Stabellini,
Steven Rostedt, Tejun Heo, Theodore Ts'o, Thomas Gleixner,
Tom Lendacky, Willem de Bruijn, x86, Yury Norov, amd-gfx, bpf,
cgroups, dri-devel, io-uring, kvm, linux-acpi, linux-block,
linux-crypto, linux-cxl, linux-efi, linux-ext4, linux-fsdevel,
linux-gpio, linux-i2c, linux-integrity, linux-mm, linux-nvme,
linux-pci, linux-perf-users, linux-scsi, linux-serial,
linux-trace-kernel, linux-usb, mptcp, netdev, usb-storage,
David Laight
From: David Laight <david.laight.linux@gmail.com>
It in not uncommon for code to use min_t(uint, a, b) when one of a or b
is 64bit and can have a value that is larger than 2^32;
This is particularly prevelant with:
uint_var = min_t(uint, uint_var, uint64_expression);
Casts to u8 and u16 are very likely to discard significant bits.
These can be detected at compile time by changing min_t(), for example:
#define CHECK_SIZE(fn, type, val) \
BUILD_BUG_ON_MSG(sizeof (val) > sizeof (type) && \
!statically_true(((val) >> 8 * (sizeof (type) - 1)) < 256), \
fn "() significant bits of '" #val "' may be discarded")
#define min_t(type, x, y) ({ \
CHECK_SIZE("min_t", type, x); \
CHECK_SIZE("min_t", type, y); \
__cmp_once(min, type, x, y); })
(and similar changes to max_t() and clamp_t().)
This shows up some real bugs, some unlikely bugs and some false positives.
In most cases both arguments are unsigned type (just different ones)
and min_t() can just be replaced by min().
The patches are all independant and are most of the ones needed to
get the x86-64 kernel I build to compile.
I've not tried building an allyesconfig or allmodconfig kernel.
I've also not included the patch to minmax.h itself.
I've tried to put the patches that actually fix things first.
The last one is 0009.
I gave up on fixing sched/fair.c - it is too broken for a single patch!
The patch for net/ipv4/tcp.c is also absent because do_tcp_getsockopt()
needs multiple/larger changes to make it 'sane'.
I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
from 124 to under 100 to be able to send the cover letter.
The individual patches only go to the addresses found for the associated files.
That reduces the number of emails to a less unsane number.
David Laight (44):
x86/asm/bitops: Change the return type of variable__ffs() to unsigned
int
ext4: Fix saturation of 64bit inode times for old filesystems
perf: Fix branch stack callchain limit
io_uring/net: Change some dubious min_t()
ipc/msg: Fix saturation of percpu counts in msgctl_info()
bpf: Verifier, remove some unusual uses of min_t() and max_t()
net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value.
net: ethtool: Use min3() instead of nested min_t(u16,...)
ipv6: __ip6_append_data() don't abuse max_t() casts
x86/crypto: ctr_crypt() use min() instead of min_t()
arch/x96/kvm: use min() instead of min_t()
block: use min() instead of min_t()
drivers/acpi: use min() instead of min_t()
drivers/char/hw_random: use min3() instead of nested min_t()
drivers/char/tpm: use min() instead of min_t()
drivers/crypto/ccp: use min() instead of min_t()
drivers/cxl: use min() instead of min_t()
drivers/gpio: use min() instead of min_t()
drivers/gpu/drm/amd: use min() instead of min_t()
drivers/i2c/busses: use min() instead of min_t()
drivers/net/ethernet/realtek: use min() instead of min_t()
drivers/nvme: use min() instead of min_t()
arch/x86/mm: use min() instead of min_t()
drivers/nvmem: use min() instead of min_t()
drivers/pci: use min() instead of min_t()
drivers/scsi: use min() instead of min_t()
drivers/tty/vt: use umin() instead of min_t(u16, ...) for row/col
limits
drivers/usb/storage: use min() instead of min_t()
drivers/xen: use min() instead of min_t()
fs: use min() or umin() instead of min_t()
block: bvec.h: use min() instead of min_t()
nodemask: use min() instead of min_t()
ipc: use min() instead of min_t()
bpf: use min() instead of min_t()
bpf_trace: use min() instead of min_t()
lib/bucket_locks: use min() instead of min_t()
lib/crypto/mpi: use min() instead of min_t()
lib/dynamic_queue_limits: use max() instead of max_t()
mm: use min() instead of min_t()
net: Don't pass bitfields to max_t()
net/core: Change loop conditions so min() can be used
net: use min() instead of min_t()
net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits
net/mptcp: Change some dubious min_t(int, ...) to min()
arch/x86/crypto/aesni-intel_glue.c | 3 +-
arch/x86/include/asm/bitops.h | 18 +++++-------
arch/x86/kvm/emulate.c | 3 +-
arch/x86/kvm/lapic.c | 2 +-
arch/x86/kvm/mmu/mmu.c | 2 +-
arch/x86/mm/pat/set_memory.c | 12 ++++----
block/blk-iocost.c | 6 ++--
block/blk-settings.c | 2 +-
block/partitions/efi.c | 3 +-
drivers/acpi/property.c | 2 +-
drivers/char/hw_random/core.c | 2 +-
drivers/char/tpm/tpm1-cmd.c | 2 +-
drivers/char/tpm/tpm_tis_core.c | 4 +--
drivers/crypto/ccp/ccp-dev.c | 2 +-
drivers/cxl/core/mbox.c | 2 +-
drivers/gpio/gpiolib-acpi-core.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c | 4 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
drivers/i2c/busses/i2c-designware-master.c | 2 +-
drivers/net/ethernet/realtek/r8169_main.c | 3 +-
drivers/nvme/host/pci.c | 3 +-
drivers/nvme/host/zns.c | 3 +-
drivers/nvmem/core.c | 2 +-
drivers/pci/probe.c | 3 +-
drivers/scsi/hosts.c | 2 +-
drivers/tty/vt/selection.c | 9 +++---
drivers/usb/storage/protocol.c | 3 +-
drivers/xen/grant-table.c | 2 +-
fs/buffer.c | 2 +-
fs/exec.c | 2 +-
fs/ext4/ext4.h | 2 +-
fs/ext4/mballoc.c | 3 +-
fs/ext4/resize.c | 2 +-
fs/ext4/super.c | 2 +-
fs/fat/dir.c | 4 +--
fs/fat/file.c | 3 +-
fs/fuse/dev.c | 2 +-
fs/fuse/file.c | 8 ++---
fs/splice.c | 2 +-
include/linux/bvec.h | 3 +-
include/linux/nodemask.h | 9 +++---
include/linux/perf_event.h | 2 +-
include/net/tcp_ecn.h | 5 ++--
io_uring/net.c | 6 ++--
ipc/mqueue.c | 4 +--
ipc/msg.c | 6 ++--
kernel/bpf/core.c | 4 +--
kernel/bpf/log.c | 2 +-
kernel/bpf/verifier.c | 29 +++++++------------
kernel/trace/bpf_trace.c | 2 +-
lib/bucket_locks.c | 2 +-
lib/crypto/mpi/mpicoder.c | 2 +-
lib/dynamic_queue_limits.c | 2 +-
mm/gup.c | 4 +--
mm/memblock.c | 2 +-
mm/memory.c | 2 +-
mm/percpu.c | 2 +-
mm/truncate.c | 3 +-
mm/vmscan.c | 2 +-
net/core/datagram.c | 6 ++--
net/core/flow_dissector.c | 7 ++---
net/core/net-sysfs.c | 3 +-
net/core/skmsg.c | 4 +--
net/ethtool/cmis_cdb.c | 7 ++---
net/ipv4/fib_trie.c | 2 +-
net/ipv4/tcp_input.c | 4 +--
net/ipv4/tcp_output.c | 5 ++--
net/ipv4/tcp_timer.c | 4 +--
net/ipv6/addrconf.c | 8 ++---
net/ipv6/ip6_output.c | 7 +++--
net/ipv6/ndisc.c | 5 ++--
net/mptcp/protocol.c | 8 ++---
net/netlink/genetlink.c | 9 +++---
net/packet/af_packet.c | 2 +-
net/unix/af_unix.c | 4 +--
76 files changed, 141 insertions(+), 176 deletions(-)
--
2.39.5
^ permalink raw reply
* [PATCH 15/44] drivers/char/tpm: use min() instead of min_t()
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, linux-integrity; +Cc: Jarkko Sakkinen, Peter Huewe, David Laight
In-Reply-To: <20251119224140.8616-1-david.laight.linux@gmail.com>
From: David Laight <david.laight.linux@gmail.com>
min_t(int, a, b) casts an 'long' to 'int'.
Use min(a, b) instead as it promotes any 'int' to 'long'
and so cannot discard significant bits.
In this case the 'long' value is small enough that the result is ok.
Detected by an extra check added to min_t().
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
drivers/char/tpm/tpm1-cmd.c | 2 +-
drivers/char/tpm/tpm_tis_core.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index cf64c7385105..11088bda4e68 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -530,7 +530,7 @@ struct tpm1_get_random_out {
int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
{
struct tpm1_get_random_out *out;
- u32 num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
+ u32 num_bytes = min(max, TPM_MAX_RNG_DATA);
struct tpm_buf buf;
u32 total = 0;
int retries = 5;
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 8954a8660ffc..2676e3a241b5 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -310,7 +310,7 @@ static int get_burstcount(struct tpm_chip *chip)
return -EBUSY;
}
-static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
+static int recv_data(struct tpm_chip *chip, u8 *buf, u32 count)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
int size = 0, burstcnt, rc;
@@ -453,7 +453,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
rc = burstcnt;
goto out_err;
}
- burstcnt = min_t(int, burstcnt, len - count - 1);
+ burstcnt = min(burstcnt, len - count - 1);
rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
burstcnt, buf + count);
if (rc < 0)
--
2.39.5
^ permalink raw reply related
* [RFC v1 1/1] ima: Implement IMA event log trimming
From: Anirudh Venkataramanan @ 2025-11-19 21:33 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E . Hallyn, linux-security-module,
Anirudh Venkataramanan, Steven Chen, Gregory Lumen,
Lakshmi Ramasubramanian, Sush Shringarputale
In-Reply-To: <20251119213359.39397-1-anirudhve@linux.microsoft.com>
From: Steven Chen <chenste@linux.microsoft.com>
Implement a PCR value based method for IMA event log trimming by
exposing a new pseudo file /sys/kernel/config/ima/pcrs. This
pseudo file allows userspace to:
- read IMA starting PCR values.
- write trim-to PCR values to trigger IMA log trimming.
If a trimming operation is successful, one or more entries will be
purged from the IMA measurements list in the kernel.
Signed-off-by: Steven Chen <chenste@linux.microsoft.com>
Signed-off-by: Anirudh Venkataramanan <anirudhve@linux.microsoft.com>
---
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/ima/Kconfig | 13 +
drivers/ima/Makefile | 2 +
drivers/ima/ima_config_pcrs.c | 291 ++++++++++++++++++
include/linux/ima.h | 27 ++
security/integrity/ima/Makefile | 4 +
security/integrity/ima/ima.h | 8 +
security/integrity/ima/ima_init.c | 44 +++
security/integrity/ima/ima_log_trim.c | 421 ++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 7 +-
security/integrity/ima/ima_queue.c | 5 +-
12 files changed, 821 insertions(+), 4 deletions(-)
create mode 100644 drivers/ima/Kconfig
create mode 100644 drivers/ima/Makefile
create mode 100644 drivers/ima/ima_config_pcrs.c
create mode 100644 security/integrity/ima/ima_log_trim.c
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 4915a63866b0..35b83be86c42 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -251,4 +251,6 @@ source "drivers/hte/Kconfig"
source "drivers/cdx/Kconfig"
+source "drivers/ima/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 8e1ffa4358d5..3aad6096d416 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -197,3 +197,4 @@ obj-$(CONFIG_DPLL) += dpll/
obj-$(CONFIG_DIBS) += dibs/
obj-$(CONFIG_S390) += s390/
+obj-$(CONFIG_IMA_PCRS) += ima/
diff --git a/drivers/ima/Kconfig b/drivers/ima/Kconfig
new file mode 100644
index 000000000000..9e465fbe3adb
--- /dev/null
+++ b/drivers/ima/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config IMA_PCRS
+ bool "IMA Event Log Trimming"
+ default n
+ help
+ Say Y here if you want support for IMA Event Log Trimming.
+
+ This creates the configfs file /sys/kernel/config/ima/pcrs.
+ Userspace
+ - writes to this file to trigger IMA event log trimming
+ - reads this file to get IMA starting PCR values
+
+ If unsure, say N.
diff --git a/drivers/ima/Makefile b/drivers/ima/Makefile
new file mode 100644
index 000000000000..ac9b9b96b5cb
--- /dev/null
+++ b/drivers/ima/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_IMA_PCRS) += ima_config_pcrs.o
diff --git a/drivers/ima/ima_config_pcrs.c b/drivers/ima/ima_config_pcrs.c
new file mode 100644
index 000000000000..f329665f2b90
--- /dev/null
+++ b/drivers/ima/ima_config_pcrs.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Microsoft Corporation
+ *
+ * Authors:
+ * Steven Chen <chenste@linux.microsoft.com>
+ *
+ * File: ima_config_pcrs.c
+ *
+ * Implements IMA interface to allow userspace to read IMA starting PCR
+ * values and trigger IMA event log trimming.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/configfs.h>
+#include <linux/printk.h>
+#include <linux/tpm.h>
+#include <linux/ima.h>
+/*
+ * The IMA PCR configfs is a childless subsystem. It cannot create
+ * any config_items. It just has attribute pcrs for read and write.
+ */
+
+#define MAX_HASH_ALGO_NAME_LENGTH 16 /*sha1, sha256, ...*/
+#define PCR_NAME_LEN 6 /*pcrxx:*/
+#define MIN_PCR_RECORD_LENGTH 20
+#define OFFSET_IDX_1 3
+#define OFFSET_IDX_2 4
+#define OFFSET_COLON 5
+
+/*
+ * mutex protects atomicity of trimming measurement list
+ * and updating ima_start_point_pcr_values
+ * protects concurrent writes to the IMA PCR values
+ * This also not allow memory allocation while waiting the mutex
+ */
+static DEFINE_MUTEX(ima_pcr_write_mutex);
+
+/* show current IMA Start Point PCR values in ascii format */
+static ssize_t ima_config_pcrs_ascii_show(struct config_item *item, char *page)
+{
+ int len = 0;
+
+ for (int i = 0; i < num_tpm_banks; i++) {
+ for (int j = 0; j < num_pcr_configured; j++) {
+ int algorithm_id = ima_start_point_pcr_values[i].alg_id;
+
+ /*
+ * Write "pcrXX:AAA:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+ * where XX is the PCR index (00-23),
+ * where AAA is the algorithm name, such as sha1, sha256, sha384..
+ * and x...x is the digest data in hex format
+ */
+ len += sprintf(page + len, "pcr%02d:", digests_pcr_map[j]);
+ len += sprintf(page + len, "%s:", hash_algo_name[algorithm_id]);
+ for (int k = 0; k < hash_digest_size[algorithm_id]; k++)
+ len += sprintf(page + len, "%02x",
+ ima_start_point_pcr_values[i].digests[j][k]);
+ len += sprintf(page + len, "\n");
+ }
+ }
+ return len;
+}
+
+/* show current IMA Start Point PCR values */
+static ssize_t ima_config_pcrs_show(struct config_item *item, char *page)
+{
+ int len = 0;
+
+ for (int i = 0; i < num_tpm_banks; i++) {
+ int algorithm_id = ima_start_point_pcr_values[i].alg_id;
+
+ for (int j = 0; j < TPM2_PLATFORM_PCR; ++j) {
+ int digest_index = pcr_digests_map[j];
+
+ if (digest_index == -1)
+ continue;
+ /*
+ * Write "pcrXX:AAA:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+ * where XX is the PCR index (00-23),
+ * where AAA is the algorithm name, such as sha1, sha256, sha384..
+ * and x...x is the digest data in raw format
+ */
+ len += sprintf(page + len, "pcr%02d:", j);
+ len += sprintf(page + len, "%s:", hash_algo_name[algorithm_id]);
+
+ memcpy(page + len, ima_start_point_pcr_values[i].digests[digest_index],
+ hash_digest_size[algorithm_id]);
+
+ len += hash_digest_size[algorithm_id];
+ }
+ }
+
+ return len;
+}
+
+/* get PCR index from input string */
+static int get_pcr_idx(char *p, int offset)
+{
+ /* validate digits first */
+ if (!isdigit((unsigned char)p[offset + OFFSET_IDX_1]) ||
+ !isdigit((unsigned char)p[offset + OFFSET_IDX_2]))
+ return -EINVAL;
+
+ return (p[offset + OFFSET_IDX_1] - '0') * 10 + (p[offset + OFFSET_IDX_2] - '0');
+}
+
+/*
+ * Parse the input data
+ * Get new PCR values and trim IMA event log
+ */
+static ssize_t ima_config_pcrs_store(struct config_item *item, const char *page, size_t count)
+{
+ struct ima_pcr_value *ima_pcr_values;
+ unsigned long pcr_found = 0;
+ char *p = (char *)page;
+ int pcr_count;
+ int offset = 0;
+ int ret = -EINVAL;
+
+ if (count <= 0)
+ return ret;
+
+ if (num_pcr_configured <= 0)
+ return -ENOMEM;
+
+ /*
+ * Only one thread can write to the PCRs at a time
+ * Not allocate memory while waiting the mutex
+ */
+
+ mutex_lock(&ima_pcr_write_mutex);
+
+ pcr_count = num_pcr_configured;
+
+ ima_pcr_values = kcalloc(num_pcr_configured, sizeof(*ima_pcr_values), GFP_KERNEL);
+
+ if (!ima_pcr_values) {
+ ret = -ENOMEM;
+ goto out_notrim_unlock;
+ }
+
+ /*
+ * parse the input data
+ * each entry is like: pcrX:AAA:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * where XX is the PCR index (00-23),
+ * where AAA is the algorithm name, e.g. sha1, sha256, sha384..
+ * and x...x is the digest data in binary format
+ * multiple entries are concatenated together
+ */
+ while (offset < count) {
+ int index, mapped_index, algo_id, algo_name_len = 0;
+ char algo_name[MAX_HASH_ALGO_NAME_LENGTH];
+ u8 digest_len;
+
+ /* Need at least "pcrXX:AAA:" */
+ if (offset + MIN_PCR_RECORD_LENGTH > count)
+ goto out_nottrim;
+
+ if (memcmp(p + offset, "pcr", 3) != 0 || p[offset + OFFSET_COLON] != ':') {
+ pr_alert("invalid input format\n");
+ goto out_nottrim;
+ }
+
+ /* Get the PCR index*/
+ index = get_pcr_idx(p, offset);
+ if (index < 0 || index >= TPM2_PLATFORM_PCR) {
+ pr_alert("invalid PCR index: %d\n", index);
+ goto out_nottrim;
+ }
+
+ offset += PCR_NAME_LEN;
+
+ while (offset + algo_name_len < count && p[offset + algo_name_len] != ':')
+ ++algo_name_len;
+
+ /* Check actual algo name length */
+ if (algo_name_len == 0 || algo_name_len >= MAX_HASH_ALGO_NAME_LENGTH) {
+ pr_err("ima pcr configuration: invalid algorithm name length\n");
+ goto out_nottrim;
+ }
+
+ memcpy(algo_name, p + offset, algo_name_len);
+ algo_name[algo_name_len] = '\0';
+
+ algo_id = match_string(hash_algo_name, HASH_ALGO__LAST, algo_name);
+ /* validate algo_id */
+ if (algo_id < HASH_ALGO_SHA1 || algo_id >= HASH_ALGO__LAST) {
+ pr_err("ima pcr configuration: invalid algorithm ID\n");
+ goto out_nottrim;
+ }
+ digest_len = hash_digest_size[algo_id];
+ offset += algo_name_len + 1;
+ /* validate we have enough data for the digest */
+ if (offset + digest_len > count)
+ goto out_nottrim;
+
+ mapped_index = pcr_digests_map[index];
+
+ if (pcr_digests_map[index] != -1 && !test_bit(index, &pcr_found)) {
+ ima_pcr_values[mapped_index].algo_id = algo_id;
+ set_bit(index, &pcr_found);
+ memcpy(ima_pcr_values[mapped_index].digest, p + offset, digest_len);
+ --pcr_count;
+ } else {
+ pr_alert("invalid PCR index or duplicate PCR set index: %d\n", index);
+ goto out_nottrim;
+ }
+ offset += digest_len;
+ }
+
+ if (pcr_count != 0) {
+ /* not all PCRs are provided */
+ pr_alert("not all PCRs are provided\n");
+ goto out_nottrim;
+ }
+
+ /*
+ * all configured PCRs values are provided, now recalculate the IMA PCRs
+ * and check if they can match the these PCR values
+ * Trim the IMA event logs if the given PCR values match
+ */
+ ret = ima_trim_event_log((const void *)ima_pcr_values);
+ if (ret >= 0)
+ ret = count;
+
+out_nottrim:
+ kfree(ima_pcr_values);
+out_notrim_unlock:
+ mutex_unlock(&ima_pcr_write_mutex);
+ return ret;
+}
+
+CONFIGFS_ATTR(ima_config_, pcrs);
+CONFIGFS_ATTR_RO(ima_config_, pcrs_ascii);
+
+static struct configfs_attribute *ima_config_pcr_attrs[] = {
+ &ima_config_attr_pcrs,
+ &ima_config_attr_pcrs_ascii,
+ NULL,
+};
+
+static const struct config_item_type ima_config_pcr_type = {
+ .ct_attrs = ima_config_pcr_attrs,
+ .ct_owner = THIS_MODULE,
+};
+
+static struct configfs_subsystem config_ima_pcrs = {
+ .su_group = {
+ .cg_item = {
+ .ci_namebuf = "ima",
+ .ci_type = &ima_config_pcr_type,
+ },
+ },
+};
+
+static int __init configfs_ima_pcrs_init(void)
+{
+ struct tpm_chip *tpm_chip;
+ int ret;
+
+ tpm_chip = tpm_default_chip();
+ if (!tpm_chip) {
+ pr_err("No TPM chip found, IMA PCR configfs disabled\n");
+ return -ENODEV;
+ }
+
+ config_group_init(&config_ima_pcrs.su_group);
+ mutex_init(&config_ima_pcrs.su_mutex);
+ ret = configfs_register_subsystem(&config_ima_pcrs);
+ if (ret) {
+ pr_err("Error %d while registering subsystem %s\n",
+ ret, config_ima_pcrs.su_group.cg_item.ci_namebuf);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void __exit configfs_ima_pcrs_exit(void)
+{
+ configfs_unregister_subsystem(&config_ima_pcrs);
+}
+
+module_init(configfs_ima_pcrs_init);
+module_exit(configfs_ima_pcrs_exit);
+MODULE_DESCRIPTION("IMA PCRS configfs module");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 8e29cb4e6a01..f8a6209b9423 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -12,6 +12,8 @@
#include <linux/security.h>
#include <linux/kexec.h>
#include <crypto/hash_info.h>
+#include <linux/tpm.h>
+
struct linux_binprm;
#ifdef CONFIG_IMA
@@ -24,6 +26,31 @@ extern int ima_measure_critical_data(const char *event_label,
const void *buf, size_t buf_len,
bool hash, u8 *digest, size_t digest_len);
+#ifdef CONFIG_IMA_PCRS
+struct ima_pcr_value {
+ u8 digest[HASH_MAX_DIGESTSIZE]; /* PCR value */
+ u8 algo_id; /* Hash algorithm ID */
+};
+
+struct tpm_bank_pcr_values {
+ u16 alg_id;
+ u8 **digests; /* Array of pointers, one per configured PCR */
+};
+
+extern u8 num_tpm_banks;
+extern u8 num_pcr_configured;
+
+extern signed char algo_pcr_bank_map[HASH_ALGO__LAST];
+extern signed char pcr_digests_map[TPM2_PLATFORM_PCR];
+extern signed char digests_pcr_map[TPM2_PLATFORM_PCR];
+
+extern struct tpm_bank_pcr_values *ima_start_point_pcr_values;
+
+extern int ima_trim_event_log(const void *bin);
+#else
+static inline int ima_trim_event_log(const void *bin) { return 0; }
+#endif /* CONFIG_IMA_PCRS */
+
#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
extern void ima_appraise_parse_cmdline(void);
#else
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index b376d38b4ee6..ae9ce210638d 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -18,3 +18,7 @@ ima-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.o
ifeq ($(CONFIG_EFI),y)
ima-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_efi.o
endif
+
+ifeq ($(CONFIG_IMA_PCRS),y)
+ima-y += ima_log_trim.o
+endif
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e3d71d8d56e3..d9accf504e42 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -55,6 +55,8 @@ struct ima_algo_desc {
enum hash_algo algo;
};
+extern struct mutex ima_extend_list_mutex; /* protects extending measurement list */
+
/* set during initialization */
extern int ima_hash_algo __ro_after_init;
extern int ima_sha1_idx __ro_after_init;
@@ -250,6 +252,12 @@ void ima_measure_kexec_event(const char *event_name);
static inline void ima_measure_kexec_event(const char *event_name) {}
#endif
+#ifdef CONFIG_IMA_PCRS
+extern int ima_add_configured_pcr(int new_pcr_index);
+#else
+static inline int ima_add_configured_pcr(int new_pcr_index) { return 0; }
+#endif /* CONFIG_IMA_PCRS */
+
/*
* The default binary_runtime_measurements list format is defined as the
* platform native format. The canonical format is defined as little-endian.
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index a2f34f2d8ad7..1102d2073336 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -24,6 +24,16 @@
const char boot_aggregate_name[] = "boot_aggregate";
struct tpm_chip *ima_tpm_chip;
+#ifdef CONFIG_IMA_PCRS
+u8 num_tpm_banks;
+u8 num_pcr_configured;
+signed char algo_pcr_bank_map[HASH_ALGO__LAST];
+signed char digests_pcr_map[TPM2_PLATFORM_PCR];
+signed char pcr_digests_map[TPM2_PLATFORM_PCR];
+
+struct tpm_bank_pcr_values *ima_start_point_pcr_values;
+#endif
+
/* Add the boot aggregate to the IMA measurement list and extend
* the PCR register.
*
@@ -134,6 +144,40 @@ int __init ima_init(void)
if (rc != 0)
return rc;
+#ifdef CONFIG_IMA_PCRS
+
+ num_tpm_banks = 0;
+ num_pcr_configured = 0;
+
+ for (int i = 0; i < TPM2_PLATFORM_PCR; i++) {
+ pcr_digests_map[i] = -1;
+ digests_pcr_map[i] = -1;
+ }
+
+ for (int i = 0; i < HASH_ALGO__LAST; i++)
+ algo_pcr_bank_map[i] = -1;
+
+ if (ima_tpm_chip) {
+ num_tpm_banks = ima_tpm_chip->nr_allocated_banks;
+ /* Allocate memory for the ima start point PCR values */
+ ima_start_point_pcr_values = kcalloc(num_tpm_banks,
+ sizeof(*ima_start_point_pcr_values),
+ GFP_KERNEL);
+
+ if (!ima_start_point_pcr_values)
+ return -ENOMEM;
+
+ for (int i = 0; i < num_tpm_banks; i++) {
+ int algo_id = ima_tpm_chip->allocated_banks[i].crypto_id;
+
+ ima_start_point_pcr_values[i].alg_id = algo_id;
+ algo_pcr_bank_map[algo_id] = i;
+ }
+
+ ima_add_configured_pcr(CONFIG_IMA_MEASURE_PCR_IDX);
+ }
+#endif
+
/* It can be called before ima_init_digests(), it does not use TPM. */
ima_load_kexec_buffer();
diff --git a/security/integrity/ima/ima_log_trim.c b/security/integrity/ima/ima_log_trim.c
new file mode 100644
index 000000000000..85025d502db2
--- /dev/null
+++ b/security/integrity/ima/ima_log_trim.c
@@ -0,0 +1,421 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Microsoft Corporation
+ *
+ * Authors:
+ * Steven Chen <chenste@linux.microsoft.com>
+ *
+ * File: ima_log_trim.c
+ * Implements PCR value based IMA Event Log Trimming
+ */
+
+#include <linux/fcntl.h>
+#include <linux/kernel_read_file.h>
+#include <linux/slab.h>
+#include <linux/seq_file.h>
+#include <linux/rculist.h>
+#include <linux/rcupdate.h>
+#include <linux/vmalloc.h>
+#include <linux/ima.h>
+#include <crypto/hash_info.h>
+
+#include "ima.h"
+
+struct digest_value {
+ u8 digest[HASH_MAX_DIGESTSIZE]; /* digest value */
+};
+
+/* Average IMA event log data length */
+#define EXTEND_BUF_LEN 300
+
+static struct digest_value *ima_extended_pcr;
+static struct ima_pcr_value *target_pcr_values;
+static unsigned long pcr_matched;
+static int pcr_match_needed;
+
+/* Calculate the hash digest for the given data using the specified algorithm */
+static int ima_calculate_pcr(const void *data, size_t len, u8 *md, int algo)
+{
+ struct crypto_shash *tfm;
+ struct shash_desc *desc;
+ int ret = 0, size;
+
+ tfm = crypto_alloc_shash(hash_algo_name[algo], 0, 0);
+
+ if (IS_ERR(tfm)) {
+ ret = -ENOMEM;
+ goto out_nocleanup;
+ }
+
+ /* Allocate memory for the shash_desc structure and initialize it */
+ size = sizeof(struct shash_desc) + crypto_shash_descsize(tfm);
+
+ desc = kzalloc(size, GFP_KERNEL);
+ if (!desc) {
+ crypto_free_shash(tfm);
+ ret = -ENOMEM;
+ goto out_nocleanup;
+ }
+
+ desc->tfm = tfm;
+
+ if (crypto_shash_init(desc)) {
+ ret = -EINVAL;
+ goto out_cleanup;
+ }
+
+ /* Update the hash with the input data */
+ if (crypto_shash_update(desc, data, len)) {
+ ret = -EINVAL;
+ goto out_cleanup;
+ }
+
+ /* Finalize the hash and retrieve the digest */
+ ret = crypto_shash_final(desc, md);
+ if (ret)
+ goto out_cleanup;
+
+ ret = crypto_shash_digestsize(tfm);
+
+out_cleanup:
+ kfree(desc);
+ crypto_free_shash(tfm);
+out_nocleanup:
+ return ret;
+}
+
+/* Add a new configured PCR */
+int ima_add_configured_pcr(int new_pcr_index)
+{
+ if (!ima_tpm_chip)
+ return -1;
+
+ if (new_pcr_index < 0 || new_pcr_index >= TPM2_PLATFORM_PCR)
+ return -1;
+
+ if (pcr_digests_map[new_pcr_index] != -1)
+ return 0;
+
+ /*
+ * For each TPM bank, expand one more entry for the new PCR
+ * Allocate new PCR digest buffers for the new PCR
+ */
+ for (int i = 0; i < num_tpm_banks; ++i) {
+ int length = hash_digest_size[ima_start_point_pcr_values[i].alg_id];
+ u8 *new_pcr = kmalloc(sizeof(u8) * length, GFP_KERNEL);
+ u8 **new_buf;
+
+ if (!new_pcr)
+ return -ENOMEM;
+ memset(new_pcr, 0, sizeof(u8) * length);
+ new_buf = (u8 **)krealloc(ima_start_point_pcr_values[i].digests,
+ (num_pcr_configured + 1) * sizeof(u8 *), GFP_KERNEL);
+ if (!new_buf) {
+ kfree(new_pcr);
+ return -ENOMEM;
+ }
+ new_buf[num_pcr_configured] = new_pcr;
+ ima_start_point_pcr_values[i].digests = new_buf;
+ }
+
+ pcr_digests_map[new_pcr_index] = num_pcr_configured;
+ digests_pcr_map[num_pcr_configured] = new_pcr_index;
+
+ ++num_pcr_configured;
+ return 0;
+}
+
+/* Delete the IMA event logs */
+static int ima_purge_event_log(int number_logs)
+{
+ struct ima_queue_entry *qe;
+ int cur = 0;
+
+ mutex_lock(&ima_extend_list_mutex);
+ rcu_read_lock();
+
+ /*
+ * Remove this entry from both hash table and the measurement list
+ * When removing from hash table, decrease the length counter
+ * so that the hash table re-sizing logic works correctly
+ */
+ list_for_each_entry_rcu(qe, &ima_measurements, later) {
+ /* if CONFIG_IMA_DISABLE_HTABLE is set, the hash table is not used */
+ if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE))
+ hlist_del_rcu(&qe->hnext);
+
+ atomic_long_dec(&ima_htable.len);
+ list_del_rcu(&qe->later);
+ ++cur;
+ if (cur >= number_logs)
+ break;
+ }
+
+ rcu_read_unlock();
+ mutex_unlock(&ima_extend_list_mutex);
+ return cur;
+}
+
+/* compare the target PCR values with IMA Start Point PCR Values */
+static int ima_compare_pcr_values(struct ima_pcr_value *target_pcr_val)
+{
+ int count_not_matched = 0;
+
+ for (int i = 0; i < num_pcr_configured; ++i) {
+ int algo_id_tmp;
+ int bank_id_tmp;
+ int hash_size;
+
+ algo_id_tmp = target_pcr_val[i].algo_id;
+ bank_id_tmp = algo_pcr_bank_map[algo_id_tmp];
+ hash_size = hash_digest_size[algo_id_tmp];
+
+ if (bank_id_tmp < 0 || bank_id_tmp >= num_tpm_banks)
+ return -EINVAL;
+
+ /* already matched this PCR, skip it */
+ if (memcmp(ima_start_point_pcr_values[bank_id_tmp].digests[i],
+ target_pcr_val[i].digest, hash_size) == 0) {
+ set_bit(i, &pcr_matched);
+ continue;
+ }
+ count_not_matched++;
+ }
+ return count_not_matched;
+}
+
+static int ima_recalculate_check_pcrs(int pcr_map_index, char *org_pointer, char *digest_pointer,
+ int total_length, bool digest_zero)
+{
+ int ret = 0;
+
+ /* Recalculate the PCR for each bank */
+ for (int i = 0; i < num_tpm_banks; i++) {
+ int digest_len, crypto_id, hash_size, idx, len;
+ u8 digest[HASH_MAX_DIGESTSIZE];
+
+ crypto_id = ima_start_point_pcr_values[i].alg_id;
+ hash_size = hash_digest_size[crypto_id];
+ digest_len = 0;
+ /* Prepare digest buffer */
+ if (digest_zero) {
+ /* all zero digest, use 0xFF to extend */
+ memset(digest, 0xFF, HASH_MAX_DIGESTSIZE);
+ digest_len = hash_size;
+ } else {
+ memcpy(digest_pointer, org_pointer, total_length);
+ digest_len = ima_calculate_pcr(digest_pointer, total_length, digest,
+ crypto_id);
+ if (digest_len < 0) {
+ ret = digest_len;
+ break;
+ }
+ }
+
+ len = digest_len + hash_size;
+
+ idx = i * num_pcr_configured + pcr_map_index;
+ memcpy(digest_pointer, ima_extended_pcr[idx].digest, hash_size);
+ memcpy(digest_pointer + hash_size, digest, digest_len);
+
+ /* Recalculate the PCR starting with the IMA Start Point PCR value */
+ digest_len = ima_calculate_pcr(digest_pointer, len, ima_extended_pcr[idx].digest,
+ crypto_id);
+
+ if (digest_len < 0) {
+ ret = digest_len;
+ break;
+ }
+
+ /*
+ * Check if the extended PCR value matches the target PCR value
+ * if matched, mark this PCR as matched
+ * if all PCRs matched, set the entry_found flag
+ */
+ if (crypto_id == target_pcr_values[pcr_map_index].algo_id) {
+ if (memcmp(ima_extended_pcr[idx].digest,
+ target_pcr_values[pcr_map_index].digest, hash_size) == 0) {
+ set_bit(pcr_map_index, &pcr_matched);
+ --pcr_match_needed;
+ }
+ }
+ }
+
+ return ret;
+}
+
+static int ima_get_log_count(void)
+{
+ u8 algo_digest_buffer[EXTEND_BUF_LEN];
+ u8 digest_buffer[EXTEND_BUF_LEN];
+ struct ima_queue_entry *qe;
+ int count = 0;
+ int ret = 0;
+ unsigned int hash_digest_length;
+
+ /* Event log digests algorithm is SHA1 */
+ hash_digest_length = hash_digest_size[HASH_ALGO_SHA1];
+ list_for_each_entry_rcu(qe, &ima_measurements, later) {
+ char *org_digest_pointer, *digest_pointer;
+ int pcr_idx, pcr_map_index, total_length;
+ struct ima_template_entry *e = qe->entry;
+ bool digest_zero;
+
+ if (!qe->entry) {
+ ret = -EINVAL;
+ break;
+ }
+ pcr_idx = e->pcr;
+ pcr_map_index = pcr_digests_map[pcr_idx];
+
+ if (test_bit(pcr_map_index, &pcr_matched) || pcr_digests_map[pcr_idx] == -1) {
+ /* already matched this PCR, something wrong */
+ ret = -EINVAL;
+ break;
+ }
+ /* The original digest buffer is used to save data for multiple banks/algorithms */
+ org_digest_pointer = digest_buffer;
+ digest_pointer = algo_digest_buffer;
+
+ total_length = e->template_data_len;
+
+ /* Allocate large memory for the original and digest buffers if needed */
+ if (total_length > EXTEND_BUF_LEN) {
+ org_digest_pointer = kzalloc(total_length, GFP_KERNEL);
+ if (!org_digest_pointer) {
+ ret = -ENOMEM;
+ break;
+ }
+ digest_pointer = kzalloc(total_length, GFP_KERNEL);
+ if (!digest_pointer) {
+ kfree(org_digest_pointer);
+ ret = -ENOMEM;
+ break;
+ }
+ }
+
+ digest_zero = true;
+ /*
+ * Check if the original digest is all zeros or not
+ * if not all zero, use template data to recalculate PCR
+ */
+ if (memchr_inv(e->digests->digest, 0, hash_digest_length) != NULL) {
+ int offset = 0;
+
+ for (int i = 0; i < e->template_desc->num_fields; i++) {
+ memcpy(org_digest_pointer + offset, &e->template_data[i].len,
+ sizeof(e->template_data[i].len));
+ offset += sizeof(e->template_data[i].len);
+ memcpy(org_digest_pointer + offset, e->template_data[i].data,
+ e->template_data[i].len);
+ offset += e->template_data[i].len;
+ }
+ digest_zero = false;
+ }
+
+ count++;
+
+ /* Check if this log entry can match the target PCRs */
+ ret = ima_recalculate_check_pcrs(pcr_map_index, org_digest_pointer,
+ digest_pointer, total_length, digest_zero);
+
+ if (total_length > EXTEND_BUF_LEN) {
+ kfree(org_digest_pointer);
+ kfree(digest_pointer);
+ }
+
+ /* If entry found or error occurred, break the loop */
+ if (ret < 0 || pcr_match_needed <= 0)
+ break;
+ }
+
+ if (ret < 0)
+ return ret;
+
+ if (pcr_match_needed <= 0)
+ return count;
+ else
+ return 0;
+}
+
+/*
+ * Trim the IMA event log to match the given PCR values
+ * Return:
+ * >0: number of log entries removed
+ * 0: no log entries removed
+ * -1: error
+ * -ENOENT: no matching log entry found
+ * -EIO: I/O error when removing log entries
+ * -EINVAL: invalid parameter
+ * -ENOMEM: memory allocation failure
+ */
+int ima_trim_event_log(const void *bin)
+{
+ int count, ret = -1;
+
+ count = 0;
+ target_pcr_values = (struct ima_pcr_value *)bin;
+ pcr_matched = 0;
+
+ pcr_match_needed = ima_compare_pcr_values(target_pcr_values);
+
+ /* No need to trim */
+ if (pcr_match_needed <= 0) {
+ ret = 0;
+ goto out_nofree;
+ }
+
+ ima_extended_pcr = kcalloc(num_tpm_banks * num_pcr_configured,
+ sizeof(*ima_extended_pcr), GFP_KERNEL);
+
+ if (!ima_extended_pcr) {
+ ret = -ENOMEM;
+ goto out_nofree;
+ }
+
+ /* Initialize ima_extended_pcr with ima_start_point_pcr_values */
+ for (int i = 0; i < num_tpm_banks; i++) {
+ int length = hash_digest_size[ima_start_point_pcr_values[i].alg_id];
+
+ for (int j = 0; j < num_pcr_configured; ++j) {
+ int record_index = i * num_pcr_configured + j;
+
+ memcpy(ima_extended_pcr[record_index].digest,
+ ima_start_point_pcr_values[i].digests[j], length);
+ }
+ }
+
+ ret = ima_get_log_count();
+
+ if (ret <= 0)
+ goto out_notrim;
+
+ count = ret;
+
+ /* Remove logs from the IMA log list */
+ ret = ima_purge_event_log(count);
+
+ if (ret == count) {
+ /* Update the IMA Start Point PCR values */
+ for (int i = 0; i < num_tpm_banks; i++) {
+ int algorithm_id = ima_start_point_pcr_values[i].alg_id;
+ int hash_size = hash_digest_size[algorithm_id];
+
+ for (int j = 0; j < num_pcr_configured; j++) {
+ int ext_idx = i * num_pcr_configured + j;
+
+ memcpy(ima_start_point_pcr_values[i].digests[j],
+ ima_extended_pcr[ext_idx].digest, hash_size);
+ }
+ }
+ } else {
+ /* something wrong, should not happen */
+ ret = -EIO;
+ }
+
+out_notrim:
+ kfree(ima_extended_pcr);
+
+out_nofree:
+ return ret;
+}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 7468afaab686..fe537827ac1f 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1895,10 +1895,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
ima_log_string(ab, "pcr", args[0].from);
result = kstrtoint(args[0].from, 10, &entry->pcr);
- if (result || INVALID_PCR(entry->pcr))
+ if (result || INVALID_PCR(entry->pcr)) {
result = -EINVAL;
- else
+ } else {
entry->flags |= IMA_PCR;
+ if (ima_add_configured_pcr(entry->pcr) < 0)
+ result = -EINVAL;
+ }
break;
case Opt_template:
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 590637e81ad1..7bbfdd2ce3b0 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -39,11 +39,12 @@ struct ima_h_table ima_htable = {
.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
};
-/* mutex protects atomicity of extending measurement list
+/*
+ * This mutex protects atomicity of extending measurement list
* and extending the TPM PCR aggregate. Since tpm_extend can take
* long (and the tpm driver uses a mutex), we can't use the spinlock.
*/
-static DEFINE_MUTEX(ima_extend_list_mutex);
+DEFINE_MUTEX(ima_extend_list_mutex);
/*
* Used internally by the kernel to suspend measurements.
--
2.43.0
^ permalink raw reply related
* [RFC v1 0/1] Implement IMA Event Log Trimming
From: Anirudh Venkataramanan @ 2025-11-19 21:33 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E . Hallyn, linux-security-module,
Anirudh Venkataramanan, Steven Chen, Gregory Lumen,
Lakshmi Ramasubramanian, Sush Shringarputale
==========================================================================
| A. Introduction |
==========================================================================
IMA events are kept in kernel memory and preserved across kexec soft
reboots. This can lead to increased kernel memory usage over time,
especially with aggressive IMA policies that measure everything. To reduce
memory pressure, it becomes necessary to discard IMA events but given that
IMA events are extended into PCRs in the TPM, just discarding events will
break the PCR extension chain, making future verification of the IMA event
log impossible.
This patch series proposes a method to discard IMA events while keeping the
log verifiable. While reducing memory pressure is the primary objective,
the second order benefit of trimming the IMA log is that IMA log verifiers
(local userspace daemon or a remote cloud service) can process smaller IMA
logs on a rolling basis, thus avoiding re-verification of previously
verified events.
The method has other advantages too:
1. It provides a userspace interface that can be used to precisely control
trim point, allowing for trim points to be optionally aligned with
userspace IMA event log validation.
2. It ensures that all necessary information required for continued IMA
log validation is made available via the userspace interface at all
times.
3. It provides a simple mechanism for userspace applications to determine
if the event log has been unexpectedly trimmed.
4. The duration for which the IMA Measurement list mutex must be held (for
trimming) is minimal.
==========================================================================
| B. Solution |
==========================================================================
--------------------------------------------------------------------------
| B.1 Overview |
--------------------------------------------------------------------------
The kernel trims the IMA event log based on PCR values supplied by userspace.
The core principles leveraged are as follows:
- Given an IMA event log, PCR values for each IMA event can be obtained by
recalulating the PCR extension for each event. Thus processing N events
from the start will yield PCR values as of event N. This is referred to
as "IMA event log replay".
- To get the PCR value for event N + 1, only the PCR value as of event N
is needed. If this can be known, events till and including N can be
safely purged.
Putting it all together, we get the following userspace + kernel flow:
1. A userspace application replays the IMA event log to generate PCR
values and then triggers a trim by providing these values to the kernel
(by writing to a pseudo file).
Optionally, the userspace application may verify these PCR values
against the corresponding TPM quote, and trigger trimming only if
the calculated PCR values match up to the expectations in the quote's
PCR digest.
2. The kernel uses the userspace supplied PCR values to trim the IMA
measurements list at a specific point, and so these are referred to as
"trim-to PCR values" in this context.
Note that the kernel doesn't really understand what these userspace
provided PCR values mean or what IMA event they correspond to, and so
it does its own IMA event replay till either the replayed PCR values
match with the userspace provided ones, or it runs out of events.
If a match is found, the kernel can proceed with trimming the IMA
measurements list. This is done in two steps, to keep locking context
minimal.
step 1: Find and return the list entry (as a count from head) of exact
match. This does not lock the measurements list mutex, ensuring
new events can be appended to the log.
step 2: Lock the measurements list mutex and trim the measurements list
at the previously identified list entry.
If the trim is successful, the trim-to PCR values are saved as "starting
PCR values". The next time userspace wants to replay the IMA event log,
it will use the starting PCR values as the base for the IMA event log
replay.
--------------------------------------------------------------------------
| B.2 Kernel Interfaces |
--------------------------------------------------------------------------
A new configfs pseudo file /sys/kernel/config/ima/pcrs that supports the
following operations is exposed.
read: returns starting PCR values stored in the kernel (within IMA
specifically).
write: writes trim-to PCR values to trigger trimming. If trimming is
successful, trim-to PCR values are stored as starting PCR values.
requires root privileges.
--------------------------------------------------------------------------
| B.3 Walk-through with a real example |
--------------------------------------------------------------------------
This is a real example from a test run.
Suppose this IMA policy is deployed:
measure func=FILE_CHECK mask=MAY_READ pcr=10
measure func=FILE_CHECK mask=MAY_WRITE pcr=11
When the policy is deployed, a zero digest starting PCR value will be set
for each PCR used. If the TPM supports multiple hashbanks, there will be
one starting PCR value per PCR, per TPM hashbank. This can be seen in the
following hexdump:
$ sudo hexdump -vC /sys/kernel/config/ima/pcrs
00000000 70 63 72 31 30 3a 73 68 61 31 3a 00 00 00 00 00 |pcr10:sha1:.....|
00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 70 |...............p|
00000020 63 72 31 31 3a 73 68 61 31 3a 00 00 00 00 00 00 |cr11:sha1:......|
00000030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 70 63 |..............pc|
00000040 72 31 30 3a 73 68 61 32 35 36 3a 00 00 00 00 00 |r10:sha256:.....|
00000050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000060 00 00 00 00 00 00 00 00 00 00 00 70 63 72 31 31 |...........pcr11|
00000070 3a 73 68 61 32 35 36 3a 00 00 00 00 00 00 00 00 |:sha256:........|
00000080 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000090 00 00 00 00 00 00 00 00 70 63 72 31 30 3a 73 68 |........pcr10:sh|
000000a0 61 33 38 34 3a 00 00 00 00 00 00 00 00 00 00 00 |a384:...........|
000000b0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
000000c0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
000000d0 00 00 00 00 00 70 63 72 31 31 3a 73 68 61 33 38 |.....pcr11:sha38|
000000e0 34 3a 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |4:..............|
000000f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000100 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000110 00 00 |..|
00000112
Let's say that a userspace utility replays the IMA event log, and triggers
trimming by writing the following PCR values (i.e. trim-to PCR values) to the
pseudo file:
pcr10:sha256:8268782906555cf3aefc179f815c878527dd4e67eaa836572ebabab31977922c
pcr11:sha256:4c7f31927183eacb53d51d95b0162916fd3fca51a8d1efc6dde3805eb891fe41
The trim is successful,
1. Some number of entries from the measurements log will disappear. This
can be verified by reading out the ASCII or binary IMA measurements
file.
2. The trim-to PCR values are saved as starting PCR values. This can be
verified by reading out the pseudo file again as shown below. Note that
even through only sha256 PCR values were written, the kernel populated
sha1 and sha384 starting values as well.
$ sudo hexdump -vC /sys/kernel/config/ima/pcrs
00000000 70 63 72 31 30 3a 73 68 61 31 3a c4 7f 9d 00 68 |pcr10:sha1:....h|
00000010 e4 86 71 bf bc ae f0 10 12 ff 68 e2 9e 74 e4 70 |..q.......h..t.p|
00000020 63 72 31 31 3a 73 68 61 31 3a 90 d7 17 ac 60 4d |cr11:sha1:....`M|
00000030 c8 25 ce 77 7d 9d 94 cf 44 7b b2 2e 2e e2 70 63 |.%.w}...D{....pc|
00000040 72 31 30 3a 73 68 61 32 35 36 3a 82 68 78 29 06 |r10:sha256:.hx).|
00000050 55 5c f3 ae fc 17 9f 81 5c 87 85 27 dd 4e 67 ea |U\......\..'.Ng.|
00000060 a8 36 57 2e ba ba b3 19 77 92 2c 70 63 72 31 31 |.6W.....w.,pcr11|
00000070 3a 73 68 61 32 35 36 3a 4c 7f 31 92 71 83 ea cb |:sha256:L.1.q...|
00000080 53 d5 1d 95 b0 16 29 16 fd 3f ca 51 a8 d1 ef c6 |S.....)..?.Q....|
00000090 dd e3 80 5e b8 91 fe 41 70 63 72 31 30 3a 73 68 |...^...Apcr10:sh|
000000a0 61 33 38 34 3a 8e d6 12 18 b1 d6 cd 95 16 98 33 |a384:..........3|
000000b0 2b 7d a2 d6 d9 05 c7 e8 5b 15 b0 91 c5 fc 23 d1 |+}......[.....#.|
000000c0 f9 a8 8d 60 50 5c e9 64 5f d7 b3 b2 f1 9c 90 0a |...`P\.d_.......|
000000d0 45 53 5d b2 57 70 63 72 31 31 3a 73 68 61 33 38 |ES].Wpcr11:sha38|
000000e0 34 3a 25 fc 21 28 31 5a f7 c6 fb 0f 40 c9 06 e6 |4:%.!(1Z....@...|
000000f0 c5 da ed 20 61 a1 03 54 4f 67 18 88 82 0f 48 d1 |... a..TOg....H.|
00000100 2f e0 3d 36 46 5e 94 a4 88 51 f8 91 39 7e e5 97 |/.=6F^...Q..9~..|
00000110 2c c5 |,.|
00000112
--------------------------------------------------------------------------
| C. Footnotes |
--------------------------------------------------------------------------
1. The 'pcrs' pseudo file is currently part of configfs. This was due to
some early internal feedback in a different context. This can as well be
in securityfs with the rest of the IMA pseudo files.
2. PCR values are never read out of the TPM at any point. All PCR values
used are derived from IMA event log replay.
3. Code is "RFC quality". Refinements can be made if the method is accepted.
4. For functional validation, base kernel version was 6.17 stable, with the
most recent tested version being 6.17.8.
5. Code has been validated to some degree using a python-based internal test
tool. This can be published if there is community interest.
Steven Chen (1):
ima: Implement IMA event log trimming
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/ima/Kconfig | 13 +
drivers/ima/Makefile | 2 +
drivers/ima/ima_config_pcrs.c | 291 ++++++++++++++++++
include/linux/ima.h | 27 ++
security/integrity/ima/Makefile | 4 +
security/integrity/ima/ima.h | 8 +
security/integrity/ima/ima_init.c | 44 +++
security/integrity/ima/ima_log_trim.c | 421 ++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 7 +-
security/integrity/ima/ima_queue.c | 5 +-
12 files changed, 821 insertions(+), 4 deletions(-)
create mode 100644 drivers/ima/Kconfig
create mode 100644 drivers/ima/Makefile
create mode 100644 drivers/ima/ima_config_pcrs.c
create mode 100644 security/integrity/ima/ima_log_trim.c
--
2.43.0
^ permalink raw reply
* Re: [PATCH v3 7/9] module: Move lockdown check into generic module loader
From: Paul Moore @ 2025-11-19 19:55 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Thomas Weißschuh, Masahiro Yamada, Nathan Chancellor,
Arnd Bergmann, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, James Morris, Serge E. Hallyn, Jonathan Corbet,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <20251119112055.W1l5FOxc@linutronix.de>
On Wed, Nov 19, 2025 at 6:20 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> On 2025-04-29 15:04:34 [+0200], Thomas Weißschuh wrote:
> > The lockdown check buried in module_sig_check() will not compose well
> > with the introduction of hash-based module validation.
>
> An explanation of why would be nice.
/me shrugs
I thought the explanation was sufficient.
> > Move it into module_integrity_check() which will work better.
> >
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 0/9] module: Introduce hash-based integrity checking
From: Sebastian Andrzej Siewior @ 2025-11-19 15:48 UTC (permalink / raw)
To: James Bottomley
Cc: Thomas Weißschuh, Masahiro Yamada, Nathan Chancellor,
Arnd Bergmann, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Paul Moore, James Morris, Serge E. Hallyn,
Jonathan Corbet, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <f1dca9daa01d0d2432c12ecabede3fa1389b1d29.camel@HansenPartnership.com>
On 2025-04-29 10:05:04 [-0400], James Bottomley wrote:
> On Tue, 2025-04-29 at 15:04 +0200, Thomas Weißschuh wrote:
> > The current signature-based module integrity checking has some
> > drawbacks in combination with reproducible builds:
> > Either the module signing key is generated at build time, which makes
> > the build unreproducible,
>
> I don't believe it does: as long as you know what the key was, which
> you can get from the kernel keyring, you can exactly reproduce the core
> build (it's a public key after all and really equivalent to built in
> configuration). Is the fact that you have to boot the kernel to get
> the key the problem? In which case we could insist it be shipped in
> the kernel packaging.
The kernel itself is signed. This is not a problem because distros have
the "unsigned" package which is used for comparison.
The modules are signed by an ephemeral key which is created at build
time. This is where the problem starts:
- the public key is embedded into the kernel. Extracting it with tooling
is possible (or it is part of the kernel package). Adding this key
into the build process while rebuilding the kernel should work.
This will however alter the build process and is not *the* original
one, which was used to build the image.
- the private key remains unknown which means the modules can not be
signed. The rebuilding would need to get past this limitation and the
logic must not be affected by this "change". Then the modules need to
be stripped of their signature for the comparison.
Doing all this requires additional handling/ tooling on the "validation"
infrastructure. This infrastructure works currently without special
care.
Adding special care will not build the package exactly like it has been
built originally _and_ the results need to be interpreted (as in we
remove this signature and do this and now it is fine).
Adding hashes of each module into the kernel image looks like a
reasonable thing to do. I don't see any downsides to this. Yes, you are
limited to the modules available at build time but this is also the case
today with the ephemeral key. It is meant for distros not for individual
developers testing their code.
With this change it is possible to build a kernel and its modules and
put the result in an archive such as tar/ deb/ rpm. You can build the
package _again_ following exactly the same steps as you did before and
the result will be the identical archive.
Bit by bit.
No need for interpreting the results, stripping signatures or altering
the build process.
I fully agree with this approach. I don't like the big hash array but I
have an idea how to optimize that part. So I don't see a problem in the
long term.
> Regards,
>
> James
Sebastian
^ permalink raw reply
* Re: [PATCH v4] ima: Access decompressed kernel module to verify appended signature
From: Mimi Zohar @ 2025-11-19 15:29 UTC (permalink / raw)
To: Coiby Xu, linux-integrity
Cc: Karel Srot, Paul Moore, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, James Morris, Serge E. Hallyn, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, open list,
open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
open list:SELINUX SECURITY MODULE
In-Reply-To: <20251119140326.787451-1-coxu@redhat.com>
On Wed, 2025-11-19 at 22:03 +0800, Coiby Xu wrote:
> Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
> is enabled, IMA has no way to verify the appended module signature as it
> can't decompress the module.
>
> Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
> IMA can calculate the compressed kernel module data hash on
> READING_MODULE_COMPRESSED and defer appraising/measuring it until on
> READING_MODULE when the module has been decompressed.
>
> Before enabling in-kernel module decompression, a kernel module in
> initramfs can still be loaded with ima_policy=secure_boot. So adjust the
> kernel module rule in secure_boot policy to allow either an IMA
> signature OR an appended signature i.e. to use
> "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
>
> Reported-by: Karel Srot <ksrot@redhat.com>
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
Thanks, Coiby! The patch is now queued in next-integrity.
--
Mimi
^ permalink raw reply
* Re: [PATCH] KEYS: encrypted: Use pr_fmt()
From: Thorsten Blum @ 2025-11-19 14:45 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, linux-integrity, keyrings, linux-security-module,
linux-kernel
In-Reply-To: <aR0v9mLOKJsr_0Zm@kernel.org>
On 19. Nov 2025, at 03:48, Jarkko Sakkinen wrote:
> On Thu, Nov 13, 2025 at 01:35:44PM +0100, Thorsten Blum wrote:
>> Use pr_fmt() to automatically prefix all pr_<level>() log messages with
>
> This fails to describe what "use" means.
I don't understand what you mean. What's wrong with "use ... to ..."?
>> "encrypted_key: " and remove all manually added prefixes.
>>
>> Reformat the code accordingly and avoid line breaks in log messages.
>>
>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>> ---
>> security/keys/encrypted-keys/encrypted.c | 74 +++++++++++-------------
>> security/keys/encrypted-keys/encrypted.h | 2 +-
>> 2 files changed, 35 insertions(+), 41 deletions(-)
>>
>> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
>> index 513c09e2b01c..a8e8bf949b4b 100644
>> --- a/security/keys/encrypted-keys/encrypted.c
>> +++ b/security/keys/encrypted-keys/encrypted.c
>> @@ -11,6 +11,8 @@
>> * See Documentation/security/keys/trusted-encrypted.rst
>> */
>>
>
> Should have undef prepending.
Why is this necessary when the #define is at the top of a source file?
The kernel documentation [1] doesn't mention this anywhere. Isn't #undef
only needed when redefining 'pr_fmt' in the middle of a file to avoid a
compiler warning/error?
>> +#define pr_fmt(fmt) "encrypted_key: " fmt
>> +
>> [...]
Thanks,
Thorsten
[1] https://docs.kernel.org/core-api/printk-basics.html
^ permalink raw reply
* Re: [PATCH v3] ima: Access decompressed kernel module to verify appended signature
From: Coiby Xu @ 2025-11-19 14:05 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, Karel Srot, Paul Moore, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, James Morris, Serge E. Hallyn,
Fan Wu, Stephen Smalley, Ondrej Mosnacek, open list,
open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
open list:SELINUX SECURITY MODULE
In-Reply-To: <fc1d67e411ef53460517db4c03bdcf1b9d9f8a8f.camel@linux.ibm.com>
On Wed, Nov 19, 2025 at 08:29:22AM -0500, Mimi Zohar wrote:
>Hi Coiby,
Hi Mimi,
>
>On Wed, 2025-11-19 at 11:47 +0800, Coiby Xu wrote:
>> Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
>> is enabled, IMA has no way to verify the appended module signature as it
>> can't decompress the module.
>>
>> Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
>> IMA can know only to collect original module data hash on
>> READING_MODULE_COMPRESSED and defer appraising/measuring it until on
>> READING_MODULE when the module has been decompressed.
>
>This paragraph is a bit awkward. Perhaps something like:
>
>-> so IMA can calculate the compressed kernel module data hash and defer
>measuring/appraising ...
>
>>
>> Before enabling in-kernel module decompression, a kernel module in
>> initramfs can still be loaded with ima_policy=secure_boot. So adjust the
>> kernel module rule in secure_boot policy to allow either an IMA
>> signature OR an appended signature i.e. to use
>> "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
>>
>> Reported-by: Karel Srot <ksrot@redhat.com>
>> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>> Suggested-by: Paul Moore <paul@paul-moore.com>
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>
>Thanks, Coiby!
>
>The patch applies cleanly to linus' tree, but needs to be applied to next-
>integrity. Please re-base.
I've sent v4 which has been rebased onto next tree with improved
wording as suggested.
>
>--
>
>thanks,
>
>Mimi
>
--
Best regards,
Coiby
^ permalink raw reply
* [PATCH v4] ima: Access decompressed kernel module to verify appended signature
From: Coiby Xu @ 2025-11-19 14:03 UTC (permalink / raw)
To: linux-integrity, Mimi Zohar
Cc: Karel Srot, Paul Moore, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, James Morris, Serge E. Hallyn, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, open list,
open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
open list:SELINUX SECURITY MODULE
In-Reply-To: <20251031074016.1975356-1-coxu@redhat.com>
Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
is enabled, IMA has no way to verify the appended module signature as it
can't decompress the module.
Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
IMA can calculate the compressed kernel module data hash on
READING_MODULE_COMPRESSED and defer appraising/measuring it until on
READING_MODULE when the module has been decompressed.
Before enabling in-kernel module decompression, a kernel module in
initramfs can still be loaded with ima_policy=secure_boot. So adjust the
kernel module rule in secure_boot policy to allow either an IMA
signature OR an appended signature i.e. to use
"appraise func=MODULE_CHECK appraise_type=imasig|modsig".
Reported-by: Karel Srot <ksrot@redhat.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
include/linux/kernel_read_file.h | 1 +
kernel/module/main.c | 17 ++++++++++++++---
security/integrity/ima/ima_main.c | 24 ++++++++++++++++--------
security/integrity/ima/ima_policy.c | 3 ++-
security/ipe/hooks.c | 1 +
security/selinux/hooks.c | 5 +++--
6 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 90451e2e12bd..d613a7b4dd35 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -14,6 +14,7 @@
id(KEXEC_INITRAMFS, kexec-initramfs) \
id(POLICY, security-policy) \
id(X509_CERTIFICATE, x509-certificate) \
+ id(MODULE_COMPRESSED, kernel-module-compressed) \
id(MAX_ID, )
#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..7b3ec2fa6e7c 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,24 +3675,35 @@ static int idempotent_wait_for_completion(struct idempotent *u)
static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
{
+ bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
struct load_info info = { };
void *buf = NULL;
int len;
+ int err;
- len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+ len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+ compressed ? READING_MODULE_COMPRESSED :
+ READING_MODULE);
if (len < 0) {
mod_stat_inc(&failed_kreads);
return len;
}
- if (flags & MODULE_INIT_COMPRESSED_FILE) {
- int err = module_decompress(&info, buf, len);
+ if (compressed) {
+ err = module_decompress(&info, buf, len);
vfree(buf); /* compressed data is no longer needed */
if (err) {
mod_stat_inc(&failed_decompress);
mod_stat_add_long(len, &invalid_decompress_bytes);
return err;
}
+ err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+ READING_MODULE);
+ if (err) {
+ mod_stat_inc(&failed_kreads);
+ free_copy(&info, flags);
+ return err;
+ }
} else {
info.hdr = buf;
info.len = len;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index ebaebccfbe9a..edd0fd3d77a0 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -235,7 +235,8 @@ static void ima_file_free(struct file *file)
static int process_measurement(struct file *file, const struct cred *cred,
struct lsm_prop *prop, char *buf, loff_t size,
- int mask, enum ima_hooks func)
+ int mask, enum ima_hooks func,
+ enum kernel_read_file_id read_id)
{
struct inode *real_inode, *inode = file_inode(file);
struct ima_iint_cache *iint = NULL;
@@ -406,6 +407,12 @@ static int process_measurement(struct file *file, const struct cred *cred,
if (rc != 0 && rc != -EBADF && rc != -EINVAL)
goto out_locked;
+ /* Defer measuring/appraising kernel modules to READING_MODULE */
+ if (read_id == READING_MODULE_COMPRESSED) {
+ must_appraise = 0;
+ goto out_locked;
+ }
+
if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */
pathname = ima_d_path(&file->f_path, &pathbuf, filename);
@@ -486,14 +493,14 @@ static int ima_file_mmap(struct file *file, unsigned long reqprot,
if (reqprot & PROT_EXEC) {
ret = process_measurement(file, current_cred(), &prop, NULL,
- 0, MAY_EXEC, MMAP_CHECK_REQPROT);
+ 0, MAY_EXEC, MMAP_CHECK_REQPROT, 0);
if (ret)
return ret;
}
if (prot & PROT_EXEC)
return process_measurement(file, current_cred(), &prop, NULL,
- 0, MAY_EXEC, MMAP_CHECK);
+ 0, MAY_EXEC, MMAP_CHECK, 0);
return 0;
}
@@ -577,7 +584,7 @@ static int ima_bprm_check(struct linux_binprm *bprm)
security_current_getlsmprop_subj(&prop);
return process_measurement(bprm->file, current_cred(),
- &prop, NULL, 0, MAY_EXEC, BPRM_CHECK);
+ &prop, NULL, 0, MAY_EXEC, BPRM_CHECK, 0);
}
/**
@@ -607,7 +614,7 @@ static int ima_creds_check(struct linux_binprm *bprm, const struct file *file)
security_current_getlsmprop_subj(&prop);
return process_measurement((struct file *)file, bprm->cred, &prop, NULL,
- 0, MAY_EXEC, CREDS_CHECK);
+ 0, MAY_EXEC, CREDS_CHECK, 0);
}
/**
@@ -655,7 +662,7 @@ static int ima_file_check(struct file *file, int mask)
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, NULL, 0,
mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
- MAY_APPEND), FILE_CHECK);
+ MAY_APPEND), FILE_CHECK, 0);
}
static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
@@ -874,12 +881,13 @@ static int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
func = read_idmap[read_id] ?: FILE_CHECK;
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, NULL, 0,
- MAY_READ, func);
+ MAY_READ, func, 0);
}
const int read_idmap[READING_MAX_ID] = {
[READING_FIRMWARE] = FIRMWARE_CHECK,
[READING_MODULE] = MODULE_CHECK,
+ [READING_MODULE_COMPRESSED] = MODULE_CHECK,
[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
[READING_POLICY] = POLICY_CHECK
@@ -917,7 +925,7 @@ static int ima_post_read_file(struct file *file, char *buf, loff_t size,
func = read_idmap[read_id] ?: FILE_CHECK;
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, buf, size,
- MAY_READ, func);
+ MAY_READ, func, read_id);
}
/**
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 164d62832f8e..7468afaab686 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -244,7 +244,8 @@ static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
{.action = APPRAISE, .func = MODULE_CHECK,
- .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+ .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
+ IMA_CHECK_BLACKLIST},
{.action = APPRAISE, .func = FIRMWARE_CHECK,
.flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index d0323b81cd8f..1053a4acf589 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -118,6 +118,7 @@ int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
op = IPE_OP_FIRMWARE;
break;
case READING_MODULE:
+ case READING_MODULE_COMPRESSED:
op = IPE_OP_KERNEL_MODULE;
break;
case READING_KEXEC_INITRAMFS:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dfc22da42f30..c1ff69d5d76e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4275,7 +4275,7 @@ static int selinux_kernel_read_file(struct file *file,
{
int rc = 0;
- BUILD_BUG_ON_MSG(READING_MAX_ID > 7,
+ BUILD_BUG_ON_MSG(READING_MAX_ID > 8,
"New kernel_read_file_id introduced; update SELinux!");
switch (id) {
@@ -4283,6 +4283,7 @@ static int selinux_kernel_read_file(struct file *file,
rc = selinux_kernel_load_from_file(file, SYSTEM__FIRMWARE_LOAD);
break;
case READING_MODULE:
+ case READING_MODULE_COMPRESSED:
rc = selinux_kernel_load_from_file(file, SYSTEM__MODULE_LOAD);
break;
case READING_KEXEC_IMAGE:
@@ -4311,7 +4312,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
{
int rc = 0;
- BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7,
+ BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8,
"New kernel_load_data_id introduced; update SELinux!");
switch (id) {
base-commit: 43369273518f57b7d56c1cf12d636a809b7bd81b
--
2.51.1
^ permalink raw reply related
* Re: [PATCH v3] ima: Access decompressed kernel module to verify appended signature
From: Mimi Zohar @ 2025-11-19 13:29 UTC (permalink / raw)
To: Coiby Xu, linux-integrity
Cc: Karel Srot, Paul Moore, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, James Morris, Serge E. Hallyn, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, open list,
open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
open list:SELINUX SECURITY MODULE
In-Reply-To: <20251119034718.618008-1-coxu@redhat.com>
Hi Coiby,
On Wed, 2025-11-19 at 11:47 +0800, Coiby Xu wrote:
> Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
> is enabled, IMA has no way to verify the appended module signature as it
> can't decompress the module.
>
> Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
> IMA can know only to collect original module data hash on
> READING_MODULE_COMPRESSED and defer appraising/measuring it until on
> READING_MODULE when the module has been decompressed.
This paragraph is a bit awkward. Perhaps something like:
-> so IMA can calculate the compressed kernel module data hash and defer
measuring/appraising ...
>
> Before enabling in-kernel module decompression, a kernel module in
> initramfs can still be loaded with ima_policy=secure_boot. So adjust the
> kernel module rule in secure_boot policy to allow either an IMA
> signature OR an appended signature i.e. to use
> "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
>
> Reported-by: Karel Srot <ksrot@redhat.com>
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
Thanks, Coiby!
The patch applies cleanly to linus' tree, but needs to be applied to next-
integrity. Please re-base.
--
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v3 7/9] module: Move lockdown check into generic module loader
From: Sebastian Andrzej Siewior @ 2025-11-19 11:20 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Masahiro Yamada, Nathan Chancellor, Arnd Bergmann,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <20250429-module-hashes-v3-7-00e9258def9e@weissschuh.net>
On 2025-04-29 15:04:34 [+0200], Thomas Weißschuh wrote:
> The lockdown check buried in module_sig_check() will not compose well
> with the introduction of hash-based module validation.
An explanation of why would be nice.
> Move it into module_integrity_check() which will work better.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
Sebastian
^ permalink raw reply
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Coiby Xu @ 2025-11-19 3:52 UTC (permalink / raw)
To: Mimi Zohar
Cc: Paul Moore, linux-integrity, linux-security-module, Karel Srot,
James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <fca9a7b41a5e428fadfe2d7e3b004ada2763375c.camel@linux.ibm.com>
On Tue, Nov 18, 2025 at 07:19:50AM -0500, Mimi Zohar wrote:
>On Thu, 2025-11-13 at 12:06 +0800, Coiby Xu wrote:
>> On Fri, Nov 07, 2025 at 02:28:13PM -0500, Mimi Zohar wrote:
>> > On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
>> > > On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
>> > > > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
>> > > > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
>> > > > [...]
>> > > > >
>> > > > > Hi Coiby,
>> > > > >
>> > > > > Based on the conversation with Paul, there is no reason to remove the existing
>> > > > > security_kernel_post_read_file() call.
>> > > > >
>> > > > > The changes are similar to the 2nd link, but a bit different.
>> > > > > - Define a single enumeration named READING_MODULE_COMPRESSED.
>> > > > >
>> > > > > - In module/main.c add a new security_kernel_post_read_file() call immediately
>> > > > > after decompressing the kernel module. Like a previous version of this patch,
>> > > > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
>> > > > > based on MODULE_INIT_COMPRESSED_FILE.
>> > > > >
>> > > > > - In ima_post_read_file() defer verifying the signature when the enumeration is
>> > > > > READING_MODULE_COMPRESSED. (No need for a new function ima_read_kernel_module.)
>> > > >
>> > > > Hi Mimi,
>> > > >
>> > > > Thanks for summarizing your conversation with Paul! I can confirm Paul's
>> > > > approach works
>> > > > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
>> > > >
>> > > > While testing the patch today, I realized there is another
>> > > > issue/challenge introduced by in-kernel module decompression. IMA
>> > > > appraisal is to verify the digest of compressed kernel module but
>> > > > currently the passed buffer is uncompressed module. When IMA uses
>> > > > uncompressed module data to calculate the digest, xattr signature
>> > > > verification will fail. If we always make IMA read the original kernel
>> > > > module data again to calculate the digest, does it look like a
>> > > > quick-and-dirty fix? If we can assume people won't load kernel module so
>> > > > often, the performance impact is negligible. Otherwise we may have to
>> > > > introduce a new LSM hook so IMA can access uncompressed and original
>> > > > module data one time.
>> > >
>> > > ima_collect_measurement() stores the file hash info in the iint and uses that
>> > > information to verify the signature as stored in the security xattr.
>> > > Decompressing the kernel module shouldn't affect the xattr signature
>> > > verification.
>> >
>> > In the case when the compressed kernel module hasn't previously been measured or
>> > appraised before loading the kernel module, we need to "collect" the file data
>> > hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.
>> >
>> > An alternative to your suggestion of re-reading the original kernel module data
>> > to calculate the digest or defining a new hook, would be to define "collect" as
>> > a new "action" and pass the kernel_read_file_id enumeration to
>> > process_measurement(). IMA_COLLECTED already exists. Only IMA_COLLECT would
>> > need to be defined. The new collect "action" should be limited to
>> > func=MODULE_CHECK.
>> >
>> > The downside of this alternative is that it requires a new collect rule:
>> > collect func=MODULE_CHECK mask=MAY_READ uid=0
>> > appraise func=MODULE_CHECK appraise_type=imasig|modsig
>
>As it turns out, the "collect" rule is unnecessary. On
>READING_MODULE_COMPRESSED, process_measurement() should calculate the compressed
>file hash. Extending the IMA measurement list and verifying the signature can
>then be differed to READING_MODULE.
>
>>
>> Thank for suggesting an alternative! I've implemented the idea in
>> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_collect
>>
>> Note besides a new collect rule, another change is needed. Currently,
>> process_measurement only accepts enum ima_hooks thus it can't tell if
>> it's READING_MODULE_COMPRESSED so to only do collect action. So I
>> create a fake MODULE_COMPRESSED_CHECK func.
>
>Correct, either extending process_measurement() with the read_idmap enum or
>defining the fake hook would work.
>
>>
>> And for the idea of re-reading the original kernel module data, it has
>> been implemented in
>> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
>>
>> Both branches have applied your requested three changes including
>> respecting the 80 char line limit. Additionally, I made a change to the
>> IPE LSM because of the new READING_MODULE_COMPRESSED kernel_read_file_id
>> enumerate.
>>
>> After comparing the two implementations, personally I prefer re-reading
>> the original kernel module data because the change is smaller and it's
>> more user-friendly. But if there are other reasons I don't know, I'll
>> post the patches of the new collect action approach to the mailing list.
>
>The "re-reading" option fails some of the tests. As the "collect" rule isn't
>needed, let's stick with the first option.
Thanks for evaluating the two options! Yeah, without the "collect" rule,
the 1st option is much better as it doesn't have the issue of re-reading
the module.
--
Best regards,
Coiby
^ permalink raw reply
* [PATCH v3] ima: Access decompressed kernel module to verify appended signature
From: Coiby Xu @ 2025-11-19 3:47 UTC (permalink / raw)
To: linux-integrity, Mimi Zohar
Cc: Karel Srot, Paul Moore, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, James Morris, Serge E. Hallyn, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, open list,
open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
open list:SELINUX SECURITY MODULE
In-Reply-To: <20251031074016.1975356-1-coxu@redhat.com>
Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
is enabled, IMA has no way to verify the appended module signature as it
can't decompress the module.
Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
IMA can know only to collect original module data hash on
READING_MODULE_COMPRESSED and defer appraising/measuring it until on
READING_MODULE when the module has been decompressed.
Before enabling in-kernel module decompression, a kernel module in
initramfs can still be loaded with ima_policy=secure_boot. So adjust the
kernel module rule in secure_boot policy to allow either an IMA
signature OR an appended signature i.e. to use
"appraise func=MODULE_CHECK appraise_type=imasig|modsig".
Reported-by: Karel Srot <ksrot@redhat.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
include/linux/kernel_read_file.h | 1 +
kernel/module/main.c | 17 ++++++++++++++---
security/integrity/ima/ima_main.c | 24 ++++++++++++++++--------
security/integrity/ima/ima_policy.c | 3 ++-
security/ipe/hooks.c | 1 +
security/selinux/hooks.c | 5 +++--
6 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 90451e2e12bd..d613a7b4dd35 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -14,6 +14,7 @@
id(KEXEC_INITRAMFS, kexec-initramfs) \
id(POLICY, security-policy) \
id(X509_CERTIFICATE, x509-certificate) \
+ id(MODULE_COMPRESSED, kernel-module-compressed) \
id(MAX_ID, )
#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..7b3ec2fa6e7c 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,24 +3675,35 @@ static int idempotent_wait_for_completion(struct idempotent *u)
static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
{
+ bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
struct load_info info = { };
void *buf = NULL;
int len;
+ int err;
- len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+ len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+ compressed ? READING_MODULE_COMPRESSED :
+ READING_MODULE);
if (len < 0) {
mod_stat_inc(&failed_kreads);
return len;
}
- if (flags & MODULE_INIT_COMPRESSED_FILE) {
- int err = module_decompress(&info, buf, len);
+ if (compressed) {
+ err = module_decompress(&info, buf, len);
vfree(buf); /* compressed data is no longer needed */
if (err) {
mod_stat_inc(&failed_decompress);
mod_stat_add_long(len, &invalid_decompress_bytes);
return err;
}
+ err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+ READING_MODULE);
+ if (err) {
+ mod_stat_inc(&failed_kreads);
+ free_copy(&info, flags);
+ return err;
+ }
} else {
info.hdr = buf;
info.len = len;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index cdd225f65a62..49f8b2b1a9af 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -235,7 +235,8 @@ static void ima_file_free(struct file *file)
static int process_measurement(struct file *file, const struct cred *cred,
struct lsm_prop *prop, char *buf, loff_t size,
- int mask, enum ima_hooks func)
+ int mask, enum ima_hooks func,
+ enum kernel_read_file_id read_id)
{
struct inode *real_inode, *inode = file_inode(file);
struct ima_iint_cache *iint = NULL;
@@ -406,6 +407,12 @@ static int process_measurement(struct file *file, const struct cred *cred,
if (rc != 0 && rc != -EBADF && rc != -EINVAL)
goto out_locked;
+ /* Defer measuring/appraising kernel modules to READING_MODULE */
+ if (read_id == READING_MODULE_COMPRESSED) {
+ must_appraise = 0;
+ goto out_locked;
+ }
+
if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */
pathname = ima_d_path(&file->f_path, &pathbuf, filename);
@@ -486,14 +493,14 @@ static int ima_file_mmap(struct file *file, unsigned long reqprot,
if (reqprot & PROT_EXEC) {
ret = process_measurement(file, current_cred(), &prop, NULL,
- 0, MAY_EXEC, MMAP_CHECK_REQPROT);
+ 0, MAY_EXEC, MMAP_CHECK_REQPROT, 0);
if (ret)
return ret;
}
if (prot & PROT_EXEC)
return process_measurement(file, current_cred(), &prop, NULL,
- 0, MAY_EXEC, MMAP_CHECK);
+ 0, MAY_EXEC, MMAP_CHECK, 0);
return 0;
}
@@ -578,13 +585,13 @@ static int ima_bprm_check(struct linux_binprm *bprm)
security_current_getlsmprop_subj(&prop);
ret = process_measurement(bprm->file, current_cred(),
- &prop, NULL, 0, MAY_EXEC, BPRM_CHECK);
+ &prop, NULL, 0, MAY_EXEC, BPRM_CHECK, 0);
if (ret)
return ret;
security_cred_getlsmprop(bprm->cred, &prop);
return process_measurement(bprm->file, bprm->cred, &prop, NULL, 0,
- MAY_EXEC, CREDS_CHECK);
+ MAY_EXEC, CREDS_CHECK, 0);
}
/**
@@ -632,7 +639,7 @@ static int ima_file_check(struct file *file, int mask)
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, NULL, 0,
mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
- MAY_APPEND), FILE_CHECK);
+ MAY_APPEND), FILE_CHECK, 0);
}
static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
@@ -851,12 +858,13 @@ static int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
func = read_idmap[read_id] ?: FILE_CHECK;
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, NULL, 0,
- MAY_READ, func);
+ MAY_READ, func, 0);
}
const int read_idmap[READING_MAX_ID] = {
[READING_FIRMWARE] = FIRMWARE_CHECK,
[READING_MODULE] = MODULE_CHECK,
+ [READING_MODULE_COMPRESSED] = MODULE_CHECK,
[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
[READING_POLICY] = POLICY_CHECK
@@ -894,7 +902,7 @@ static int ima_post_read_file(struct file *file, char *buf, loff_t size,
func = read_idmap[read_id] ?: FILE_CHECK;
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, buf, size,
- MAY_READ, func);
+ MAY_READ, func, read_id);
}
/**
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 128fab897930..ae520e6bb1cf 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -241,7 +241,8 @@ static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
{.action = APPRAISE, .func = MODULE_CHECK,
- .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+ .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
+ IMA_CHECK_BLACKLIST},
{.action = APPRAISE, .func = FIRMWARE_CHECK,
.flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index d0323b81cd8f..1053a4acf589 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -118,6 +118,7 @@ int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
op = IPE_OP_FIRMWARE;
break;
case READING_MODULE:
+ case READING_MODULE_COMPRESSED:
op = IPE_OP_KERNEL_MODULE;
break;
case READING_KEXEC_INITRAMFS:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dfc22da42f30..c1ff69d5d76e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4275,7 +4275,7 @@ static int selinux_kernel_read_file(struct file *file,
{
int rc = 0;
- BUILD_BUG_ON_MSG(READING_MAX_ID > 7,
+ BUILD_BUG_ON_MSG(READING_MAX_ID > 8,
"New kernel_read_file_id introduced; update SELinux!");
switch (id) {
@@ -4283,6 +4283,7 @@ static int selinux_kernel_read_file(struct file *file,
rc = selinux_kernel_load_from_file(file, SYSTEM__FIRMWARE_LOAD);
break;
case READING_MODULE:
+ case READING_MODULE_COMPRESSED:
rc = selinux_kernel_load_from_file(file, SYSTEM__MODULE_LOAD);
break;
case READING_KEXEC_IMAGE:
@@ -4311,7 +4312,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
{
int rc = 0;
- BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7,
+ BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8,
"New kernel_load_data_id introduced; update SELinux!");
switch (id) {
base-commit: 6a23ae0a96a600d1d12557add110e0bb6e32730c
--
2.51.1
^ permalink raw reply related
* Re: [PATCH] KEYS: encrypted: Replace deprecated strcpy and improve get_derived_key
From: Jarkko Sakkinen @ 2025-11-19 2:50 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, linux-hardening, linux-integrity, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <20251113135831.98587-1-thorsten.blum@linux.dev>
On Thu, Nov 13, 2025 at 02:58:31PM +0100, Thorsten Blum wrote:
> Determine 'key_name' before allocating memory for 'derived_buf' to only
> allocate as many bytes as needed. Currently, we potentially allocate one
Who is "we"?
> more byte than necessary when 'key_name' is "ENC_KEY".
>
> strcpy() is deprecated and uses an additional strlen() internally; use
> memcpy() directly to copy 'key_name' since we already know its length
> and that it is guaranteed to be NUL-terminated.
>
> Also reuse 'key_name_len' when copying 'master_key' instead of calling
> strlen() again.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> security/keys/encrypted-keys/encrypted.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 15841466b5d4..b16a5b8b935b 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -12,6 +12,7 @@
> */
>
> #include <linux/uaccess.h>
> +#include <linux/minmax.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/slab.h>
> @@ -330,23 +331,18 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
> const u8 *master_key, size_t master_keylen)
> {
> u8 *derived_buf;
> - unsigned int derived_buf_len;
> -
> - derived_buf_len = strlen("AUTH_KEY") + 1 + master_keylen;
> - if (derived_buf_len < HASH_SIZE)
> - derived_buf_len = HASH_SIZE;
> + size_t derived_buf_len;
> + const char *key_name;
> + size_t key_name_len;
>
> + key_name = key_type ? "AUTH_KEY" : "ENC_KEY";
> + key_name_len = strlen(key_name) + 1;
> + derived_buf_len = max(key_name_len + master_keylen, HASH_SIZE);
> derived_buf = kzalloc(derived_buf_len, GFP_KERNEL);
> if (!derived_buf)
> return -ENOMEM;
> -
> - if (key_type)
> - strcpy(derived_buf, "AUTH_KEY");
> - else
> - strcpy(derived_buf, "ENC_KEY");
> -
> - memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
> - master_keylen);
> + memcpy(derived_buf, key_name, key_name_len);
> + memcpy(derived_buf + key_name_len, master_key, master_keylen);
> sha256(derived_buf, derived_buf_len, derived_key);
> kfree_sensitive(derived_buf);
> return 0;
> --
> 2.51.1
>
I don't see much value in this other than potentially causing merge
conflicts when backporting bug fixes.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: encrypted: Use pr_fmt()
From: Jarkko Sakkinen @ 2025-11-19 2:48 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, linux-integrity, keyrings, linux-security-module,
linux-kernel
In-Reply-To: <20251113123544.11287-2-thorsten.blum@linux.dev>
On Thu, Nov 13, 2025 at 01:35:44PM +0100, Thorsten Blum wrote:
> Use pr_fmt() to automatically prefix all pr_<level>() log messages with
This fails to describe what "use" means.
> "encrypted_key: " and remove all manually added prefixes.
>
> Reformat the code accordingly and avoid line breaks in log messages.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> security/keys/encrypted-keys/encrypted.c | 74 +++++++++++-------------
> security/keys/encrypted-keys/encrypted.h | 2 +-
> 2 files changed, 35 insertions(+), 41 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 513c09e2b01c..a8e8bf949b4b 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -11,6 +11,8 @@
> * See Documentation/security/keys/trusted-encrypted.rst
> */
>
Should have undef prepending.
> +#define pr_fmt(fmt) "encrypted_key: " fmt
> +
> #include <linux/uaccess.h>
> #include <linux/module.h>
> #include <linux/init.h>
> @@ -84,8 +86,7 @@ static int aes_get_sizes(void)
>
> tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
> if (IS_ERR(tfm)) {
> - pr_err("encrypted_key: failed to alloc_cipher (%ld)\n",
> - PTR_ERR(tfm));
> + pr_err("failed to alloc_cipher (%ld)\n", PTR_ERR(tfm));
> return PTR_ERR(tfm);
> }
> ivsize = crypto_skcipher_ivsize(tfm);
> @@ -106,15 +107,14 @@ static int valid_ecryptfs_desc(const char *ecryptfs_desc)
> int i;
>
> if (strlen(ecryptfs_desc) != KEY_ECRYPTFS_DESC_LEN) {
> - pr_err("encrypted_key: key description must be %d hexadecimal "
> - "characters long\n", KEY_ECRYPTFS_DESC_LEN);
> + pr_err("key description must be %d hexadecimal characters long\n",
> + KEY_ECRYPTFS_DESC_LEN);
> return -EINVAL;
> }
>
> for (i = 0; i < KEY_ECRYPTFS_DESC_LEN; i++) {
> if (!isxdigit(ecryptfs_desc[i])) {
> - pr_err("encrypted_key: key description must contain "
> - "only hexadecimal characters\n");
> + pr_err("key description must contain only hexadecimal characters\n");
> return -EINVAL;
> }
> }
> @@ -180,7 +180,7 @@ static int datablob_parse(char *datablob, const char **format,
>
> keyword = strsep(&datablob, " \t");
> if (!keyword) {
> - pr_info("encrypted_key: insufficient parameters specified\n");
> + pr_info("insufficient parameters specified\n");
> return ret;
> }
> key_cmd = match_token(keyword, key_tokens, args);
> @@ -188,7 +188,7 @@ static int datablob_parse(char *datablob, const char **format,
> /* Get optional format: default | ecryptfs */
> p = strsep(&datablob, " \t");
> if (!p) {
> - pr_err("encrypted_key: insufficient parameters specified\n");
> + pr_err("insufficient parameters specified\n");
> return ret;
> }
>
> @@ -206,20 +206,20 @@ static int datablob_parse(char *datablob, const char **format,
> }
>
> if (!*master_desc) {
> - pr_info("encrypted_key: master key parameter is missing\n");
> + pr_info("master key parameter is missing\n");
> goto out;
> }
>
> if (valid_master_desc(*master_desc, NULL) < 0) {
> - pr_info("encrypted_key: master key parameter \'%s\' "
> - "is invalid\n", *master_desc);
> + pr_info("master key parameter \'%s\' is invalid\n",
> + *master_desc);
> goto out;
> }
>
> if (decrypted_datalen) {
> *decrypted_datalen = strsep(&datablob, " \t");
> if (!*decrypted_datalen) {
> - pr_info("encrypted_key: keylen parameter is missing\n");
> + pr_info("keylen parameter is missing\n");
> goto out;
> }
> }
> @@ -227,8 +227,8 @@ static int datablob_parse(char *datablob, const char **format,
> switch (key_cmd) {
> case Opt_new:
> if (!decrypted_datalen) {
> - pr_info("encrypted_key: keyword \'%s\' not allowed "
> - "when called from .update method\n", keyword);
> + pr_info("keyword \'%s\' not allowed when called from .update method\n",
> + keyword);
> break;
> }
> *decrypted_data = strsep(&datablob, " \t");
> @@ -236,29 +236,27 @@ static int datablob_parse(char *datablob, const char **format,
> break;
> case Opt_load:
> if (!decrypted_datalen) {
> - pr_info("encrypted_key: keyword \'%s\' not allowed "
> - "when called from .update method\n", keyword);
> + pr_info("keyword \'%s\' not allowed when called from .update method\n",
> + keyword);
> break;
> }
> *hex_encoded_iv = strsep(&datablob, " \t");
> if (!*hex_encoded_iv) {
> - pr_info("encrypted_key: hex blob is missing\n");
> + pr_info("hex blob is missing\n");
> break;
> }
> ret = 0;
> break;
> case Opt_update:
> if (decrypted_datalen) {
> - pr_info("encrypted_key: keyword \'%s\' not allowed "
> - "when called from .instantiate method\n",
> + pr_info("keyword \'%s\' not allowed when called from .instantiate method\n",
> keyword);
> break;
> }
> ret = 0;
> break;
> case Opt_err:
> - pr_info("encrypted_key: keyword \'%s\' not recognized\n",
> - keyword);
> + pr_info("keyword \'%s\' not recognized\n", keyword);
> break;
> }
> out:
> @@ -362,22 +360,21 @@ static struct skcipher_request *init_skcipher_req(const u8 *key,
>
> tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
> if (IS_ERR(tfm)) {
> - pr_err("encrypted_key: failed to load %s transform (%ld)\n",
> - blkcipher_alg, PTR_ERR(tfm));
> + pr_err("failed to load %s transform (%ld)\n", blkcipher_alg,
> + PTR_ERR(tfm));
> return ERR_CAST(tfm);
> }
>
> ret = crypto_skcipher_setkey(tfm, key, key_len);
> if (ret < 0) {
> - pr_err("encrypted_key: failed to setkey (%d)\n", ret);
> + pr_err("failed to setkey (%d)\n", ret);
> crypto_free_skcipher(tfm);
> return ERR_PTR(ret);
> }
>
> req = skcipher_request_alloc(tfm, GFP_KERNEL);
> if (!req) {
> - pr_err("encrypted_key: failed to allocate request for %s\n",
> - blkcipher_alg);
> + pr_err("failed to allocate request for %s\n", blkcipher_alg);
> crypto_free_skcipher(tfm);
> return ERR_PTR(-ENOMEM);
> }
> @@ -406,13 +403,10 @@ static struct key *request_master_key(struct encrypted_key_payload *epayload,
>
> if (IS_ERR(mkey)) {
> int ret = PTR_ERR(mkey);
> -
> if (ret == -ENOTSUPP)
> - pr_info("encrypted_key: key %s not supported",
> - epayload->master_desc);
> + pr_info("key %s not supported", epayload->master_desc);
> else
> - pr_info("encrypted_key: key %s not found",
> - epayload->master_desc);
> + pr_info("key %s not found", epayload->master_desc);
> goto out;
> }
>
> @@ -457,7 +451,7 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload,
> skcipher_request_free(req);
> crypto_free_skcipher(tfm);
> if (ret < 0)
> - pr_err("encrypted_key: failed to encrypt (%d)\n", ret);
> + pr_err("failed to encrypt (%d)\n", ret);
> else
> dump_encrypted_data(epayload, encrypted_datalen);
> out:
> @@ -596,16 +590,16 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
>
> if (decrypted_data) {
> if (!user_decrypted_data) {
> - pr_err("encrypted key: instantiation of keys using provided decrypted data is disabled since CONFIG_USER_DECRYPTED_DATA is set to false\n");
> + pr_err("instantiation of keys using provided decrypted data is disabled since CONFIG_USER_DECRYPTED_DATA is set to false\n");
> return ERR_PTR(-EINVAL);
> }
> if (strlen(decrypted_data) != decrypted_datalen * 2) {
> - pr_err("encrypted key: decrypted data provided does not match decrypted data length provided\n");
> + pr_err("decrypted data provided does not match decrypted data length provided\n");
> return ERR_PTR(-EINVAL);
> }
> for (i = 0; i < strlen(decrypted_data); i++) {
> if (!isxdigit(decrypted_data[i])) {
> - pr_err("encrypted key: decrypted data provided must contain only hexadecimal characters\n");
> + pr_err("decrypted data provided must contain only hexadecimal characters\n");
> return ERR_PTR(-EINVAL);
> }
> }
> @@ -614,7 +608,7 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
> if (format) {
> if (!strcmp(format, key_format_ecryptfs)) {
> if (dlen != ECRYPTFS_MAX_KEY_BYTES) {
> - pr_err("encrypted_key: keylen for the ecryptfs format must be equal to %d bytes\n",
> + pr_err("keylen for the ecryptfs format must be equal to %d bytes\n",
> ECRYPTFS_MAX_KEY_BYTES);
> return ERR_PTR(-EINVAL);
> }
> @@ -622,8 +616,8 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
> payload_datalen = sizeof(struct ecryptfs_auth_tok);
> } else if (!strcmp(format, key_format_enc32)) {
> if (decrypted_datalen != KEY_ENC32_PAYLOAD_LEN) {
> - pr_err("encrypted_key: enc32 key payload incorrect length: %d\n",
> - decrypted_datalen);
> + pr_err("enc32 key payload incorrect length: %d\n",
> + decrypted_datalen);
> return ERR_PTR(-EINVAL);
> }
> }
> @@ -689,7 +683,7 @@ static int encrypted_key_decrypt(struct encrypted_key_payload *epayload,
>
> ret = datablob_hmac_verify(epayload, format, master_key, master_keylen);
> if (ret < 0) {
> - pr_err("encrypted_key: bad hmac (%d)\n", ret);
> + pr_err("bad hmac (%d)\n", ret);
> goto out;
> }
>
> @@ -699,7 +693,7 @@ static int encrypted_key_decrypt(struct encrypted_key_payload *epayload,
>
> ret = derived_key_decrypt(epayload, derived_key, sizeof derived_key);
> if (ret < 0)
> - pr_err("encrypted_key: failed to decrypt key (%d)\n", ret);
> + pr_err("failed to decrypt key (%d)\n", ret);
> out:
> up_read(&mkey->sem);
> key_put(mkey);
> diff --git a/security/keys/encrypted-keys/encrypted.h b/security/keys/encrypted-keys/encrypted.h
> index 1809995db452..7b05c66bafa6 100644
> --- a/security/keys/encrypted-keys/encrypted.h
> +++ b/security/keys/encrypted-keys/encrypted.h
> @@ -41,7 +41,7 @@ static inline void dump_hmac(const char *str, const u8 *digest,
> unsigned int hmac_size)
> {
> if (str)
> - pr_info("encrypted_key: %s", str);
> + pr_info("%s", str);
> print_hex_dump(KERN_ERR, "hmac: ", DUMP_PREFIX_NONE, 32, 1, digest,
> hmac_size, 0);
> }
> --
> 2.51.1
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: Remove the ad-hoc compilation flag CAAM_DEBUG
From: Jarkko Sakkinen @ 2025-11-19 2:25 UTC (permalink / raw)
To: Ye Bin
Cc: a.fatoum, kernel, James.Bottomley, zohar, dhowells, paul, jmorris,
serge, linux-integrity, keyrings, linux-security-module, yebin10
In-Reply-To: <20251028132254.841715-1-yebin@huaweicloud.com>
On Tue, Oct 28, 2025 at 09:22:54PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> Fix the broken design based on Jarkko Sakkinen's suggestions as follows:
>
> 1. Remove the ad-hoc compilation flag (i.e., CAAM_DEBUG).
> 2. Substitute pr_info calls with pr_debug calls.
>
> Closes: https://patchwork.kernel.org/project/linux-integrity/patch/20251024061153.61470-1-yebin@huaweicloud.com/
> Signed-off-by: Ye Bin <yebin10@huawei.com>
$ git am -3 20251028_yebin_keys_remove_the_ad_hoc_compilation_flag_caam_debug.mbx
Applying: KEYS: Remove the ad-hoc compilation flag CAAM_DEBUG
error: sha1 information is lacking or useless (security/keys/trusted-keys/trusted_caam.c).
error: could not build fake ancestor
Patch failed at 0001 KEYS: Remove the ad-hoc compilation flag CAAM_DEBUG
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Hmm.. Could you send me a new revision rebased on top of my tree?
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Mimi Zohar @ 2025-11-18 12:19 UTC (permalink / raw)
To: Coiby Xu
Cc: Paul Moore, linux-integrity, linux-security-module, Karel Srot,
James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <42qcfcxxlmwphctzvji76hy5tycfabiiv5u6zw6lgg2p3e2jwv@fp4g2y7ecf2y>
On Thu, 2025-11-13 at 12:06 +0800, Coiby Xu wrote:
> On Fri, Nov 07, 2025 at 02:28:13PM -0500, Mimi Zohar wrote:
> > On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
> > > On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
> > > > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
> > > > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> > > > [...]
> > > > >
> > > > > Hi Coiby,
> > > > >
> > > > > Based on the conversation with Paul, there is no reason to remove the existing
> > > > > security_kernel_post_read_file() call.
> > > > >
> > > > > The changes are similar to the 2nd link, but a bit different.
> > > > > - Define a single enumeration named READING_MODULE_COMPRESSED.
> > > > >
> > > > > - In module/main.c add a new security_kernel_post_read_file() call immediately
> > > > > after decompressing the kernel module. Like a previous version of this patch,
> > > > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
> > > > > based on MODULE_INIT_COMPRESSED_FILE.
> > > > >
> > > > > - In ima_post_read_file() defer verifying the signature when the enumeration is
> > > > > READING_MODULE_COMPRESSED. (No need for a new function ima_read_kernel_module.)
> > > >
> > > > Hi Mimi,
> > > >
> > > > Thanks for summarizing your conversation with Paul! I can confirm Paul's
> > > > approach works
> > > > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> > > >
> > > > While testing the patch today, I realized there is another
> > > > issue/challenge introduced by in-kernel module decompression. IMA
> > > > appraisal is to verify the digest of compressed kernel module but
> > > > currently the passed buffer is uncompressed module. When IMA uses
> > > > uncompressed module data to calculate the digest, xattr signature
> > > > verification will fail. If we always make IMA read the original kernel
> > > > module data again to calculate the digest, does it look like a
> > > > quick-and-dirty fix? If we can assume people won't load kernel module so
> > > > often, the performance impact is negligible. Otherwise we may have to
> > > > introduce a new LSM hook so IMA can access uncompressed and original
> > > > module data one time.
> > >
> > > ima_collect_measurement() stores the file hash info in the iint and uses that
> > > information to verify the signature as stored in the security xattr.
> > > Decompressing the kernel module shouldn't affect the xattr signature
> > > verification.
> >
> > In the case when the compressed kernel module hasn't previously been measured or
> > appraised before loading the kernel module, we need to "collect" the file data
> > hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.
> >
> > An alternative to your suggestion of re-reading the original kernel module data
> > to calculate the digest or defining a new hook, would be to define "collect" as
> > a new "action" and pass the kernel_read_file_id enumeration to
> > process_measurement(). IMA_COLLECTED already exists. Only IMA_COLLECT would
> > need to be defined. The new collect "action" should be limited to
> > func=MODULE_CHECK.
> >
> > The downside of this alternative is that it requires a new collect rule:
> > collect func=MODULE_CHECK mask=MAY_READ uid=0
> > appraise func=MODULE_CHECK appraise_type=imasig|modsig
As it turns out, the "collect" rule is unnecessary. On
READING_MODULE_COMPRESSED, process_measurement() should calculate the compressed
file hash. Extending the IMA measurement list and verifying the signature can
then be differed to READING_MODULE.
>
> Thank for suggesting an alternative! I've implemented the idea in
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_collect
>
> Note besides a new collect rule, another change is needed. Currently,
> process_measurement only accepts enum ima_hooks thus it can't tell if
> it's READING_MODULE_COMPRESSED so to only do collect action. So I
> create a fake MODULE_COMPRESSED_CHECK func.
Correct, either extending process_measurement() with the read_idmap enum or
defining the fake hook would work.
>
> And for the idea of re-reading the original kernel module data, it has
> been implemented in
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
>
> Both branches have applied your requested three changes including
> respecting the 80 char line limit. Additionally, I made a change to the
> IPE LSM because of the new READING_MODULE_COMPRESSED kernel_read_file_id
> enumerate.
>
> After comparing the two implementations, personally I prefer re-reading
> the original kernel module data because the change is smaller and it's
> more user-friendly. But if there are other reasons I don't know, I'll
> post the patches of the new collect action approach to the mailing list.
The "re-reading" option fails some of the tests. As the "collect" rule isn't
needed, let's stick with the first option.
--
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v2] KEYS: encrypted: Replace deprecated strcpy and improve get_derived_key
From: Thorsten Blum @ 2025-11-14 10:30 UTC (permalink / raw)
To: David Laight
Cc: Eric Biggers, Mimi Zohar, David Howells, Jarkko Sakkinen,
Paul Moore, James Morris, Serge E. Hallyn, linux-hardening,
linux-integrity, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251114093445.0ec74428@pumpkin>
Hi David,
On 14. Nov 2025, at 10:34, David Laight wrote:
> On Thu, 13 Nov 2025 22:55:45 +0100
> Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
>> strcpy() is deprecated; use the safer strscpy() and use its return
>> value, the number of bytes copied, instead of calling strlen() on the
>> destination buffer again. String truncation can be ignored since
>> 'derived_buf' is guaranteed to be large enough.
>>
>> Link: https://github.com/KSPP/linux/issues/88
>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>> ---
>> [...]
>
> I'm not sure this is an improvement, but has this code ever been correct?
> The buffer passed to sha256 is either:
> "AUTH_KEY"'\0'master_key
> or
> "ENC_KEY"'\0'master_key
> For short master_key the buffer is HASH_SIZE bytes and padded with zeros (ok).
> However for long master_key the length is calculated using "AUTH_KEY" so
> there is an additional trailing '\0' in the "ENC_KEY" case.
I removed the trailing '\0' in v1, but since Eric pointed out that it
changes the sha256 hash, I reverted it in v2.
Thanks,
Thorsten
^ permalink raw reply
* Re: [PATCH v2] KEYS: encrypted: Replace deprecated strcpy and improve get_derived_key
From: David Laight @ 2025-11-14 9:34 UTC (permalink / raw)
To: Thorsten Blum
Cc: Eric Biggers, Mimi Zohar, David Howells, Jarkko Sakkinen,
Paul Moore, James Morris, Serge E. Hallyn, linux-hardening,
linux-integrity, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251113215546.136145-1-thorsten.blum@linux.dev>
On Thu, 13 Nov 2025 22:55:45 +0100
Thorsten Blum <thorsten.blum@linux.dev> wrote:
> strcpy() is deprecated; use the safer strscpy() and use its return
> value, the number of bytes copied, instead of calling strlen() on the
> destination buffer again. String truncation can be ignored since
> 'derived_buf' is guaranteed to be large enough.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Changes in v2:
> - Revert some changes to include the trailing '\0' even if key_type == 0
> since this would change the bytes passed to sha256() (Eric Biggers)
> - Use strscpy() and its return value instead
> - Link to v1: https://lore.kernel.org/lkml/20251113135831.98587-1-thorsten.blum@linux.dev/
> ---
> security/keys/encrypted-keys/encrypted.c | 21 ++++++++-------------
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 15841466b5d4..94408909f1dd 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -12,6 +12,7 @@
> */
>
> #include <linux/uaccess.h>
> +#include <linux/minmax.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/slab.h>
> @@ -330,23 +331,17 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
> const u8 *master_key, size_t master_keylen)
> {
> u8 *derived_buf;
> - unsigned int derived_buf_len;
> -
> - derived_buf_len = strlen("AUTH_KEY") + 1 + master_keylen;
> - if (derived_buf_len < HASH_SIZE)
> - derived_buf_len = HASH_SIZE;
> + size_t derived_buf_len;
> + const char *key_name;
> + ssize_t len;
>
> + derived_buf_len = max(strlen("AUTH_KEY") + 1 + master_keylen, HASH_SIZE);
> derived_buf = kzalloc(derived_buf_len, GFP_KERNEL);
> if (!derived_buf)
> return -ENOMEM;
> -
> - if (key_type)
> - strcpy(derived_buf, "AUTH_KEY");
> - else
> - strcpy(derived_buf, "ENC_KEY");
> -
> - memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
> - master_keylen);
> + key_name = key_type ? "AUTH_KEY" : "ENC_KEY";
> + len = strscpy(derived_buf, key_name, derived_buf_len);
> + memcpy(derived_buf + len + 1, master_key, master_keylen);
> sha256(derived_buf, derived_buf_len, derived_key);
I'm not sure this is an improvement, but has this code ever been correct?
The buffer passed to sha256 is either:
"AUTH_KEY"'\0'master_key
or
"ENC_KEY"'\0'master_key
For short master_key the buffer is HASH_SIZE bytes and padded with zeros (ok).
However for long master_key the length is calculated using "AUTH_KEY" so
there is an additional trailing '\0' in the "ENC_KEY" case.
David
> kfree_sensitive(derived_buf);
> return 0;
^ permalink raw reply
* [PATCH v2] KEYS: encrypted: Replace deprecated strcpy and improve get_derived_key
From: Thorsten Blum @ 2025-11-13 21:55 UTC (permalink / raw)
To: Eric Biggers, Mimi Zohar, David Howells, Jarkko Sakkinen,
Paul Moore, James Morris, Serge E. Hallyn
Cc: linux-hardening, Thorsten Blum, linux-integrity, keyrings,
linux-security-module, linux-kernel
strcpy() is deprecated; use the safer strscpy() and use its return
value, the number of bytes copied, instead of calling strlen() on the
destination buffer again. String truncation can be ignored since
'derived_buf' is guaranteed to be large enough.
Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
Changes in v2:
- Revert some changes to include the trailing '\0' even if key_type == 0
since this would change the bytes passed to sha256() (Eric Biggers)
- Use strscpy() and its return value instead
- Link to v1: https://lore.kernel.org/lkml/20251113135831.98587-1-thorsten.blum@linux.dev/
---
security/keys/encrypted-keys/encrypted.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 15841466b5d4..94408909f1dd 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -12,6 +12,7 @@
*/
#include <linux/uaccess.h>
+#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/slab.h>
@@ -330,23 +331,17 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
const u8 *master_key, size_t master_keylen)
{
u8 *derived_buf;
- unsigned int derived_buf_len;
-
- derived_buf_len = strlen("AUTH_KEY") + 1 + master_keylen;
- if (derived_buf_len < HASH_SIZE)
- derived_buf_len = HASH_SIZE;
+ size_t derived_buf_len;
+ const char *key_name;
+ ssize_t len;
+ derived_buf_len = max(strlen("AUTH_KEY") + 1 + master_keylen, HASH_SIZE);
derived_buf = kzalloc(derived_buf_len, GFP_KERNEL);
if (!derived_buf)
return -ENOMEM;
-
- if (key_type)
- strcpy(derived_buf, "AUTH_KEY");
- else
- strcpy(derived_buf, "ENC_KEY");
-
- memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
- master_keylen);
+ key_name = key_type ? "AUTH_KEY" : "ENC_KEY";
+ len = strscpy(derived_buf, key_name, derived_buf_len);
+ memcpy(derived_buf + len + 1, master_key, master_keylen);
sha256(derived_buf, derived_buf_len, derived_key);
kfree_sensitive(derived_buf);
return 0;
--
2.51.1
^ permalink raw reply related
* Re: [PATCH] KEYS: encrypted: Replace deprecated strcpy and improve get_derived_key
From: Thorsten Blum @ 2025-11-13 20:23 UTC (permalink / raw)
To: Eric Biggers
Cc: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, linux-integrity,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251113172913.GC1792@sol>
On 13. Nov 2025, at 18:29, Eric Biggers wrote:
> [...]
>
> This changes the resulting derived key when key_type == 0 &&
> master_keylen >= 24, because different bytes are passed to sha256().
I see, thanks. I'll submit a v2.
^ 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