On Fri, May 29, 2026 at 10:08:52AM -0400, James Bottomley wrote: > On Tue, 2026-05-26 at 10:53 +0300, Jarkko Sakkinen wrote: > > On Mon, May 25, 2026 at 01:50:51PM -0400, James Bottomley wrote: > > > On Fri, 2026-05-22 at 04:35 +0300, Jarkko Sakkinen wrote: > > > > Decouple kzalloc from buffer creation, so that a managed > > > > allocation > > > > can be > > > > used: > > > > > > > > struct tpm_buf *buf __free(kfree) buf = > > > > kzalloc(TPM_BUFSIZE, > > > > GFP_KERNEL); > > > > if (!buf) > > > > return -ENOMEM; > > > > > > > > tpm_buf_init(buf, TPM_BUFSIZE); > > > > > > > > Alternatively, stack allocations are also possible: > > > > > > > > u8 buf_data[512]; > > > > struct tpm_buf *buf = (struct tpm_buf *)buf_data; > > > > tpm_buf_init(buf, sizeof(buf_data)); > > > > > > This isn't really a good idea from a security point of view. > > > Remember the buffer has to be big enough for both the sent and the > > > received data. Today we simply set TPM_BUFSIZE to the maximum > > > amount a TPM requires and all the send and receives just work. If > > > we let callers set this size, we're asking for them to get it wrong > > > (or at least forget about the receive part) and for us to get a DMA > > > overrun from the TPM ... which might be potentially exploitable > > > depending on how it occurs (think of an unseal of user chosen data > > > overrunning). > > > > It's one patch so you're free to remark the call sites where this > > happens. This is not a majorn concern at all. > > Nearly twenty years ago, when the kernel was a lot smaller, a then > kernel luminary called Rusty Russell realized we needed to pay much > more attention to how we design APIs inside the kernel if we wanted it > to grow successfully. He published his initial thoughts and gave talks > at both the kernel summit and OLS on it: > > https://ozlabs.org/~rusty/index.cgi/tech/2008-03-18.html > > The key point that's always stuck with me is "hard to misuse beats easy > to use". Later he came up with a rating scale (now known as the Rusty > API classification): > > https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html > > and for chuckles and grins on April fools day he came up with a > negative rating ridiculing some of our dafter API choices: > > https://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html > > The point for this patch set is that the sizing of the original tpm_buf > interface scores 10/10 on the Rusty scale (it's impossible to get > wrong). Simply threading size through the whole API, as this patch > does, may look like the right answer, but it causes a massive reduction > in API score. In fact, since the buffer has to be sized not only > according to what goes in, but also what gets returned and this is > nowhere mentioned in the new documentation it scores -3 (read the > documentation and you can still get it wrong). Now by mentioning the > sizing problems in the doc, you can probably get it up to +3 (read the > documentation and you'll get it right) but my question was not if you > got it wrong somewhere in the patch but whether we couldn't do a whole > lot better in terms of API score by designing a better API. > > A key point about the 185 version of the TPM spec is that it's really > only a few commands that need larger buffers (the Post Quantum ML-KEM > keys) which doesn't apply to most of the in-kernel TPM callsites. > Since tpm_buf_init takes the ordinal, we can actually tell at runtime > (or compile time if the ordinal is a constant) if the command would > need a larger buffer. We can also tell from the TPM properties whether > the TPM itself can take a larger buffer, so for every current TPM we > could retain the original score 10/10 API and warn at runtime if there > might be a problem. Then the larger keys seem to fit into 8k, so we > could still retain most of the original API properties of being > difficult to misuse simply by having an 8k size flag (which we could > ignore if the TPM doesn't support it) and warn at runtime if > tpm_buf_init sends an ordinal which might need a larger buffer. At > worst we should be able to get to an API which scores 5/10 (do it right > or it will break at runtime). > > Regards, > > James
This patch has pre-existed long before any of this post-quantum stuff, and there are good reasons so to have buffers managed given e.g., complexity of tpm2-sessions code. It prevents any major risk for memory leaks. Trenchboot extends the use buffers to early boot and we want a robust structure. I'm not going to spend my time reading about philosophical aspects of API design. There are quantitative reasons to decrease the risk of memory leaks. BR, Jarkko
