Toot

Written by Jarkko Sakkinen on 2024-10-21 at 16:07

@gromit i might actually might have hunch for this but have been unmotivated to move forward before these patches are merged. One thing at a time IMHO, especially with performance fixes.

NOTE: this is a hyphothesis.

tpm2_get_random() needs to be reconsidered with encryption as hwrng calls it often enough and there's now more overhead.

It pulls entropy in small chunks subtracting the length in the look like you would use read() syscall from user space. This is not right. This leads to small and variable size exchanges and yeah generally that equates to an inefficient implemenation with bad latency properties.

A better implementation would build on two concepts:

1. Have a pool "struct tpm2_random_pool" per caller with a fixed-size buffer in page granularity. E.g. hwrng end of TPM driver would have an instance. This would be exta argument for tpm2_get_random().

2. Have low and high watermarks (or thresholds) for triggering the pull from the chip so that it does not start to throttle.

Normally when the call is made entropy is served from the pool. When going below low water mark a thread (e.g. kthread, workqueue) is woken and less entropy is returned to the caller. Thread then fills the pool with fixes-size requests up until it goes above the high watermark.

I've posted my first patch set early on when the bug was recognized and also reporter confirmed that my fixes do in fact sort out the issue. There's been already some push to also sort some IMA issue but it is chaos if you start to include tons of random fixes to a single patch only because nobody pays any attention.

So I will work on that random pool patch after and also promise not to do anything before my first patch set has landed. With those ideas anyone who wants send me e.g. an RFC patch that I'm happy to give feedback.

=> More informations about this toot | View the thread | More toots from jarkko@social.kernel.org

Mentions

=> View gromit@chaos.social profile

Tags