All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/2] tpm: Fix error handling in async work
@ 2022-01-16  1:26 Tadeusz Struk
  2022-01-16  1:26 ` [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle Tadeusz Struk
  2022-01-16 14:34 ` [PATCH v4 1/2] tpm: Fix error handling in async work Jarkko Sakkinen
  0 siblings, 2 replies; 9+ messages in thread
From: Tadeusz Struk @ 2022-01-16  1:26 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Tadeusz Struk, Jason Gunthorpe, linux-integrity, stable, linux-kernel

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 <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")
Tested-by: Jarkko Sakkinen<jarkko@kernel.org>
Signed-off-by: Tadeusz Struk <tstruk@gmail.com>
---
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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle
  2022-01-16  1:26 [PATCH v4 1/2] tpm: Fix error handling in async work Tadeusz Struk
@ 2022-01-16  1:26 ` Tadeusz Struk
  2022-01-16 14:34   ` Jarkko Sakkinen
  2022-01-16 14:34 ` [PATCH v4 1/2] tpm: Fix error handling in async work Jarkko Sakkinen
  1 sibling, 1 reply; 9+ messages in thread
From: Tadeusz Struk @ 2022-01-16  1:26 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Tadeusz Struk, Shuah Khan, linux-integrity, linux-kselftest,
	linux-kernel

Add a test for /dev/tpmrm0 in async mode that checks if
the code handles invalid handles correctly.

Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: <linux-integrity@vger.kernel.org>
Cc: <linux-kselftest@vger.kernel.org>
Cc: <linux-kernel@vger.kernel.org>

Tested-by: Jarkko Sakkinen<jarkko@kernel.org>
Signed-off-by: Tadeusz Struk <tstruk@gmail.com>
---
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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 1/2] tpm: Fix error handling in async work
  2022-01-16  1:26 [PATCH v4 1/2] tpm: Fix error handling in async work Tadeusz Struk
  2022-01-16  1:26 ` [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle Tadeusz Struk
@ 2022-01-16 14:34 ` Jarkko Sakkinen
  1 sibling, 0 replies; 9+ messages in thread
From: Jarkko Sakkinen @ 2022-01-16 14:34 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: Jason Gunthorpe, linux-integrity, stable, linux-kernel

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 <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")
> Tested-by: Jarkko Sakkinen<jarkko@kernel.org>
> Signed-off-by: Tadeusz Struk <tstruk@gmail.com>

Thank you.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle
  2022-01-16  1:26 ` [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle Tadeusz Struk
@ 2022-01-16 14:34   ` Jarkko Sakkinen
  2022-01-16 20:04     ` Jarkko Sakkinen
  0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2022-01-16 14:34 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: Shuah Khan, linux-integrity, linux-kselftest, linux-kernel

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 <jarkko@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: <linux-integrity@vger.kernel.org>
> Cc: <linux-kselftest@vger.kernel.org>
> Cc: <linux-kernel@vger.kernel.org>
> 
> Tested-by: Jarkko Sakkinen<jarkko@kernel.org>
> Signed-off-by: Tadeusz Struk <tstruk@gmail.com>
> ---
> 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 <jarkko@kernel.org>

BR, Jarkko

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle
  2022-01-16 14:34   ` Jarkko Sakkinen
@ 2022-01-16 20:04     ` Jarkko Sakkinen
  2022-01-16 22:43       ` Tadeusz Struk
  0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2022-01-16 20:04 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: Shuah Khan, linux-integrity, linux-kselftest, linux-kernel

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 <jarkko@kernel.org>
> > Cc: Shuah Khan <shuah@kernel.org>
> > Cc: <linux-integrity@vger.kernel.org>
> > Cc: <linux-kselftest@vger.kernel.org>
> > Cc: <linux-kernel@vger.kernel.org>
> > 
> > Tested-by: Jarkko Sakkinen<jarkko@kernel.org>
> > Signed-off-by: Tadeusz Struk <tstruk@gmail.com>
> > ---
> > 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 <jarkko@kernel.org>
> 
> BR, Jarkko



$ b4 am 20220116012627.2031-2-tstruk@gmail.com
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 <jarkko@kernel.org> (✓ DKIM/kernel.org)
  ERROR: missing [2/2]!
---
Total patches: 1
---
WARNING: Thread incomplete!
 Link: https://lore.kernel.org/r/20220116012627.2031-1-tstruk@gmail.com
 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle
  2022-01-16 20:04     ` Jarkko Sakkinen
@ 2022-01-16 22:43       ` Tadeusz Struk
  2022-01-17  1:44         ` Jarkko Sakkinen
  0 siblings, 1 reply; 9+ messages in thread
From: Tadeusz Struk @ 2022-01-16 22:43 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Shuah Khan, linux-integrity, linux-kselftest, linux-kernel

On 1/16/22 12:04, Jarkko Sakkinen wrote:
> $ b4 am20220116012627.2031-2-tstruk@gmail.com
> 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<jarkko@kernel.org>  (✓ DKIM/kernel.org)
>    ERROR: missing [2/2]!
> ---
> Total patches: 1
> ---
> WARNING: Thread incomplete!
>   Link:https://lore.kernel.org/r/20220116012627.2031-1-tstruk@gmail.com
>   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: <20220116012627.2031-1-tstruk@gmail.com>

and the second is:

Message-Id: <20220116012627.2031-2-tstruk@gmail.com>

and contains:

In-Reply-To: <20220116012627.2031-1-tstruk@gmail.com>
References: <20220116012627.2031-1-tstruk@gmail.com>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle
  2022-01-16 22:43       ` Tadeusz Struk
@ 2022-01-17  1:44         ` Jarkko Sakkinen
  2022-01-17  2:55           ` Tadeusz Struk
  0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2022-01-17  1:44 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: Shuah Khan, linux-integrity, linux-kselftest, linux-kernel

On Sun, Jan 16, 2022 at 02:43:32PM -0800, Tadeusz Struk wrote:
> On 1/16/22 12:04, Jarkko Sakkinen wrote:
> > $ b4 am20220116012627.2031-2-tstruk@gmail.com
> > 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<jarkko@kernel.org>  (✓ DKIM/kernel.org)
> >    ERROR: missing [2/2]!
> > ---
> > Total patches: 1
> > ---
> > WARNING: Thread incomplete!
> >   Link:https://lore.kernel.org/r/20220116012627.2031-1-tstruk@gmail.com
> >   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: <20220116012627.2031-1-tstruk@gmail.com>
> 
> and the second is:
> 
> Message-Id: <20220116012627.2031-2-tstruk@gmail.com>
> 
> and contains:
> 
> In-Reply-To: <20220116012627.2031-1-tstruk@gmail.com>
> References: <20220116012627.2031-1-tstruk@gmail.com>
> 
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle
  2022-01-17  1:44         ` Jarkko Sakkinen
@ 2022-01-17  2:55           ` Tadeusz Struk
  2022-01-17  7:02             ` Jarkko Sakkinen
  0 siblings, 1 reply; 9+ messages in thread
From: Tadeusz Struk @ 2022-01-17  2:55 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Shuah Khan, linux-integrity, linux-kselftest, linux-kernel

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle
  2022-01-17  2:55           ` Tadeusz Struk
@ 2022-01-17  7:02             ` Jarkko Sakkinen
  0 siblings, 0 replies; 9+ messages in thread
From: Jarkko Sakkinen @ 2022-01-17  7:02 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: Shuah Khan, linux-integrity, linux-kselftest, linux-kernel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-01-17  7:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-16  1:26 [PATCH v4 1/2] tpm: Fix error handling in async work Tadeusz Struk
2022-01-16  1:26 ` [PATCH v3 2/2] selftests: tpm: add async space test with noneexisting handle Tadeusz Struk
2022-01-16 14:34   ` Jarkko Sakkinen
2022-01-16 20:04     ` Jarkko Sakkinen
2022-01-16 22:43       ` Tadeusz Struk
2022-01-17  1:44         ` Jarkko Sakkinen
2022-01-17  2:55           ` Tadeusz Struk
2022-01-17  7:02             ` Jarkko Sakkinen
2022-01-16 14:34 ` [PATCH v4 1/2] tpm: Fix error handling in async work Jarkko Sakkinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.