On Tue, May 26, 2026 at 07:20:31PM +0100, Breno Leitao wrote: > Hello Jarkko, > > On Mon, May 25, 2026 at 02:24:38AM +0000, Jarkko Sakkinen wrote: > > On Sun, May 24, 2026 at 10:01:17AM -0400, Breno Leitao wrote: > > > tpm_buf_append() guards against overflow of the underlying buffer by > > > comparing the running length against PAGE_SIZE. Every other site in the > > > TPM core uses TPM_BUFSIZE (4096) as the protocol-level cap on TPM > > > command and response sizes. > > > > > > On 4K-page kernels PAGE_SIZE == TPM_BUFSIZE, so the two caps coincide > > > and the inconsistency is invisible. On kernels with a larger base page > > > size, e.g. CONFIG_ARM64_64K_PAGES=y or 16K pages, PAGE_SIZE exceeds > > > TPM_BUFSIZE. > > > > > > This is a latent bug rather than user-visible bug, given most of the > > > cases PAGE_SIZE = 4096. The mismatch is still worth fixing because > > > future callers (e.g. the proposed TPM_BUFSIZE increase to 8 KiB, and the > > > Secure Launch tpm_buf rework) rely on the overflow flag being > > > authoritative. > > > > > > Use TPM_BUFSIZE instead of PAGE_SIZE so the append-time check > > > matches the transmit-time cap on every page size. > > > > > > Signed-off-by: Breno Leitao <[email protected]> > > > Fixes: a74f8b36352e ("tpm: introduce tpm_buf") > > > --- > > > > There is no bug w/o a sympton of some sort. Not sure what the problem is > > here. > > Sorry, there is no current problem in here, but there is inconsistency. > > There isn't a real problem today, just an inconsistency and what I > called a latent bug, let me justify myself. > > Everywhere else in the TPM core uses TPM_BUFSIZE as the protocol cap, > but this particular site uses PAGE_SIZE instead. Since PAGE_SIZE >= > TPM_BUFSIZE, it doesn't cause any issue at the moment. > > That said, I still think the change is worthwhile for two reasons: > > 1. Consistency with the rest of the TPM core. > 2. Decoupling TPM_BUFSIZE from PAGE_SIZE, so that if TPM_BUFSIZE ever > grows beyond PAGE_SIZE[1], this code won't silently break. That's what I > was referring to as a "latent bug" — though admittedly that phrasing > was probably too strong. > > Link: https://lore.kernel.org/all/[email protected]/ > [1]
IMHO, if that is required for buffer chunking patch set, Arun should pick it up a precursory patch but as far as mainline is concerned right now, it is not broken. I'm somewhat resistant to any pure cleanup patches given how things are right now in the world, *unless* they are part of some relevant patch set as precursory patches. > > Thanks for the review, > --breno > > > BR, Jarkko
