tcti: Replace 2 read calls in the 'receive' function with a single read. #949
Conversation
|
this resolves #947 |
src/tss2-tcti/tcti-device.c
Outdated
| } | ||
| if ((size_t)size != tcti_common->header.size) { | ||
| LOG_INFO ("TPM2 header size disagrees with number of bytes read from " | ||
| "fd %d. Header says %u but we read %zu bytes.", |
There was a problem hiding this comment.
IMHO this should be LOG_WARNING at least; i.e. we want people to see this on default log-level settings...
Maybe also rephrase to mention "... read from TPM device node using fd $d ..." or something similar.
There was a problem hiding this comment.
Promoted this message to a warning as requested but I left the message as-is. Having the fd # is sufficient data.
src/tss2-tcti/tcti-device.c
Outdated
| if (*response_size < tcti_common->header.size) { | ||
| LOG_WARNING ("TPM2 response header size is larger than the provided " | ||
| "buffer: future use of this TCTI will likely fail."); | ||
| } |
There was a problem hiding this comment.
We should rc = TSS2_TCTI_RC_GENERAL_FAILURE; in this case...
| * buffer we print a warning. This allows "expert applications" to | ||
| * precalculate the required response buffer size for whatever commands they | ||
| * may send. | ||
| */ |
There was a problem hiding this comment.
As mentioned in the issue tracking this, should we fix the driver instead? Is this just an implementation detail of the tpm driver or a constraint of the TPM? If the former, I really think we should change the driver instead of having workarounds in user-space.
There was a problem hiding this comment.
It would be great if interaction with the kernel driver could be made a bit more friendly to user space. Having the time to hack up a patch for the kernel is another issue. For now, userspace needs this workaround.
flihp
force-pushed
the
tcti-device-one-read
branch
from
405f1e9
to
2459f85
Compare
|
whoops, forgot to fixup test cases to account for the RC change requested by @AndreasFuchsSIT |
The linux kernel driver closes connections that don't read the full response buffer in one read. This requires that the device TCTI do exactly this to keep our FDs closed prematurely. It also means we can't read the response header to know the size of the response ahead of reading the response body. If the caller queries for the size of the response before providing us with the buffer (by providing a NULL buffer) then we just tell them to give us a 4k buffer to be safe. We do not however require that the buffer be this large to allow "expert" applications that may want to precalculate the size of their response buffers in advance. This commit also updates the unit tests to account for this change. Signed-off-by: Philip Tricca <flihp@twobit.us>
flihp
force-pushed
the
tcti-device-one-read
branch
from
2459f85
to
5806b47
Compare
|
Review comments have been addressed IMHO. I'm going to push this to unblock downstream work that requires the device TCTI be functional. |
The linux kernel driver closes connections that don't read the full
response buffer in one read. This requires that the device TCTI
do exactly this to keep our FDs closed prematurely. It also means we
can't read the response header to know the size of the response ahead
of reading the response body.
If the caller queries for the size of the response before providing us
with the buffer (by providing a NULL buffer) then we just tell them to
give us a 4k buffer to be safe. We do not however require that the
buffer be this large to allow "expert" applications that may want to
precalculate the size of their response buffers in advance.
This commit also updates the unit tests to account for this change.
Signed-off-by: Philip Tricca flihp@twobit.us