linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Stalled /dev/tpmr0 when context size increases to support RSA 3072 bit keys
@ 2020-06-16 22:30 Stefan Berger
  2020-06-17 16:04 ` Stefan Berger
  2020-06-17 23:34 ` Jarkko Sakkinen
  0 siblings, 2 replies; 5+ messages in thread
From: Stefan Berger @ 2020-06-16 22:30 UTC (permalink / raw)
  To: linux-integrity, James Bottomley; +Cc: Jarkko Sakkinen

I am upgrading libtpms's TPM 2 to support RSA 3072 keys (increase 
context size to 2680 bytes) and wanted to test an upgrade from previous 
version (0.7.2) which only supports RSA 2048 keys to this newer version 
(git master). I tried to run this with clevis setting up automatic 
decryption via TPM 2, but it doesn't work and it seems the issue is due 
to a stall of /dev/tpmr0 that doesn't respond anymore.


So here's a simple sequence with added 'set -x' into clevis-encrypt-tpm2 
and clevis-decrypt-tpm2:

All good in the encryption part:

[root@localhost ~]# echo hi | clevis encrypt tpm2 '{"key":"rsa"}' > 
hi-rsa3072.jwe
+ case "$TPM2TOOLS_VERSION" in
+ tpm2_createprimary -Q -C o -g sha256 -G rsa -c 
/tmp/tmp.cqg0DMjuL4/primary.context
+ set +x
+ case "$TPM2TOOLS_VERSION" in
+ tpm2_create -Q -g sha256 -C /tmp/tmp.cqg0DMjuL4/primary.context -u 
/tmp/tmp.cqg0DMjuL4/jwk.pub -r /tmp/tmp.cqg0DMjuL4/jwk.priv -a 
'fixedtpm|fixedparent|noda|adminwithpolicy|userwithauth' -i-
+ set +x

The decryption part stalls:

[root@localhost ~]# clevis decrypt < hi-rsa3072.jwe 2>&1 | tee strace.log
+ case "$TPM2TOOLS_VERSION" in
+ tpm2_createprimary -Q -C o -g sha256 -G rsa -c 
/tmp/tmp.9W2U5Fw8HX/primary.context
+ set +x
+ case "$TPM2TOOLS_VERSION" in
+ tpm2_load -Q -C /tmp/tmp.9W2U5Fw8HX/primary.context -u 
/tmp/tmp.9W2U5Fw8HX/jwk.pub -r /tmp/tmp.9W2U5Fw8HX/jwk.priv -c 
/tmp/tmp.9W2U5Fw8HX/load.context -V
INFO on line: "362" in file: "lib/files.c": Assuming tpm context file
INFO on line: "293" in file: "lib/files.c": load: 
TPMS_CONTEXT->savedHandle: 0x80000000
^Z
[1]+  Stopped                 clevis decrypt < hi-rsa3072.jwe 2>&1 | tee 
strace.log


Note: I put the tool in the background using ctrl-Z and now I can run 
this stalled command and it works!

[root@localhost ~]# tpm2_load -Q -C /tmp/tmp.9W2U5Fw8HX/primary.context 
-u /tmp/tmp.9W2U5Fw8HX/jwk.pub -r /tmp/tmp.9W2U5Fw8HX/jwk.priv -c 
load.context -V
INFO on line: "362" in file: "lib/files.c": Assuming tpm context file
INFO on line: "293" in file: "lib/files.c": load: 
TPMS_CONTEXT->savedHandle: 0x80000000
INFO on line: "190" in file: "lib/files.c": Save 
TPMS_CONTEXT->savedHandle: 0x80000000


I know that the above is stalled because I had strace'd it:

openat(AT_FDCWD, "/dev/tpmrm0", O_RDWR|O_NONBLOCK) = 3

[...]

write(3, "\200\1\0\0\0;\0\0\1v@\0\0\7@\0\0\7\0 
\316)s\332fV_\177\326\303\221#"..., 59) = 59
poll([{fd=3, events=POLLIN}], 1, -1)    = 1 ([{fd=3, revents=POLLIN}])
read(3, "\200\1\0\0\0000\0\0\0\0\2\0\0\0\0 
\303cQ\225\0\235F\4-\225:y\353\254\220\247"..., 4096) = 48
openat(AT_FDCWD, "/tmp/tmp.9WItRVOByv/primary.context", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=3036, ...}) = 0
brk(NULL)                               = 0x55fc8d415000
brk(0x55fc8d436000)                     = 0x55fc8d436000
read(4, 
"\272\334\300\336\0\0\0\1@\0\0\1\200\0\0\0\0\0\0\0\0\0\0B\v\302\0\0\0\0\nr"..., 
4096) = 3036
lseek(4, -3036, SEEK_CUR)               = 0
write(2, "INFO on line: \"362\" in file: \"li"..., 44INFO on line: "362" 
in file: "lib/files.c": ) = 44
write(2, "Assuming tpm context file", 25Assuming tpm context file) = 25
write(2, "\n", 1
)                       = 1
read(4, 
"\272\334\300\336\0\0\0\1@\0\0\1\200\0\0\0\0\0\0\0\0\0\0B\v\302\0\0\0\0\nr"..., 
4096) = 3036
write(2, "INFO on line: \"293\" in file: \"li"..., 44INFO on line: "293" 
in file: "lib/files.c": ) = 44
write(2, "load: TPMS_CONTEXT->savedHandle:"..., 43load: 
TPMS_CONTEXT->savedHandle: 0x80000000) = 43
write(2, "\n", 1
)                       = 1
write(3, 
"\200\1\0\0\n\216\0\0\1a\0\0\0\0\0\0\0B\200\0\0\0@\0\0\1\nr\0@]\234"..., 
2702) = 2702
poll([{fd=3, events=POLLIN}], 1, -1)    = 1 ([{fd=3, revents=POLLIN}])
read(3, "\200\1\0\0\0\16\0\0\0\0\200\377\377\377", 4096) = 14
close(4)                                = 0
getpid()                                = 1591
getpid()                                = 1591
write(3, "\200\2\0\0\1{\0\0\1W\200\377\377\377\0\0\0I\2\0\0\0\0 
\203\33\326qO\214\r\0"..., 379) = 379
poll([{fd=3, events=POLLIN}], 1, -1

It's stuck polling on /dev/tpmrm0.

    Any ideas?


     Stefan



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

* Re: Stalled /dev/tpmr0 when context size increases to support RSA 3072 bit keys
  2020-06-16 22:30 Stalled /dev/tpmr0 when context size increases to support RSA 3072 bit keys Stefan Berger
@ 2020-06-17 16:04 ` Stefan Berger
  2020-06-19 17:05   ` Stefan Berger
  2020-06-17 23:34 ` Jarkko Sakkinen
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Berger @ 2020-06-17 16:04 UTC (permalink / raw)
  To: linux-integrity, James Bottomley; +Cc: Jarkko Sakkinen

On 6/16/20 6:30 PM, Stefan Berger wrote:
> I am upgrading libtpms's TPM 2 to support RSA 3072 keys (increase 
> context size to 2680 bytes) and wanted to test an upgrade from 
> previous version (0.7.2) which only supports RSA 2048 keys to this 
> newer version (git master). I tried to run this with clevis setting up 
> automatic decryption via TPM 2, but it doesn't work and it seems the 
> issue is due to a stall of /dev/tpmr0 that doesn't respond anymore.
>
>
> [...]
>
> It's stuck polling on /dev/tpmrm0.
>
>    Any ideas?

It has something to do with the offset parameter and the PAGE_SIZE as a 
limit.

[  842.288597] *offset=0
[  842.295345] *offset=2692
[  842.301011] body_size=2692, *offset=2692, buf_size=4096
[  842.301584] tpm tpm0: tpm2_save_context: out of backing storage
[  842.305463] tpm tpm0: tpm2_commit_space: error -12
[  850.793691] tpm tpm0: A TPM error (459) occurred flushing context

This here fixes it. Any suggestion for a proper fix?

Does it concatenate contexts into this PAGE_SIZE'd buffer?


diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 982d341d8837..b5e7307c7ecd 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -40,7 +40,7 @@ static void tpm2_flush_sessions(struct tpm_chip *chip, 
struct tpm_space *space)

  int tpm2_init_space(struct tpm_space *space)
  {
-       space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+       space->context_buf = kzalloc(3*PAGE_SIZE, GFP_KERNEL);
         if (!space->context_buf)
                 return -ENOMEM;

@@ -123,6 +123,7 @@ static int tpm2_save_context(struct tpm_chip *chip, 
u32 handle, u8 *buf,
         unsigned int body_size;
         int rc;

+printk(KERN_INFO "*offset=%u\n", *offset);
         rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, 
TPM2_CC_CONTEXT_SAVE);
         if (rc)
                 return rc;
@@ -147,6 +148,7 @@ static int tpm2_save_context(struct tpm_chip *chip, 
u32 handle, u8 *buf,

         body_size = tpm_buf_length(&tbuf) - TPM_HEADER_SIZE;
         if ((*offset + body_size) > buf_size) {
+printk(KERN_INFO "body_size=%u, *offset=%u, buf_size=%u\n", body_size, 
*offset, buf_size);
                 dev_warn(&chip->dev, "%s: out of backing storage\n", 
__func__);
                 tpm_buf_destroy(&tbuf);
                 return -ENOMEM;
@@ -311,7 +313,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct 
tpm_space *space, u8 *cmd,
                sizeof(space->context_tbl));
         memcpy(&chip->work_space.session_tbl, &space->session_tbl,
                sizeof(space->session_tbl));
-       memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
+       memcpy(chip->work_space.context_buf, space->context_buf, 
3*PAGE_SIZE);
         memcpy(chip->work_space.session_buf, space->session_buf, 
PAGE_SIZE);

         rc = tpm2_load_space(chip);
@@ -487,12 +489,13 @@ static int tpm2_save_space(struct tpm_chip *chip)
         int i;
         int rc;

+       printk(KERN_INFO "ARRAY_SIZE(space->context_tbl) = %lu\n", 
ARRAY_SIZE(space->context_tbl));
         for (i = 0, offset = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
                 if (!(space->context_tbl[i] && ~space->context_tbl[i]))
                         continue;

                 rc = tpm2_save_context(chip, space->context_tbl[i],
-                                      space->context_buf, PAGE_SIZE,
+                                      space->context_buf, 3*PAGE_SIZE,
                                        &offset);
                 if (rc == -ENOENT) {
                         space->context_tbl[i] = 0;
@@ -530,6 +533,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct 
tpm_space *space,
         struct tpm_header *header = buf;
         int rc;

+printk(KERN_INFO "%s:%d\n", __func__, __LINE__);
         if (!space)
                 return 0;

@@ -557,7 +561,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct 
tpm_space *space,
                sizeof(space->context_tbl));
         memcpy(&space->session_tbl, &chip->work_space.session_tbl,
                sizeof(space->session_tbl));
-       memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
+       memcpy(space->context_buf, chip->work_space.context_buf, 
3*PAGE_SIZE);
         memcpy(space->session_buf, chip->work_space.session_buf, 
PAGE_SIZE);

         return 0;


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

* Re: Stalled /dev/tpmr0 when context size increases to support RSA 3072 bit keys
  2020-06-16 22:30 Stalled /dev/tpmr0 when context size increases to support RSA 3072 bit keys Stefan Berger
  2020-06-17 16:04 ` Stefan Berger
@ 2020-06-17 23:34 ` Jarkko Sakkinen
  1 sibling, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2020-06-17 23:34 UTC (permalink / raw)
  To: Stefan Berger, Tadeusz Struk; +Cc: linux-integrity, James Bottomley

On Tue, Jun 16, 2020 at 06:30:24PM -0400, Stefan Berger wrote:
> I am upgrading libtpms's TPM 2 to support RSA 3072 keys (increase context
> size to 2680 bytes) and wanted to test an upgrade from previous version
> (0.7.2) which only supports RSA 2048 keys to this newer version (git
> master). I tried to run this with clevis setting up automatic decryption via
> TPM 2, but it doesn't work and it seems the issue is due to a stall of
> /dev/tpmr0 that doesn't respond anymore.
> 
> 
> So here's a simple sequence with added 'set -x' into clevis-encrypt-tpm2 and
> clevis-decrypt-tpm2:
> 
> All good in the encryption part:
> 
> [root@localhost ~]# echo hi | clevis encrypt tpm2 '{"key":"rsa"}' >
> hi-rsa3072.jwe
> + case "$TPM2TOOLS_VERSION" in
> + tpm2_createprimary -Q -C o -g sha256 -G rsa -c
> /tmp/tmp.cqg0DMjuL4/primary.context
> + set +x
> + case "$TPM2TOOLS_VERSION" in
> + tpm2_create -Q -g sha256 -C /tmp/tmp.cqg0DMjuL4/primary.context -u
> /tmp/tmp.cqg0DMjuL4/jwk.pub -r /tmp/tmp.cqg0DMjuL4/jwk.priv -a
> 'fixedtpm|fixedparent|noda|adminwithpolicy|userwithauth' -i-
> + set +x
> 
> The decryption part stalls:
> 
> [root@localhost ~]# clevis decrypt < hi-rsa3072.jwe 2>&1 | tee strace.log
> + case "$TPM2TOOLS_VERSION" in
> + tpm2_createprimary -Q -C o -g sha256 -G rsa -c
> /tmp/tmp.9W2U5Fw8HX/primary.context
> + set +x
> + case "$TPM2TOOLS_VERSION" in
> + tpm2_load -Q -C /tmp/tmp.9W2U5Fw8HX/primary.context -u
> /tmp/tmp.9W2U5Fw8HX/jwk.pub -r /tmp/tmp.9W2U5Fw8HX/jwk.priv -c
> /tmp/tmp.9W2U5Fw8HX/load.context -V
> INFO on line: "362" in file: "lib/files.c": Assuming tpm context file
> INFO on line: "293" in file: "lib/files.c": load: TPMS_CONTEXT->savedHandle:
> 0x80000000
> ^Z
> [1]+  Stopped                 clevis decrypt < hi-rsa3072.jwe 2>&1 | tee
> strace.log
> 
> 
> Note: I put the tool in the background using ctrl-Z and now I can run this
> stalled command and it works!
> 
> [root@localhost ~]# tpm2_load -Q -C /tmp/tmp.9W2U5Fw8HX/primary.context -u
> /tmp/tmp.9W2U5Fw8HX/jwk.pub -r /tmp/tmp.9W2U5Fw8HX/jwk.priv -c load.context
> -V
> INFO on line: "362" in file: "lib/files.c": Assuming tpm context file
> INFO on line: "293" in file: "lib/files.c": load: TPMS_CONTEXT->savedHandle:
> 0x80000000
> INFO on line: "190" in file: "lib/files.c": Save TPMS_CONTEXT->savedHandle:
> 0x80000000
> 
> 
> I know that the above is stalled because I had strace'd it:
> 
> openat(AT_FDCWD, "/dev/tpmrm0", O_RDWR|O_NONBLOCK) = 3
> 
> [...]
> 
> write(3, "\200\1\0\0\0;\0\0\1v@\0\0\7@\0\0\7\0
> \316)s\332fV_\177\326\303\221#"..., 59) = 59
> poll([{fd=3, events=POLLIN}], 1, -1)    = 1 ([{fd=3, revents=POLLIN}])
> read(3, "\200\1\0\0\0000\0\0\0\0\2\0\0\0\0
> \303cQ\225\0\235F\4-\225:y\353\254\220\247"..., 4096) = 48
> openat(AT_FDCWD, "/tmp/tmp.9WItRVOByv/primary.context", O_RDONLY) = 4
> fstat(4, {st_mode=S_IFREG|0644, st_size=3036, ...}) = 0
> brk(NULL)                               = 0x55fc8d415000
> brk(0x55fc8d436000)                     = 0x55fc8d436000
> read(4, "\272\334\300\336\0\0\0\1@\0\0\1\200\0\0\0\0\0\0\0\0\0\0B\v\302\0\0\0\0\nr"...,
> 4096) = 3036
> lseek(4, -3036, SEEK_CUR)               = 0
> write(2, "INFO on line: \"362\" in file: \"li"..., 44INFO on line: "362" in
> file: "lib/files.c": ) = 44
> write(2, "Assuming tpm context file", 25Assuming tpm context file) = 25
> write(2, "\n", 1
> )                       = 1
> read(4, "\272\334\300\336\0\0\0\1@\0\0\1\200\0\0\0\0\0\0\0\0\0\0B\v\302\0\0\0\0\nr"...,
> 4096) = 3036
> write(2, "INFO on line: \"293\" in file: \"li"..., 44INFO on line: "293" in
> file: "lib/files.c": ) = 44
> write(2, "load: TPMS_CONTEXT->savedHandle:"..., 43load:
> TPMS_CONTEXT->savedHandle: 0x80000000) = 43
> write(2, "\n", 1
> )                       = 1
> write(3,
> "\200\1\0\0\n\216\0\0\1a\0\0\0\0\0\0\0B\200\0\0\0@\0\0\1\nr\0@]\234"...,
> 2702) = 2702
> poll([{fd=3, events=POLLIN}], 1, -1)    = 1 ([{fd=3, revents=POLLIN}])
> read(3, "\200\1\0\0\0\16\0\0\0\0\200\377\377\377", 4096) = 14
> close(4)                                = 0
> getpid()                                = 1591
> getpid()                                = 1591
> write(3, "\200\2\0\0\1{\0\0\1W\200\377\377\377\0\0\0I\2\0\0\0\0
> \203\33\326qO\214\r\0"..., 379) = 379
> poll([{fd=3, events=POLLIN}], 1, -1
> 
> It's stuck polling on /dev/tpmrm0.
> 
>    Any ideas?
> 
> 
>     Stefan
> 
> 

Tadeusz,

Could this possibly be something to do with partial reads?

/Jarkko

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

* Re: Stalled /dev/tpmr0 when context size increases to support RSA 3072 bit keys
  2020-06-17 16:04 ` Stefan Berger
@ 2020-06-19 17:05   ` Stefan Berger
  2020-06-23  1:15     ` Jarkko Sakkinen
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Berger @ 2020-06-19 17:05 UTC (permalink / raw)
  To: linux-integrity, James Bottomley; +Cc: Jarkko Sakkinen

On 6/17/20 12:04 PM, Stefan Berger wrote:
> On 6/16/20 6:30 PM, Stefan Berger wrote:
>> I am upgrading libtpms's TPM 2 to support RSA 3072 keys (increase 
>> context size to 2680 bytes) and wanted to test an upgrade from 
>> previous version (0.7.2) which only supports RSA 2048 keys to this 
>> newer version (git master). I tried to run this with clevis setting 
>> up automatic decryption via TPM 2, but it doesn't work and it seems 
>> the issue is due to a stall of /dev/tpmr0 that doesn't respond anymore.
>>
>>
>> [...]
>>
>> It's stuck polling on /dev/tpmrm0.
>>
>>    Any ideas?
>
> It has something to do with the offset parameter and the PAGE_SIZE as 
> a limit.
>
> [  842.288597] *offset=0
> [  842.295345] *offset=2692
> [  842.301011] body_size=2692, *offset=2692, buf_size=4096
> [  842.301584] tpm tpm0: tpm2_save_context: out of backing storage
> [  842.305463] tpm tpm0: tpm2_commit_space: error -12
> [  850.793691] tpm tpm0: A TPM error (459) occurred flushing context
>
> This here fixes it. Any suggestion for a proper fix?
>
> Does it concatenate contexts into this PAGE_SIZE'd buffer? 


This may be a better workable patch that needs to apply to many previous 
kernel versions:


  drivers/char/tpm/tpm-chip.c   |  2 +-
  drivers/char/tpm/tpm.h        |  2 ++
  drivers/char/tpm/tpm2-space.c | 12 ++++++++----
  3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 8c77e88012e9..d32a173117b8 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -386,7 +386,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
      chip->cdev.owner = THIS_MODULE;
      chip->cdevs.owner = THIS_MODULE;

-    chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+    chip->work_space.context_buf = kzalloc(TPM_SPACE_SIZE, GFP_KERNEL);
      if (!chip->work_space.context_buf) {
          rc = -ENOMEM;
          goto out;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 0fbcede241ea..5f34187da858 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -36,6 +36,8 @@
  #define TPM_NUM_DEVICES        65536
  #define TPM_RETRY        50

+#define TPM_SPACE_SIZE          (3 * 4096)
+
  enum tpm_timeout {
      TPM_TIMEOUT = 5,    /* msecs */
      TPM_TIMEOUT_RETRY = 100, /* msecs */
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 982d341d8837..493900008fa2 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -40,7 +40,7 @@ static void tpm2_flush_sessions(struct tpm_chip *chip, 
struct tpm_space *space)

  int tpm2_init_space(struct tpm_space *space)
  {
-    space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+    space->context_buf = kzalloc(TPM_SPACE_SIZE, GFP_KERNEL);
      if (!space->context_buf)
          return -ENOMEM;

@@ -123,6 +123,7 @@ static int tpm2_save_context(struct tpm_chip *chip, 
u32 handle, u8 *buf,
      unsigned int body_size;
      int rc;

+printk(KERN_INFO "*offset=%u\n", *offset);
      rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE);
      if (rc)
          return rc;
@@ -147,6 +148,7 @@ static int tpm2_save_context(struct tpm_chip *chip, 
u32 handle, u8 *buf,

      body_size = tpm_buf_length(&tbuf) - TPM_HEADER_SIZE;
      if ((*offset + body_size) > buf_size) {
+printk(KERN_INFO "body_size=%u, *offset=%u, buf_size=%u\n", body_size, 
*offset, buf_size);
          dev_warn(&chip->dev, "%s: out of backing storage\n", __func__);
          tpm_buf_destroy(&tbuf);
          return -ENOMEM;
@@ -311,7 +313,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct 
tpm_space *space, u8 *cmd,
             sizeof(space->context_tbl));
      memcpy(&chip->work_space.session_tbl, &space->session_tbl,
             sizeof(space->session_tbl));
-    memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
+    memcpy(chip->work_space.context_buf, space->context_buf, 
TPM_SPACE_SIZE);
      memcpy(chip->work_space.session_buf, space->session_buf, PAGE_SIZE);

      rc = tpm2_load_space(chip);
@@ -487,12 +489,13 @@ static int tpm2_save_space(struct tpm_chip *chip)
      int i;
      int rc;

+    printk(KERN_INFO "ARRAY_SIZE(space->context_tbl) = %lu\n", 
ARRAY_SIZE(space->context_tbl));
      for (i = 0, offset = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
          if (!(space->context_tbl[i] && ~space->context_tbl[i]))
              continue;

          rc = tpm2_save_context(chip, space->context_tbl[i],
-                       space->context_buf, PAGE_SIZE,
+                       space->context_buf, TPM_SPACE_SIZE,
                         &offset);
          if (rc == -ENOENT) {
              space->context_tbl[i] = 0;
@@ -530,6 +533,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct 
tpm_space *space,
      struct tpm_header *header = buf;
      int rc;

+printk(KERN_INFO "%s:%d\n", __func__, __LINE__);
      if (!space)
          return 0;

@@ -557,7 +561,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct 
tpm_space *space,
             sizeof(space->context_tbl));
      memcpy(&space->session_tbl, &chip->work_space.session_tbl,
             sizeof(space->session_tbl));
-    memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
+    memcpy(space->context_buf, chip->work_space.context_buf, 
TPM_SPACE_SIZE);
      memcpy(space->session_buf, chip->work_space.session_buf, PAGE_SIZE);

      return 0;



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

* Re: Stalled /dev/tpmr0 when context size increases to support RSA 3072 bit keys
  2020-06-19 17:05   ` Stefan Berger
@ 2020-06-23  1:15     ` Jarkko Sakkinen
  0 siblings, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2020-06-23  1:15 UTC (permalink / raw)
  To: Stefan Berger; +Cc: linux-integrity, James Bottomley

On Fri, Jun 19, 2020 at 01:05:59PM -0400, Stefan Berger wrote:
> On 6/17/20 12:04 PM, Stefan Berger wrote:
> > On 6/16/20 6:30 PM, Stefan Berger wrote:
> > > I am upgrading libtpms's TPM 2 to support RSA 3072 keys (increase
> > > context size to 2680 bytes) and wanted to test an upgrade from
> > > previous version (0.7.2) which only supports RSA 2048 keys to this
> > > newer version (git master). I tried to run this with clevis setting
> > > up automatic decryption via TPM 2, but it doesn't work and it seems
> > > the issue is due to a stall of /dev/tpmr0 that doesn't respond
> > > anymore.
> > > 
> > > 
> > > [...]
> > > 
> > > It's stuck polling on /dev/tpmrm0.
> > > 
> > >    Any ideas?
> > 
> > It has something to do with the offset parameter and the PAGE_SIZE as a
> > limit.
> > 
> > [  842.288597] *offset=0
> > [  842.295345] *offset=2692
> > [  842.301011] body_size=2692, *offset=2692, buf_size=4096
> > [  842.301584] tpm tpm0: tpm2_save_context: out of backing storage
> > [  842.305463] tpm tpm0: tpm2_commit_space: error -12
> > [  850.793691] tpm tpm0: A TPM error (459) occurred flushing context
> > 
> > This here fixes it. Any suggestion for a proper fix?
> > 
> > Does it concatenate contexts into this PAGE_SIZE'd buffer?
> 
> 
> This may be a better workable patch that needs to apply to many previous
> kernel versions:
> 
> 
>  drivers/char/tpm/tpm-chip.c   |  2 +-
>  drivers/char/tpm/tpm.h        |  2 ++
>  drivers/char/tpm/tpm2-space.c | 12 ++++++++----
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 8c77e88012e9..d32a173117b8 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -386,7 +386,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>      chip->cdev.owner = THIS_MODULE;
>      chip->cdevs.owner = THIS_MODULE;
> 
> -    chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +    chip->work_space.context_buf = kzalloc(TPM_SPACE_SIZE, GFP_KERNEL);
>      if (!chip->work_space.context_buf) {
>          rc = -ENOMEM;
>          goto out;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 0fbcede241ea..5f34187da858 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -36,6 +36,8 @@
>  #define TPM_NUM_DEVICES        65536
>  #define TPM_RETRY        50
> 
> +#define TPM_SPACE_SIZE          (3 * 4096)
> +
>  enum tpm_timeout {
>      TPM_TIMEOUT = 5,    /* msecs */
>      TPM_TIMEOUT_RETRY = 100, /* msecs */
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 982d341d8837..493900008fa2 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -40,7 +40,7 @@ static void tpm2_flush_sessions(struct tpm_chip *chip,
> struct tpm_space *space)
> 
>  int tpm2_init_space(struct tpm_space *space)
>  {
> -    space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +    space->context_buf = kzalloc(TPM_SPACE_SIZE, GFP_KERNEL);
>      if (!space->context_buf)
>          return -ENOMEM;
> 
> @@ -123,6 +123,7 @@ static int tpm2_save_context(struct tpm_chip *chip, u32
> handle, u8 *buf,
>      unsigned int body_size;
>      int rc;
> 
> +printk(KERN_INFO "*offset=%u\n", *offset);
>      rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE);
>      if (rc)
>          return rc;
> @@ -147,6 +148,7 @@ static int tpm2_save_context(struct tpm_chip *chip, u32
> handle, u8 *buf,
> 
>      body_size = tpm_buf_length(&tbuf) - TPM_HEADER_SIZE;
>      if ((*offset + body_size) > buf_size) {
> +printk(KERN_INFO "body_size=%u, *offset=%u, buf_size=%u\n", body_size,
> *offset, buf_size);
>          dev_warn(&chip->dev, "%s: out of backing storage\n", __func__);
>          tpm_buf_destroy(&tbuf);
>          return -ENOMEM;
> @@ -311,7 +313,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct
> tpm_space *space, u8 *cmd,
>             sizeof(space->context_tbl));
>      memcpy(&chip->work_space.session_tbl, &space->session_tbl,
>             sizeof(space->session_tbl));
> -    memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
> +    memcpy(chip->work_space.context_buf, space->context_buf,
> TPM_SPACE_SIZE);
>      memcpy(chip->work_space.session_buf, space->session_buf, PAGE_SIZE);
> 
>      rc = tpm2_load_space(chip);
> @@ -487,12 +489,13 @@ static int tpm2_save_space(struct tpm_chip *chip)
>      int i;
>      int rc;
> 
> +    printk(KERN_INFO "ARRAY_SIZE(space->context_tbl) = %lu\n",
> ARRAY_SIZE(space->context_tbl));
>      for (i = 0, offset = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
>          if (!(space->context_tbl[i] && ~space->context_tbl[i]))
>              continue;
> 
>          rc = tpm2_save_context(chip, space->context_tbl[i],
> -                       space->context_buf, PAGE_SIZE,
> +                       space->context_buf, TPM_SPACE_SIZE,
>                         &offset);
>          if (rc == -ENOENT) {
>              space->context_tbl[i] = 0;
> @@ -530,6 +533,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct
> tpm_space *space,
>      struct tpm_header *header = buf;
>      int rc;
> 
> +printk(KERN_INFO "%s:%d\n", __func__, __LINE__);
>      if (!space)
>          return 0;
> 
> @@ -557,7 +561,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct
> tpm_space *space,
>             sizeof(space->context_tbl));
>      memcpy(&space->session_tbl, &chip->work_space.session_tbl,
>             sizeof(space->session_tbl));
> -    memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
> +    memcpy(space->context_buf, chip->work_space.context_buf,
> TPM_SPACE_SIZE);
>      memcpy(space->session_buf, chip->work_space.session_buf, PAGE_SIZE);
> 
>      return 0;
> 
> 

The implementation works correctly, the buffer just doesn't have enough
space for the keys:

	body_size = tpm_buf_length(&tbuf) - TPM_HEADER_SIZE;
	if ((*offset + body_size) > buf_size) {
		dev_warn(&chip->dev, "%s: out of backing storage\n", __func__);
		tpm_buf_destroy(&tbuf);
		return -ENOMEM;
	}

I think that the right way to refine this would be to reallocate the
buffer when needed.

I'll send a patch that does this.

/Jarkko

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

end of thread, other threads:[~2020-06-23  1:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-16 22:30 Stalled /dev/tpmr0 when context size increases to support RSA 3072 bit keys Stefan Berger
2020-06-17 16:04 ` Stefan Berger
2020-06-19 17:05   ` Stefan Berger
2020-06-23  1:15     ` Jarkko Sakkinen
2020-06-17 23:34 ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).