Differential D19713

tpm: Prevent session hijack.
ClosedPublic
Actions

Authored by kd on Mar 26 2019, 12:00 PM.

Details

Reviewers

mw
mst_semihalf.com
delphij

Commits

rS346259: tpm: Prevent session hijack

Summary

Check caller thread id before allowing to read the buffer to make sure that it can only be accessed by the thread that did the associated write to the TPM.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kd created this revision.Mar 26 2019, 12:00 PM

Herald added a subscriber: imp. · View Herald TranscriptMar 26 2019, 12:00 PM

kd added a reviewer: delphij.Apr 11 2019, 11:06 AM

I don't really know the internals of this driver (ideally this should be done by someone who is familiar with it), but are we sure that the write method is always called before a read? Also, if the discard callout is fired, should the owner tid be reset (because the contents is now discarded)?

In D19713#427028, @delphij wrote:

I don't really know the internals of this driver (ideally this should be done by someone who is familiar with it), but are we sure that the write method is always called before a read? Also, if the discard callout is fired, should the owner tid be reset (because the contents is now discarded)?

If the write method hasn't been called before a read then there will be nothing in the buffer and the read will fail - as pending_data_length equals 0.
Essentially the way it works is that write is used to do the entire communication with TPM and read just copies the response to userspace.
As for the discard callout, since it also clears the buffer read would fail either way and tid is not checked in write, as it is used only to restrict access to buffer contents which is empty when a write is performed.

In D19713#427179, @mindal_semihalf.com wrote:

In D19713#427028, @delphij wrote:

I don't really know the internals of this driver (ideally this should be done by someone who is familiar with it), but are we sure that the write method is always called before a read? Also, if the discard callout is fired, should the owner tid be reset (because the contents is now discarded)?

If the write method hasn't been called before a read then there will be nothing in the buffer and the read will fail - as pending_data_length equals 0.
Essentially the way it works is that write is used to do the entire communication with TPM and read just copies the response to userspace.
As for the discard callout, since it also clears the buffer read would fail either way and tid is not checked in write, as it is used only to restrict access to buffer contents which is empty when a write is performed.