diff mbox series

[1/2] tpm: Fix error handling in async work

Message ID 20211220211700.5772-1-tstruk@gmail.com (mailing list archive)
State New
Headers show
Series [1/2] tpm: Fix error handling in async work | expand

Commit Message

Tadeusz Struk Dec. 20, 2021, 9:16 p.m. UTC
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(-)

Comments

Jarkko Sakkinen Dec. 28, 2021, 11:47 p.m. UTC | #1
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
Tadeusz Struk Dec. 29, 2021, 12:08 a.m. UTC | #2
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
Jarkko Sakkinen Dec. 29, 2021, 1:42 a.m. UTC | #3
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 mbox series

Patch

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));
 	}