Re: [PATCH AUTOSEL 7.0-6.18] ima: Define and use a digest_size field in the ima_algo_desc structure
Mimi Zohar <[email protected]>
| Newsgroups | org.kernel.vger.linux-integrity,dev.linux.lists.patches,org.kernel.vger.linux-kernel,org.kernel.vger.linux-security-module,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 2026-04-20 at 09:18 -0400, Sasha Levin wrote: > From: Roberto Sassu <[email protected]> > > [ Upstream commit a74d7197ebe5b1b8028911d47e78c119d9aaf193 ] > > Add the digest_size field to the ima_algo_desc structure to determine the > digest size from the correct source. > > If the hash algorithm is among allocated PCR banks, take the value from the > TPM bank info (equal to the value from the crypto subsystem if the TPM > algorithm is supported by it; otherwise, not exceding the size of the > digest buffer in the tpm_digest structure, used by IMA). > > If the hash algorithm is SHA1, use the predefined value. Lastly, if the > hash algorithm is the default one but not among the PCR banks, take the > digest size from the crypto subsystem (the default hash algorithm is > checked when parsing the ima_hash= command line option). > > Finally, use the new information to correctly show the template digest in > ima_measurements_show() and ima_ascii_measurements_show(). > > Link: https://github.com/linux-integrity/linux/issues/14 > Signed-off-by: Roberto Sassu <[email protected]> > Signed-off-by: Mimi Zohar <[email protected]> > Signed-off-by: Sasha Levin <[email protected]> > --- > > LLM Generated explanations, may be completely bogus: > > Now I have complete information. Let me provide the final analysis. > > ## Phase 1: Commit Message Forensics > > ### Step 1.1: Subject Line > **Record:** Subsystem: `ima:` (Integrity Measurement Architecture, a > security subsystem). Action verb: "Define and use" - sounds like > enhancement, but functionally equivalent to "fix digest size source". > One-line summary: Add a digest_size field to ima_algo_desc for accurate > template digest output. > > ### Step 1.2: Tags > **Record:** > - `Link: https://github.com/linux-integrity/linux/issues/14` (bug > tracker for the linux-integrity subsystem) > - `Signed-off-by: Roberto Sassu <[email protected]>` (author) > - `Signed-off-by: Mimi Zohar <[email protected]>` (IMA maintainer - > applied the patch) > - No `Fixes:`, no `Cc: stable`, no `Reported-by:` tag. > > ### Step 1.3: Commit Body Analysis > **Record:** The message describes three cases for digest size: > 1. TPM bank-allocated algos: take from TPM bank info (which may differ > from crypto subsystem size) > 2. SHA1: use predefined value > 3. Default hash algo not among banks: use crypto subsystem's size > > Author's framing is additive/improvement ("Add the ... field"), but the > Link points to GitHub issue #14 titled "Out of bound when creating per- > algo measurement list interfaces" - describing a KASAN out-of-bounds > read when TPM has unsupported algorithms (e.g., SHA3_256). > > ### Step 1.4: Hidden Bug Fix Detection > **Record:** This IS a hidden bug fix. The old code used > `hash_digest_size[algo]` where `algo` can be `HASH_ALGO__LAST` (for > unsupported TPM algos). Since `hash_digest_size` is declared > `[HASH_ALGO__LAST]`, that access is out-of-bounds. The new code uses the > TPM bank's `digest_size` (always valid) or a known constant. > > ## Phase 2: Diff Analysis > > ### Step 2.1: Inventory > **Record:** 3 files changed: > - `security/integrity/ima/ima.h` (+1) > - `security/integrity/ima/ima_crypto.c` (+6) > - `security/integrity/ima/ima_fs.c` (+6/-12) > > Total: 13 insertions, 12 deletions. Scope: single-subsystem surgical > change. > > ### Step 2.2-2.3: Code Flow and Bug Mechanism > **Record:** Bug category: **Out-of-bounds read** (KASAN-detectable). > > Before fix: `ima_putc(m, e->digests[algo_idx].digest, > hash_digest_size[algo])` where `algo = ima_algo_array[algo_idx].algo`. > If the TPM has an algorithm not supported by the kernel's crypto > subsystem (e.g., SHA3_256 which was not yet in `tpm2_hash_map`), `algo > == HASH_ALGO__LAST`, and `hash_digest_size[HASH_ALGO__LAST]` is an OOB > read of the `[HASH_ALGO__LAST]`-sized array. > > After fix: `ima_putc(m, e->digests[algo_idx].digest, > ima_algo_array[algo_idx].digest_size)`. `digest_size` is populated from > `tpm_bank_info.digest_size` (which is filled via `tpm2_pcr_read` for > unknown algos, or `hash_digest_size[crypto_algo]` for known ones), > `SHA1_DIGEST_SIZE`, or `hash_digest_size[ima_hash_algo]` - all safe > indexes. > > ### Step 2.4: Fix Quality > **Record:** Fix is obviously correct, minimal, and well-contained. The > new `digest_size` field is populated once during init (`__init`), then > only read later. Regression risk is low - the change is semantically > equivalent to the old code when the TPM algo is supported, and correct > when it isn't. > > ## Phase 3: Git History > > ### Step 3.1-3.2: Blame and Fixes target > **Record:** The buggy line `ima_putc(m, e->digests[algo_idx].digest, > hash_digest_size[algo])` was introduced by commit `9fa8e76250082a` > ("ima: add crypto agility support for template-hash algorithm", by > Enrico Bravi, merged in v6.10). This code is present in every stable > tree from v6.10 onwards (so 6.12.y and newer). > > ### Step 3.3: Related Commits > **Record:** Companion commit `d7bd8cf0b348d` ("ima_fs: Correctly create > securityfs files for unsupported hash algos") was applied 12 days after > this one, sharing the same `Link:` to issue #14. That commit has an > explicit `Fixes: 9fa8e7625008` tag and includes a KASAN dump showing > `create_securityfs_measurement_lists+0x396/0x440` OOB in > `hash_algo_name`. The two commits address two sides of the same bug: > `a74d7197ebe5b` fixes OOB in `hash_digest_size[algo]` (runtime, at file > read), `d7bd8cf0b348d` fixes OOB in `hash_algo_name[algo]` (boot, at > file creation). > > ### Step 3.4: Author Context > **Record:** Roberto Sassu is a long-term IMA contributor. Mimi Zohar is > the IMA subsystem maintainer who merged the patch. > > ### Step 3.5: Dependencies > **Record:** The fix depends on `tpm_bank_info.digest_size` being > available, which has existed since commit `879b589210a9a` (2019). No new > dependencies. Applies to any stable tree containing `9fa8e76250082a` > (v6.10+). > > ## Phase 4: Mailing List Research > > ### Step 4.1-4.4: Patch Discussion > **Record:** > - `b4 dig -c a74d7197ebe5b` found single v1 submission at `https://lore. > kernel.org/all/[email protected]/` > - Discussion thread contains 3 messages from Mimi Zohar (maintainer) and > Roberto Sassu. Mimi requested title rename and asked for a note about > the design change (from crypto subsystem's digest size to TPM's). > - No explicit stable nomination, no mention of KASAN in discussion > thread itself. > - GitHub issue #14 (referenced via Link: tag) explicitly documents the > OOB bug this is fixing: "If a TPM algorithm is not supported the PCR > bank info is initialized with HASH_ALGO__LAST, which passed to > hash_algo_name[] causes an out of bound." > - No v2, applied as single revision. > > ### Step 4.5: Stable Discussion > **Record:** No prior stable mailing list discussion found for this > specific commit. > > ## Phase 5: Code Semantic Analysis > > ### Step 5.1-5.4: Call Paths > **Record:** `ima_measurements_show()` is called when a userspace process > reads `/sys/kernel/security/ima/binary_runtime_measurements*`. > `ima_ascii_measurements_show()` similarly for ASCII files. These files > are readable by root. The path is reachable from userspace via a simple > `read()` syscall against the securityfs files. `ima_init_crypto()` is > called once at boot via initcall. > > ### Step 5.5: Similar Patterns > **Record:** The sister commit `d7bd8cf0b348d` addresses the same pattern > (`hash_algo_name[algo]` with `algo == HASH_ALGO__LAST`) in the file- > creation path. > > ## Phase 6: Stable Tree Cross-Reference > > ### Step 6.1-6.3: Applicability > **Record:** > - Buggy code exists in 6.12.y (verified via `git blame stable- > push/linux-6.12.y` showing line 184 originated from 9fa8e76250082a). > Also in 6.15, 6.17, 6.18, 6.19, 7.0. > - 6.1.y and 6.6.y don't have the crypto agility code > (`hash_digest_size[algo]` usage) - the fix is NOT applicable/needed > there. 6.6.y uses `TPM_DIGEST_SIZE`. > - Backport difficulty to 6.12.y: minor rework needed (ima_algo_array > allocation uses `kcalloc` instead of `kzalloc_objs` in newer tree, but > that's not affected by this patch - the field addition and assignments > apply straightforwardly). > - Neither this commit nor `d7bd8cf0b348d` is yet in 6.12.y (verified via > `git log stable-push/linux-6.12.y`). > > ## Phase 7: Subsystem Context > > ### Step 7.1-7.2 > **Record:** Subsystem: IMA (security/integrity/ima/). Criticality: > IMPORTANT - used for measured boot/attestation on enterprise/embedded > systems. Activity: active subsystem with regular fixes. The code is only > reachable when CONFIG_IMA is enabled AND a TPM is present, further > narrowing impact to TPM-equipped systems. > > ## Phase 8: Impact and Risk > > ### Step 8.1: Affected Users > **Record:** Users with IMA enabled + TPM 2.0 chip that exposes an > algorithm not in the kernel's `tpm2_hash_map`. The KASAN dump in > d7bd8cf0b348d shows this was hit on real hardware (SHA3_256-capable > TPM). > > ### Step 8.2: Trigger > **Record:** The secondary OOB fixed by THIS commit > (hash_digest_size[HASH_ALGO__LAST]) triggers when: > 1. A TPM exposes an unsupported algorithm (e.g., SHA3_256) > 2. A user (root) reads the unsupported-algo measurements file > Root privilege required - not a remote attack vector, but reproducible > with specific hardware. The primary OOB (in create_securityfs) hits > every boot with such TPMs, which is what the KASAN report showed. > > ### Step 8.3: Failure Mode > **Record:** Out-of-bounds read from kernel memory. Under KASAN: reported > as BUG. Without KASAN: may return garbage digest size, which could cause > excessive data to be read from `e->digests[algo_idx].digest` (a fixed- > size `[TPM2_MAX_DIGEST_SIZE]` buffer) or leak a few bytes past the > `hash_digest_size` array. Severity: **MEDIUM-HIGH** (OOB read is KASAN- > reportable security-relevant behavior, not a guaranteed crash without > KASAN but can leak info or cause incorrect behavior). > > ### Step 8.4: Risk vs Benefit > **Record:** > - **Benefit: MEDIUM** - Fixes one half of a KASAN-reportable OOB read > with real-hardware reproducer. > - **Risk: LOW** - 13-line structural change, all within the IMA init > path + two show functions, no change of external behavior for > supported TPM algos. > - **Ratio: Favorable for backport** - but only valuable when paired with > d7bd8cf0b348d (the boot-time crash fix). > > ## Phase 9: Final Synthesis > > ### Step 9.1-9.4: Evidence Summary > > **For backport:** > - Small, contained (13/12 lines, 3 files) > - Fixes real OOB read (hash_digest_size[HASH_ALGO__LAST]) > - Obviously correct - reviewed by IMA maintainer (Mimi Zohar) > - Low regression risk > - Reachable from userspace (root reads securityfs file) > - Bug has real-hardware reproducer (SHA3_256 TPMs) > - Companion commit d7bd8cf0b348d has `Fixes:` tag and will be auto- > selected; backporting only d7bd8cf0b348d leaves a latent OOB in the > read path > > **Against backport:** > - No explicit `Fixes:` tag, no `Cc: stable` > - Framed as enhancement, not bug fix > - Alone doesn't fix the primary crash (boot-time OOB in > `create_securityfs_measurement_lists`) - that's d7bd8cf0b348d > - Design change (TPM's size vs crypto's size) noted by maintainer in > review > > **Exception Category:** Not a device ID/quirk/DT/build/doc. Standard bug > fix evaluation. > > **Stable Rules Check:** > 1. Obviously correct: YES (reviewed, simple struct field addition + safe > sources) > 2. Fixes real bug: YES (OOB read) > 3. Important issue: MEDIUM (KASAN-reportable OOB with real hardware) > 4. Small and contained: YES (13/12 lines) > 5. No new features/APIs: YES (internal struct field, not user-visible) > 6. Applies to stable: YES (6.12.y+ with minor/no conflicts) > > ## Verification > > - [Phase 1] Parsed tags: no Fixes:, no Cc: stable, has `Link: > github.com/linux-integrity/linux/issues/14` > - [Phase 1] Hidden bug fix confirmed: commit uses additive language but > eliminates OOB read > - [Phase 2] Diff analysis: `git show a74d7197ebe5b` confirmed +13/-12 > across 3 files; key change is replacing `hash_digest_size[algo]` with > `ima_algo_array[algo_idx].digest_size` > - [Phase 3] `git blame stable-push/linux-6.12.y > security/integrity/ima/ima_fs.c`: buggy line 184 was introduced by > 9fa8e76250082a (v6.10) > - [Phase 3] `git show 9fa8e76250082`: confirmed merged in v6.10 > (2024-04-12) > - [Phase 3] Found companion commit d7bd8cf0b348d sharing the same GitHub > issue link > - [Phase 3] Verified `include/linux/tpm.h` defines > `tpm_bank_info.digest_size` (dependency satisfied in all target stable > trees since 2019 via commit 879b589210a9a) > - [Phase 4] `b4 dig -c a74d7197ebe5b`: found single v1 submission > - [Phase 4] `b4 dig -c a74d7197ebe5b -a`: only v1, no earlier revisions > - [Phase 4] `b4 dig -c a74d7197ebe5b -w`: maintainer Mimi Zohar was a > recipient > - [Phase 4] Read `/tmp/thread.mbx`: confirmed maintainer review, no > stable nomination, no KASAN mention in thread > - [Phase 4] WebFetch github issue #14: confirmed OOB bug rationale > - [Phase 4] WebFetch marc.info original patch: confirmed patch content > matches commit > - [Phase 5] Verified `ima_measurements_show` is reachable via read() on > `/sys/kernel/security/ima/*_runtime_measurements*` > - [Phase 6] `git show stable-push/linux-6.12.y:.../ima_fs.c`: confirmed > buggy line present at 184 > - [Phase 6] `git show stable-push/linux-6.6.y:.../ima_fs.c`: uses > `TPM_DIGEST_SIZE`, no crypto agility, bug doesn't exist there > - [Phase 6] `git show stable-push/linux-6.1.y`: ima_fs.c does not have > the buggy code > - [Phase 6] `git log stable-push/linux-6.12.y`: neither this commit nor > d7bd8cf0b348d is yet in 6.12.y > - [Phase 8] Failure mode: OOB read of > `hash_digest_size[HASH_ALGO__LAST]` - triggered by reading per-algo > measurement file on TPM with unsupported algo > - UNVERIFIED: Whether d7bd8cf0b348d has been auto-selected by Fixes:-tag > tooling; whether user-visible symptoms (without KASAN) would be > observable in the field. > > ## Conclusion > > This commit is a small, surgical fix for a real out-of-bounds read bug > that affects stable trees from v6.12 onwards. While the commit message > frames it as an "add a field" improvement rather than a bug fix, the > code change explicitly eliminates an OOB read in the user-reachable path > `ima_measurements_show()` / `ima_ascii_measurements_show()` when a TPM > exposes an algorithm not supported by the kernel crypto subsystem. The > companion commit `d7bd8cf0b348d` addresses the parallel boot-time OOB > (in `create_securityfs_measurement_lists`) and has a `Fixes:` tag, so it > will likely be auto-selected. If d7bd8cf0b348d reaches stable (as it > should), this commit is needed to plug the remaining runtime OOB on the > same hardware. > > **YES** Thanks, Sasha! The AI's conclusion is correct. This patch is needed when backporting commit d7bd8cf0b348 ("ima_fs: Correctly create securityfs files for unsupported hash algos") is correct. Mimi