> -----Original Message-----
> From: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> Sent: Tuesday, May 26, 2020 7:25 PM
> To: linux-integrity@xxxxxxxxxxxxxxx
> Cc: Jarkko Sakkinen; Jason Gunthorpe; Limonciello, Mario; Alex Guzman
> Subject: [PATCH] tpm2: fix TIS locality timeout problems
>
>
> [EXTERNAL EMAIL]
>
> It has been reported that some TIS based TPMs are giving unexpected
> errors when using the O_NONBLOCK path. The problem is that some TPMs
> don't like it when you get and then relinquish a locality (as the
> tpm_try_get_ops/tpm_put_ops pair does) without sending a command.
> This currently happens all the time in the O_NONBLOCK write path. We
> can fix this by moving the tpm_try_get_ops further down the code to
> after the O_NONBLOCK determination is made. This is safe because the
> priv->buffer_mutex still protects the priv state being modified.
>
> Fixes: d23d12484307 ("tpm: fix invalid locking in NONBLOCKING mode")
> Reported-by: Mario Limonciello <Mario.Limonciello@xxxxxxxx>
> Tested-by: Alex Guzman <alex@xxxxxxxxx>
> Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
A few other notes for right here:
* You should probably mention this buglink:
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=206275
* And add another Reported-by: For Alex probably makes sense , as he is
the original reporter to the bug. I just helped to bring it here.
* This should CC stable (as this regression managed to hit the stable
kernels too).
> ---
> drivers/char/tpm/tpm-dev-common.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-
> common.c
> index 87f449340202..1784530b8387 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -189,15 +189,6 @@ ssize_t tpm_common_write(struct file *file, const char
> __user *buf,
> goto out;
> }
>
> - /* atomic tpm command send and result receive. We only hold the ops
> - * lock during this period so that the tpm can be unregistered even if
> - * the char dev is held open.
> - */
> - if (tpm_try_get_ops(priv->chip)) {
> - ret = -EPIPE;
> - goto out;
> - }
> -
> priv->response_length = 0;
> priv->response_read = false;
> *off = 0;
> @@ -211,11 +202,19 @@ ssize_t tpm_common_write(struct file *file, const char
> __user *buf,
> if (file->f_flags & O_NONBLOCK) {
> priv->command_enqueued = true;
> queue_work(tpm_dev_wq, &priv->async_work);
> - tpm_put_ops(priv->chip);
> mutex_unlock(&priv->buffer_mutex);
> return size;
> }
>
> + /* atomic tpm command send and result receive. We only hold the ops
> + * lock during this period so that the tpm can be unregistered even if
> + * the char dev is held open.
> + */
> + if (tpm_try_get_ops(priv->chip)) {
> + ret = -EPIPE;
> + goto out;
> + }
> +
> ret = tpm_dev_transmit(priv->chip, priv->space, priv->data_buffer,
> sizeof(priv->data_buffer));
> tpm_put_ops(priv->chip);
> --
> 2.26.2
