Re: [RFC PATCH] tpm: don't return -EINVAL if TPM command validation fails

Ken Goldman
2017-12-08T19:51:06Z

On 11/17/2017 5:07 AM, Javier Martinez Canillas wrote:
> According to the TPM Library Specification, a TPM device must do a command
> header validation before processing and return a TPM_RC_COMMAND_CODE code
> if the command is not implemented and the TPM_RC_COMMAND_SIZE code if the
> command buffer size is not correct.
>
> So user-space will expect to handle these response codes as errors, but if
> the in-kernel resource manager is used (/dev/tpmrm?) then an -EINVAL errno
> code is returned instead if the command isn't implemented or the buffer
> size isn't correct. This confuses user-space since doesn't expect that.
>
> This is also not consistent with the behavior when not using TPM spaces
> and accessing the TPM directly (/dev/tpm?), in this case the command is
> is sent to the TPM anyways and user-space can get an error from the TPM.
>
> Instead of returning an -EINVAL errno code when the tpm_validate_command()
> function fails, allow the command to be sent to the TPM but just don't do
> any TPM space management. That way the TPM can report back a proper error
> and the behavior be consistent when using either /dev/tpm? or /dev/tpmrm?.

At a basic level, I agree with Javier.

If the device driver cannot send the command to the TPM (e.g., if the
size of the transmit buffer doesn't match commandSize), then the driver
should return an error.

Beyond those, IMHO the driver should pass through well formed commands
and return the TPM response to the user level applications.

Some reasons:

- It's what the application developer expects.

- The behavior should be (as much as possible) the same, whether one is
using /dev/tpmrm0, /dev/tpm0, or the TPM simulator.

- The application developer can use the TPM spec (or set a breakpoint in
the simulator) to debug. -EINVAL requires the application developer to
dig into kernel code.

It also seems easier for the device driver, and so potentially will
require less maintenance.