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
the tpm_dev_transmit 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.
The result is that the poll call blocks and the application gets stuck
until the user_read_timer wakes it up after 120 sec.
Change the tpm_dev_async_work function to handle error conditions
returned from tpm_dev_transmit they are also reflected in the poll mask
and 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")
Tested-by: Jarkko Sakkinen<[email protected]>
Signed-off-by: Tadeusz Struk <[email protected]>
---
Changed in v2:
- Updated commit message with better problem description
- Fixed typeos.
Changed in v3:
- Added a comment to tpm_dev_async_work.
- Updated commit message.
Changed in v4:
- Fixed a typo in the comment.
---
drivers/char/tpm/tpm-dev-common.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index c08cbb306636..dc4c0a0a5129 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -69,7 +69,13 @@ 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 is > 0 then tpm_dev_transmit returned the size of the
+ * response. If ret is < 0 then tpm_dev_transmit failed and
+ * returned an error code.
+ */
+ if (ret != 0) {
priv->response_length = ret;
mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
}
--
2.30.2
Add a test for /dev/tpmrm0 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]>
Tested-by: Jarkko Sakkinen<[email protected]>
Signed-off-by: Tadeusz Struk <[email protected]>
---
Changed in v2:
- Updated commit message
Changed in v3:
- Fixed typo in the function name
---
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..340ffef97fb6 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_invalid_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
On Sat, Jan 15, 2022 at 05:26:26PM -0800, Tadeusz Struk 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
> the tpm_dev_transmit 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.
> The result is that the poll call blocks and the application gets stuck
> until the user_read_timer wakes it up after 120 sec.
> Change the tpm_dev_async_work function to handle error conditions
> returned from tpm_dev_transmit they are also reflected in the poll mask
> and 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")
> Tested-by: Jarkko Sakkinen<[email protected]>
> Signed-off-by: Tadeusz Struk <[email protected]>
Thank you.
Reviewed-by: Jarkko Sakkinen <[email protected]>
BR, Jarkko
On Sat, Jan 15, 2022 at 05:26:27PM -0800, Tadeusz Struk wrote:
> Add a test for /dev/tpmrm0 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]>
>
> Tested-by: Jarkko Sakkinen<[email protected]>
> Signed-off-by: Tadeusz Struk <[email protected]>
> ---
> Changed in v2:
> - Updated commit message
> Changed in v3:
> - Fixed typo in the function name
> ---
> 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..340ffef97fb6 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_invalid_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
>
Thank you.
Reviewed-by: Jarkko Sakkinen <[email protected]>
BR, Jarkko
On Sun, Jan 16, 2022 at 04:34:20PM +0200, Jarkko Sakkinen wrote:
> On Sat, Jan 15, 2022 at 05:26:27PM -0800, Tadeusz Struk wrote:
> > Add a test for /dev/tpmrm0 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]>
> >
> > Tested-by: Jarkko Sakkinen<[email protected]>
> > Signed-off-by: Tadeusz Struk <[email protected]>
> > ---
> > Changed in v2:
> > - Updated commit message
> > Changed in v3:
> > - Fixed typo in the function name
> > ---
> > 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..340ffef97fb6 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_invalid_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
> >
>
> Thank you.
>
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> BR, Jarkko
$ b4 am [email protected]
Looking up https://lore.kernel.org/r/20220116012627.2031-2-tstruk%40gmail.com
Grabbing thread from lore.kernel.org/all/20220116012627.2031-2-tstruk%40gmail.com/t.mbox.gz
Analyzing 4 messages in the thread
Will use the latest revision: v4
You can pick other revisions using the -vN flag
Checking attestation on all messages, may take a moment...
---
✓ [PATCH v4 1/2] tpm: Fix error handling in async work
✓ Signed: DKIM/gmail.com
+ Reviewed-by: Jarkko Sakkinen <[email protected]> (✓ DKIM/kernel.org)
ERROR: missing [2/2]!
---
Total patches: 1
---
WARNING: Thread incomplete!
Link: https://lore.kernel.org/r/[email protected]
Base: applies clean to current tree
git am ./v4_20220115_tstruk_tpm_fix_error_handling_in_async_work.mbx
The patch set is broken. It has patches from two different patch sets.
I saved the 2nd patch from my email client.
BR, Jarkko
On 1/16/22 12:04, Jarkko Sakkinen wrote:
> $ b4 [email protected]
> Looking uphttps://lore.kernel.org/r/20220116012627.2031-2-tstruk%40gmail.com
> Grabbing thread from lore.kernel.org/all/20220116012627.2031-2-tstruk%40gmail.com/t.mbox.gz
> Analyzing 4 messages in the thread
> Will use the latest revision: v4
> You can pick other revisions using the -vN flag
> Checking attestation on all messages, may take a moment...
> ---
> ✓ [PATCH v4 1/2] tpm: Fix error handling in async work
> ✓ Signed: DKIM/gmail.com
> + Reviewed-by: Jarkko Sakkinen<[email protected]> (✓ DKIM/kernel.org)
> ERROR: missing [2/2]!
> ---
> Total patches: 1
> ---
> WARNING: Thread incomplete!
> Link:https://lore.kernel.org/r/[email protected]
> Base: applies clean to current tree
> git am ./v4_20220115_tstruk_tpm_fix_error_handling_in_async_work.mbx
>
> The patch set is broken. It has patches from two different patch sets.
>
> I saved the 2nd patch from my email client.
I'm not sure why it doesn't like it. They both are generated with the same
git format-patch command. If you look at the source, the first one is:
Message-Id: <[email protected]>
and the second is:
Message-Id: <[email protected]>
and contains:
In-Reply-To: <[email protected]>
References: <[email protected]>
Maybe it doesn't like the fact that the first in v4 and the second in v3?
Do you need me to do anything else to get that applied?
--
Thanks,
Tadeusz
On Sun, Jan 16, 2022 at 02:43:32PM -0800, Tadeusz Struk wrote:
> On 1/16/22 12:04, Jarkko Sakkinen wrote:
> > $ b4 [email protected]
> > Looking uphttps://lore.kernel.org/r/20220116012627.2031-2-tstruk%40gmail.com
> > Grabbing thread from lore.kernel.org/all/20220116012627.2031-2-tstruk%40gmail.com/t.mbox.gz
> > Analyzing 4 messages in the thread
> > Will use the latest revision: v4
> > You can pick other revisions using the -vN flag
> > Checking attestation on all messages, may take a moment...
> > ---
> > ✓ [PATCH v4 1/2] tpm: Fix error handling in async work
> > ✓ Signed: DKIM/gmail.com
> > + Reviewed-by: Jarkko Sakkinen<[email protected]> (✓ DKIM/kernel.org)
> > ERROR: missing [2/2]!
> > ---
> > Total patches: 1
> > ---
> > WARNING: Thread incomplete!
> > Link:https://lore.kernel.org/r/[email protected]
> > Base: applies clean to current tree
> > git am ./v4_20220115_tstruk_tpm_fix_error_handling_in_async_work.mbx
> >
> > The patch set is broken. It has patches from two different patch sets.
> >
> > I saved the 2nd patch from my email client.
>
> I'm not sure why it doesn't like it. They both are generated with the same
> git format-patch command. If you look at the source, the first one is:
>
> Message-Id: <[email protected]>
>
> and the second is:
>
> Message-Id: <[email protected]>
>
> and contains:
>
> In-Reply-To: <[email protected]>
> References: <[email protected]>
>
> Maybe it doesn't like the fact that the first in v4 and the second in v3?
> Do you need me to do anything else to get that applied?
>
> --
> Thanks,
> Tadeusz
NP, Both are applied now.
BR, Jarkko
On 1/16/22 17:44, Jarkko Sakkinen wrote:
> NP, Both are applied now.
Looking at
https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/
I can see only the 2/2 selftest applied.
On Sun, Jan 16, 2022 at 06:55:25PM -0800, Tadeusz Struk wrote:
> On 1/16/22 17:44, Jarkko Sakkinen wrote:
> > NP, Both are applied now.
>
> Looking at
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/
> I can see only the 2/2 selftest applied.
It was my bad, forgot the update the remote master. Now it should be
good.
BR, Jarkko