2021-12-20 21:17:36

by Tadeusz Struk

[permalink] [raw]
Subject: [PATCH 1/2] tpm: Fix error handling in async work

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 <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Fixes: 9e1b74a63f77 ("tpm: add support for nonblocking operation")
Signed-off-by: Tadeusz Struk <[email protected]>
---
drivers/char/tpm/tpm-dev-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

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



2021-12-20 21:17:47

by Tadeusz Struk

[permalink] [raw]
Subject: [PATCH 2/2] selftests: tpm: add async space test with noneexisting handle

Add a test for tpm2 spaces in async mode that checks if
the code handles invalid handles correctly.

Cc: Jarkko Sakkinen <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Signed-off-by: Tadeusz Struk <[email protected]>
---
tools/testing/selftests/tpm2/tpm2_tests.py | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/tools/testing/selftests/tpm2/tpm2_tests.py b/tools/testing/selftests/tpm2/tpm2_tests.py
index 9d764306887b..b373b0936e40 100644
--- a/tools/testing/selftests/tpm2/tpm2_tests.py
+++ b/tools/testing/selftests/tpm2/tpm2_tests.py
@@ -302,3 +302,19 @@ class AsyncTest(unittest.TestCase):
log.debug("Calling get_cap in a NON_BLOCKING mode")
async_client.get_cap(tpm2.TPM2_CAP_HANDLES, tpm2.HR_LOADED_SESSION)
async_client.close()
+
+ def test_flush_invlid_context(self):
+ log = logging.getLogger(__name__)
+ log.debug(sys._getframe().f_code.co_name)
+
+ async_client = tpm2.Client(tpm2.Client.FLAG_SPACE | tpm2.Client.FLAG_NONBLOCK)
+ log.debug("Calling flush_context passing in an invalid handle ")
+ handle = 0x80123456
+ rc = 0
+ try:
+ async_client.flush_context(handle)
+ except OSError as e:
+ rc = e.errno
+
+ self.assertEqual(rc, 22)
+ async_client.close()
--
2.30.2


2021-12-28 23:47:51

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 1/2] tpm: Fix error handling in async work

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

2021-12-29 00:08:46

by Tadeusz Struk

[permalink] [raw]
Subject: Re: [PATCH 1/2] tpm: Fix error handling in async work

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

2021-12-29 01:42:26

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 1/2] tpm: Fix error handling in async work

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