Page MenuHomeFreeBSD

tpm: fix multi-threaded access with per-open state
Needs ReviewPublic

Authored by chs on Sep 1 2025, 11:59 PM.
Tags
None
Referenced Files
F144958875: D52328.id171895.diff
Sat, Feb 14, 12:23 PM
F144954812: D52328.id171895.diff
Sat, Feb 14, 11:24 AM
Unknown Object (File)
Mon, Jan 26, 9:07 AM
Unknown Object (File)
Sat, Jan 24, 6:39 AM
Unknown Object (File)
Jan 14 2026, 11:30 AM
Unknown Object (File)
Jan 12 2026, 12:42 PM
Unknown Object (File)
Dec 12 2025, 4:50 AM
Unknown Object (File)
Nov 26 2025, 8:43 PM
Subscribers

Details

Reviewers
mw
imp
olivier
Summary

tpm: fix multi-threaded access with per-open state

The TPM driver currently has a single buffer per instance to hold the
result of a command, and does not allow subsequent commands to be sent
until the current result is read by the same OS thread that sent the
command, with a timeout to throw away the result after a while if the
result is not read in a timely fashion. This has a couple problems:

  • The timeout code has a bug which causes all subsequent commands to hang forever if a different OS thread tries to read the result before the OS thread which sent the command, and the OS thread which sent the command never tries to read the result.
  • Even if the first problem is fixed, applications expect to be able to read the result from a different OS thread than the OS thread which sent the command. The particular case that we saw was a go application where the go runtime scheduled the goroutine which read the result to a different OS thread from one where the goroutine that sent the command ran, and there's no way to force these to always run on the same OS thread.

Fix all of this by replacing the global result buffer with a per-open
result buffer via devfs_set_cdevpriv(), so that we no longer need to
block subsequent commands until the results of a previous command are
retrieved or care about which OS thread is reading the result of a
comamnd.

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 70671
Build 67554: arc lint + arc unit

Event Timeline

chs requested review of this revision.Sep 1 2025, 11:59 PM
This revision is now accepted and ready to land.Sep 2 2025, 12:04 AM
olivier added a subscriber: olivier.

we are using this patch since months now, and it works great (because it solve the problem mentionned)

chs retitled this revision from tpm: prevent hang when a thread that has not written tries to read to tpm: fix multi-threaded access with per-open state.Fri, Feb 13, 7:25 PM

replace the old fix with all new better fix

This revision now requires review to proceed.Fri, Feb 13, 11:13 PM
sys/dev/tpm/tpm20.c
272

This looks wonky. You add sizeof(uint16_t) below, but you only look at 1 byte here. And the second byte after the header at that. I know you just copied this code from before, but it's weird enough that it deserves a comment, I'd think. I'd think you'd want to deref the uint16_t, and use be16toh like you do below for a 32-bit count...