| Message ID | 20211220211700.5772-1-tstruk@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] tpm: Fix error handling in async work | expand |
On Mon, Dec 20, 2021 at 01:16:59PM -0800, Tadeusz Struk wrote: > When an invalid (non exitsinting) handle is used in a tpm command, > that uses the resource manager interface (/dev/tpmrm0) the resource > manager tries to load it from its internal cache, but fails and > returns an -EINVAL error to the caller. The existing async handler > doesn't handle these error cases currently and the condition in the > poll handler never returns mask with EPOLLIN set causing the userspace > code to get stack. Make sure that error conditions also contribute > to the poll mask so that a correct error code could passed back > to the caller. Can you instead describe a failure scenario? This is very cryptic. /Jarkko
On 12/28/21 15:47, Jarkko Sakkinen wrote: >> When an invalid (non existing) handle is used in a tpm command, >> that uses the resource manager interface (/dev/tpmrm0) the resource >> manager tries to load it from its internal cache, but fails and >> returns an -EINVAL error to the caller. The existing async handler >> doesn't handle these error cases currently and the condition in the >> poll handler never returns mask with EPOLLIN set causing the userspace >> code to get stack. Make sure that error conditions also contribute >> to the poll mask so that a correct error code could passed back >> to the caller. > Can you instead describe a failure scenario? This is very cryptic. The problem is that the poll call blocks and the application gets stuck until the tpm_timeout_work() wakes it up after 120 sec (jiffies + (120 * HZ)). I will update the description, fix all the typos, and resend it. Thanks, Tadeusz
On Tue, Dec 28, 2021 at 04:08:42PM -0800, Tadeusz Struk wrote: > On 12/28/21 15:47, Jarkko Sakkinen wrote: > > > When an invalid (non existing) handle is used in a tpm command, > > > that uses the resource manager interface (/dev/tpmrm0) the resource > > > manager tries to load it from its internal cache, but fails and > > > returns an -EINVAL error to the caller. The existing async handler > > > doesn't handle these error cases currently and the condition in the > > > poll handler never returns mask with EPOLLIN set causing the userspace > > > code to get stack. Make sure that error conditions also contribute > > > to the poll mask so that a correct error code could passed back > > > to the caller. > > Can you instead describe a failure scenario? This is very cryptic. > > The problem is that the poll call blocks and the application gets stuck > until the tpm_timeout_work() wakes it up after 120 sec (jiffies + (120 * HZ)). > I will update the description, fix all the typos, and resend it. > > Thanks, > Tadeusz OK, great, thank you. BR, Jarkko
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c index c08cbb306636..fe2679f84cb6 100644 --- a/drivers/char/tpm/tpm-dev-common.c +++ b/drivers/char/tpm/tpm-dev-common.c @@ -69,7 +69,7 @@ static void tpm_dev_async_work(struct work_struct *work) ret = tpm_dev_transmit(priv->chip, priv->space, priv->data_buffer, sizeof(priv->data_buffer)); tpm_put_ops(priv->chip); - if (ret > 0) { + if (ret != 0) { priv->response_length = ret; mod_timer(&priv->user_read_timer, jiffies + (120 * HZ)); }
When an invalid (non exitsinting) handle is used in a tpm command, that uses the resource manager interface (/dev/tpmrm0) the resource manager tries to load it from its internal cache, but fails and returns an -EINVAL error to the caller. The existing async handler doesn't handle these error cases currently and the condition in the poll handler never returns mask with EPOLLIN set causing the userspace code to get stack. Make sure that error conditions also contribute to the poll mask so that a correct error code could passed back to the caller. Cc: Jarkko Sakkinen <jarkko@kernel.org> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: <linux-integrity@vger.kernel.org> Cc: <stable@vger.kernel.org> Cc: <linux-kernel@vger.kernel.org> Fixes: 9e1b74a63f77 ("tpm: add support for nonblocking operation") Signed-off-by: Tadeusz Struk <tstruk@gmail.com> --- drivers/char/tpm/tpm-dev-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)