[PATCH v2] tpm: Disable tpm verifier if tpm is not present

[PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

This helps to prevent out of memory error when reading large files via disabling
tpm device as verifier has to read all content into memory in one chunk to
measure the hash and extend to tpm.

Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
 grub-core/commands/efi/tpm.c | 37 ++++++++++++++++++++++++++++++++++++
 grub-core/commands/tpm.c     |  4 ++++
 include/grub/tpm.h           |  1 +
 3 files changed, 42 insertions(+)

diff --git a/grub-core/commands/efi/tpm.c b/grub-core/commands/efi/tpm.c
index ae09c1bf8..e1f343fea 100644
--- a/grub-core/commands/efi/tpm.c
+++ b/grub-core/commands/efi/tpm.c
@@ -287,3 +287,40 @@ grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
   else
     return grub_tpm2_log_event (tpm_handle, buf, size, pcr, description);
 }
+
+int
+grub_tpm_present (void)
+{
+  grub_efi_handle_t tpm_handle;
+  grub_efi_uint8_t protocol_version;
+
+  if (!grub_tpm_handle_find (&tpm_handle, &protocol_version))
+    return 0;
+
+  if (protocol_version == 1)
+    {
+      grub_efi_tpm_protocol_t *tpm;
+
+      tpm = grub_efi_open_protocol (tpm_handle, &tpm_guid,
+				    GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+      if (!tpm)
+	{
+	  grub_dprintf ("tpm", "Cannot open TPM protocol\n");
+	  return 0;
+	}
+      return grub_tpm1_present (tpm);
+    }
+  else
+    {
+      grub_efi_tpm2_protocol_t *tpm;
+
+      tpm = grub_efi_open_protocol (tpm_handle, &tpm2_guid,
+				    GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+      if (!tpm)
+	{
+	  grub_dprintf ("tpm", "Cannot open TPM protocol\n");
+	  return 0;
+	}
+      return grub_tpm2_present (tpm);
+    }
+}
diff --git a/grub-core/commands/tpm.c b/grub-core/commands/tpm.c
index 2052c36ea..cb8ed6b94 100644
--- a/grub-core/commands/tpm.c
+++ b/grub-core/commands/tpm.c
@@ -86,10 +86,14 @@ struct grub_file_verifier grub_tpm_verifier = {
 
 GRUB_MOD_INIT (tpm)
 {
+  if (!grub_tpm_present())
+    return;
   grub_verifier_register (&grub_tpm_verifier);
 }
 
 GRUB_MOD_FINI (tpm)
 {
+  if (!grub_tpm_present())
+    return;
   grub_verifier_unregister (&grub_tpm_verifier);
 }
diff --git a/include/grub/tpm.h b/include/grub/tpm.h
index 5c285cbc5..c19fcbd0a 100644
--- a/include/grub/tpm.h
+++ b/include/grub/tpm.h
@@ -36,4 +36,5 @@
 
 grub_err_t grub_tpm_measure (unsigned char *buf, grub_size_t size,
 			     grub_uint8_t pcr, const char *description);
+int grub_tpm_present (void);
 #endif
-- 
2.35.3

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Daniel Kiper]

On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> This helps to prevent out of memory error when reading large files via disabling
> tpm device as verifier has to read all content into memory in one chunk to
> measure the hash and extend to tpm.

How does this patch help when the TPM is present in the system?

Daniel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > This helps to prevent out of memory error when reading large files via disabling
> > tpm device as verifier has to read all content into memory in one chunk to
> > measure the hash and extend to tpm.
> 
> How does this patch help when the TPM is present in the system?

If the firmware menu offers option to disable TPM device, then this
patch can be useful to get around 'out of memory error' through
disabling TPM device from firmware in order to make tpm verifier won't
be in the way of reading huge files.

This is essentially a compromised solution as long as tpm module can be
a built-in module in signed image and at the same time user may come
across the need to open huge files, for eg, loopback mount in grub for
the rescue image. In this case they could be opted in to disable tpm
device from firmware to proceed if they run into out of memory or other
(slow) reading issues.

Thanks,
Michael

> 
> Daniel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Daniel Kiper]

On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > This helps to prevent out of memory error when reading large files via disabling
> > > tpm device as verifier has to read all content into memory in one chunk to
> > > measure the hash and extend to tpm.
> >
> > How does this patch help when the TPM is present in the system?
>
> If the firmware menu offers option to disable TPM device, then this
> patch can be useful to get around 'out of memory error' through
> disabling TPM device from firmware in order to make tpm verifier won't
> be in the way of reading huge files.
>
> This is essentially a compromised solution as long as tpm module can be
> a built-in module in signed image and at the same time user may come
> across the need to open huge files, for eg, loopback mount in grub for
> the rescue image. In this case they could be opted in to disable tpm
> device from firmware to proceed if they run into out of memory or other
> (slow) reading issues.

I think I would prefer something similar to this [1] patch. Of course
if [1] is not enough...

Daniel

[1] http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=a4356538d03c5a5350790b6453b523fb9214c2e9

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > > This helps to prevent out of memory error when reading large files via disabling
> > > > tpm device as verifier has to read all content into memory in one chunk to
> > > > measure the hash and extend to tpm.
> > >
> > > How does this patch help when the TPM is present in the system?
> >
> > If the firmware menu offers option to disable TPM device, then this
> > patch can be useful to get around 'out of memory error' through
> > disabling TPM device from firmware in order to make tpm verifier won't
> > be in the way of reading huge files.
> >
> > This is essentially a compromised solution as long as tpm module can be
> > a built-in module in signed image and at the same time user may come
> > across the need to open huge files, for eg, loopback mount in grub for
> > the rescue image. In this case they could be opted in to disable tpm
> > device from firmware to proceed if they run into out of memory or other
> > (slow) reading issues.
> 
> I think I would prefer something similar to this [1] patch. Of course
> if [1] is not enough...

The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all
incoming files, which gets loaded into memory in its entirety as an
duplicated copy to disk files. The overhead is too huge to some low
profile hardwares with smaller memory or when the boot path has to cover
very large files, hence the out of memory error.

I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
measures and extends file intergrity. But we ought to avoid the overhead
when TPM device is not present or disabled by the user.

The patch [1] seems to deal with the tpm error which prevents a file
from being opened, which is orthogonal to the memory allocation issue in
the common verifier before tpm doing measurement.

Thanks,
Michael

> 
> Daniel
> 
> [1] http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=a4356538d03c5a5350790b6453b523fb9214c2e9
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Daniel Kiper]

On Fri, Nov 25, 2022 at 03:00:48PM +0800, Michael Chang via Grub-devel wrote:
> On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > > > This helps to prevent out of memory error when reading large files via disabling
> > > > > tpm device as verifier has to read all content into memory in one chunk to
> > > > > measure the hash and extend to tpm.
> > > >
> > > > How does this patch help when the TPM is present in the system?
> > >
> > > If the firmware menu offers option to disable TPM device, then this
> > > patch can be useful to get around 'out of memory error' through
> > > disabling TPM device from firmware in order to make tpm verifier won't
> > > be in the way of reading huge files.
> > >
> > > This is essentially a compromised solution as long as tpm module can be
> > > a built-in module in signed image and at the same time user may come
> > > across the need to open huge files, for eg, loopback mount in grub for
> > > the rescue image. In this case they could be opted in to disable tpm
> > > device from firmware to proceed if they run into out of memory or other
> > > (slow) reading issues.
> >
> > I think I would prefer something similar to this [1] patch. Of course
> > if [1] is not enough...
>
> The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all
> incoming files, which gets loaded into memory in its entirety as an
> duplicated copy to disk files. The overhead is too huge to some low
> profile hardwares with smaller memory or when the boot path has to cover
> very large files, hence the out of memory error.
>
> I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
> measures and extends file intergrity. But we ought to avoid the overhead
> when TPM device is not present or disabled by the user.
>
> The patch [1] seems to deal with the tpm error which prevents a file
> from being opened, which is orthogonal to the memory allocation issue in
> the common verifier before tpm doing measurement.

This is what I expected. So, that is why I said "Of course if [1] is not
enough..."... :-) Now I think it would be nice if TPM verifier is
disabled when the TPM device is broken/disabled/... and/or somebody sets
a separate environemnt variable in the GRUB. WDYT?

Daniel

[1] http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=a4356538d03c5a5350790b6453b523fb9214c2e9

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

On Tue, Nov 29, 2022 at 04:11:48PM +0100, Daniel Kiper wrote:
> On Fri, Nov 25, 2022 at 03:00:48PM +0800, Michael Chang via Grub-devel wrote:
> > On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> > > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> > > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > This helps to prevent out of memory error when reading large files via disabling
> > > > > > tpm device as verifier has to read all content into memory in one chunk to
> > > > > > measure the hash and extend to tpm.
> > > > >
> > > > > How does this patch help when the TPM is present in the system?
> > > >
> > > > If the firmware menu offers option to disable TPM device, then this
> > > > patch can be useful to get around 'out of memory error' through
> > > > disabling TPM device from firmware in order to make tpm verifier won't
> > > > be in the way of reading huge files.
> > > >
> > > > This is essentially a compromised solution as long as tpm module can be
> > > > a built-in module in signed image and at the same time user may come
> > > > across the need to open huge files, for eg, loopback mount in grub for
> > > > the rescue image. In this case they could be opted in to disable tpm
> > > > device from firmware to proceed if they run into out of memory or other
> > > > (slow) reading issues.
> > >
> > > I think I would prefer something similar to this [1] patch. Of course
> > > if [1] is not enough...
> >
> > The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all
> > incoming files, which gets loaded into memory in its entirety as an
> > duplicated copy to disk files. The overhead is too huge to some low
> > profile hardwares with smaller memory or when the boot path has to cover
> > very large files, hence the out of memory error.
> >
> > I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
> > measures and extends file intergrity. But we ought to avoid the overhead
> > when TPM device is not present or disabled by the user.
> >
> > The patch [1] seems to deal with the tpm error which prevents a file
> > from being opened, which is orthogonal to the memory allocation issue in
> > the common verifier before tpm doing measurement.
> 
> This is what I expected. So, that is why I said "Of course if [1] is not
> enough..."... :-) Now I think it would be nice if TPM verifier is
> disabled when the TPM device is broken/disabled/... and/or somebody sets
> a separate environemnt variable in the GRUB. WDYT?

I'm not sure if a separate environment a good idea, because it would
expose the funtion to disable verifier in a way much readily accessible
through one of grub command line interface, grub.cfg and grubenv file so
that the intention have to be very carefully reviewed here.

Thanks,
Michael

> 
> Daniel
> 
> [1] http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=a4356538d03c5a5350790b6453b523fb9214c2e9
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Daniel Kiper]

Adding James, Stefan and Robbie...

On Mon, Feb 20, 2023 at 12:57:01PM +0800, Michael Chang via Grub-devel wrote:
> On Tue, Nov 29, 2022 at 04:11:48PM +0100, Daniel Kiper wrote:
> > On Fri, Nov 25, 2022 at 03:00:48PM +0800, Michael Chang via Grub-devel wrote:
> > > On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> > > > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> > > > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > This helps to prevent out of memory error when reading large files via disabling
> > > > > > > tpm device as verifier has to read all content into memory in one chunk to
> > > > > > > measure the hash and extend to tpm.
> > > > > >
> > > > > > How does this patch help when the TPM is present in the system?
> > > > >
> > > > > If the firmware menu offers option to disable TPM device, then this
> > > > > patch can be useful to get around 'out of memory error' through
> > > > > disabling TPM device from firmware in order to make tpm verifier won't
> > > > > be in the way of reading huge files.
> > > > >
> > > > > This is essentially a compromised solution as long as tpm module can be
> > > > > a built-in module in signed image and at the same time user may come
> > > > > across the need to open huge files, for eg, loopback mount in grub for
> > > > > the rescue image. In this case they could be opted in to disable tpm
> > > > > device from firmware to proceed if they run into out of memory or other
> > > > > (slow) reading issues.
> > > >
> > > > I think I would prefer something similar to this [1] patch. Of course
> > > > if [1] is not enough...
> > >
> > > The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all
> > > incoming files, which gets loaded into memory in its entirety as an
> > > duplicated copy to disk files. The overhead is too huge to some low
> > > profile hardwares with smaller memory or when the boot path has to cover
> > > very large files, hence the out of memory error.
> > >
> > > I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
> > > measures and extends file intergrity. But we ought to avoid the overhead
> > > when TPM device is not present or disabled by the user.
> > >
> > > The patch [1] seems to deal with the tpm error which prevents a file
> > > from being opened, which is orthogonal to the memory allocation issue in
> > > the common verifier before tpm doing measurement.
> >
> > This is what I expected. So, that is why I said "Of course if [1] is not
> > enough..."... :-) Now I think it would be nice if TPM verifier is
> > disabled when the TPM device is broken/disabled/... and/or somebody sets
> > a separate environemnt variable in the GRUB. WDYT?
>
> I'm not sure if a separate environment a good idea, because it would
> expose the funtion to disable verifier in a way much readily accessible
> through one of grub command line interface, grub.cfg and grubenv file so
> that the intention have to be very carefully reviewed here.

I think it should be safe because even if somebody is doing nasty things
with disabling the TPM verifier they can be easily detected or will lead
to early boot failures when the TPM is used to store secrets. Of course
there is small chance somebody disables the TPM verifier during platform
initialization/installation. However, this should be also easily to
detect due to, e.g., lack of measurements. Anyway, I would extend the
GRUB documentation with a note saying that platform
initialization/installation should happen in well controlled
environment. Just in case...

Daniel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

On Thu, Feb 23, 2023 at 02:22:27PM +0100, Daniel Kiper wrote:
> Adding James, Stefan and Robbie...
> 
> On Mon, Feb 20, 2023 at 12:57:01PM +0800, Michael Chang via Grub-devel wrote:
> > On Tue, Nov 29, 2022 at 04:11:48PM +0100, Daniel Kiper wrote:
> > > On Fri, Nov 25, 2022 at 03:00:48PM +0800, Michael Chang via Grub-devel wrote:
> > > > On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> > > > > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > > > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > > This helps to prevent out of memory error when reading large files via disabling
> > > > > > > > tpm device as verifier has to read all content into memory in one chunk to
> > > > > > > > measure the hash and extend to tpm.
> > > > > > >
> > > > > > > How does this patch help when the TPM is present in the system?
> > > > > >
> > > > > > If the firmware menu offers option to disable TPM device, then this
> > > > > > patch can be useful to get around 'out of memory error' through
> > > > > > disabling TPM device from firmware in order to make tpm verifier won't
> > > > > > be in the way of reading huge files.
> > > > > >
> > > > > > This is essentially a compromised solution as long as tpm module can be
> > > > > > a built-in module in signed image and at the same time user may come
> > > > > > across the need to open huge files, for eg, loopback mount in grub for
> > > > > > the rescue image. In this case they could be opted in to disable tpm
> > > > > > device from firmware to proceed if they run into out of memory or other
> > > > > > (slow) reading issues.
> > > > >
> > > > > I think I would prefer something similar to this [1] patch. Of course
> > > > > if [1] is not enough...
> > > >
> > > > The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all
> > > > incoming files, which gets loaded into memory in its entirety as an
> > > > duplicated copy to disk files. The overhead is too huge to some low
> > > > profile hardwares with smaller memory or when the boot path has to cover
> > > > very large files, hence the out of memory error.
> > > >
> > > > I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
> > > > measures and extends file intergrity. But we ought to avoid the overhead
> > > > when TPM device is not present or disabled by the user.
> > > >
> > > > The patch [1] seems to deal with the tpm error which prevents a file
> > > > from being opened, which is orthogonal to the memory allocation issue in
> > > > the common verifier before tpm doing measurement.
> > >
> > > This is what I expected. So, that is why I said "Of course if [1] is not
> > > enough..."... :-) Now I think it would be nice if TPM verifier is
> > > disabled when the TPM device is broken/disabled/... and/or somebody sets
> > > a separate environemnt variable in the GRUB. WDYT?
> >
> > I'm not sure if a separate environment a good idea, because it would
> > expose the funtion to disable verifier in a way much readily accessible
> > through one of grub command line interface, grub.cfg and grubenv file so
> > that the intention have to be very carefully reviewed here.
> 
> I think it should be safe because even if somebody is doing nasty things
> with disabling the TPM verifier they can be easily detected or will lead
> to early boot failures when the TPM is used to store secrets. Of course
> there is small chance somebody disables the TPM verifier during platform
> initialization/installation. However, this should be also easily to
> detect due to, e.g., lack of measurements.

IMHO it is hardly be detectable, there's no clear distinction to it is a
result of lack of measurement or a compromised system if we know that
tpm is active and working ...

> Anyway, I would extend the
> GRUB documentation with a note saying that platform
> initialization/installation should happen in well controlled
> environment. Just in case...

Yes the initial bootstrapping environemt or process is important, but
also there's system update which should happen in a well controlled
environment as well.

Thanks,
Michael

> 
> Daniel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Daniel Kiper]

On Tue, Feb 28, 2023 at 11:22:14AM +0800, Michael Chang via Grub-devel wrote:
> On Thu, Feb 23, 2023 at 02:22:27PM +0100, Daniel Kiper wrote:
> > Adding James, Stefan and Robbie...
> >
> > On Mon, Feb 20, 2023 at 12:57:01PM +0800, Michael Chang via Grub-devel wrote:
> > > On Tue, Nov 29, 2022 at 04:11:48PM +0100, Daniel Kiper wrote:
> > > > On Fri, Nov 25, 2022 at 03:00:48PM +0800, Michael Chang via Grub-devel wrote:
> > > > > On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> > > > > > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > > > > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > > > This helps to prevent out of memory error when reading large files via disabling
> > > > > > > > > tpm device as verifier has to read all content into memory in one chunk to
> > > > > > > > > measure the hash and extend to tpm.
> > > > > > > >
> > > > > > > > How does this patch help when the TPM is present in the system?
> > > > > > >
> > > > > > > If the firmware menu offers option to disable TPM device, then this
> > > > > > > patch can be useful to get around 'out of memory error' through
> > > > > > > disabling TPM device from firmware in order to make tpm verifier won't
> > > > > > > be in the way of reading huge files.
> > > > > > >
> > > > > > > This is essentially a compromised solution as long as tpm module can be
> > > > > > > a built-in module in signed image and at the same time user may come
> > > > > > > across the need to open huge files, for eg, loopback mount in grub for
> > > > > > > the rescue image. In this case they could be opted in to disable tpm
> > > > > > > device from firmware to proceed if they run into out of memory or other
> > > > > > > (slow) reading issues.
> > > > > >
> > > > > > I think I would prefer something similar to this [1] patch. Of course
> > > > > > if [1] is not enough...
> > > > >
> > > > > The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all
> > > > > incoming files, which gets loaded into memory in its entirety as an
> > > > > duplicated copy to disk files. The overhead is too huge to some low
> > > > > profile hardwares with smaller memory or when the boot path has to cover
> > > > > very large files, hence the out of memory error.
> > > > >
> > > > > I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
> > > > > measures and extends file intergrity. But we ought to avoid the overhead
> > > > > when TPM device is not present or disabled by the user.
> > > > >
> > > > > The patch [1] seems to deal with the tpm error which prevents a file
> > > > > from being opened, which is orthogonal to the memory allocation issue in
> > > > > the common verifier before tpm doing measurement.
> > > >
> > > > This is what I expected. So, that is why I said "Of course if [1] is not
> > > > enough..."... :-) Now I think it would be nice if TPM verifier is
> > > > disabled when the TPM device is broken/disabled/... and/or somebody sets
> > > > a separate environemnt variable in the GRUB. WDYT?
> > >
> > > I'm not sure if a separate environment a good idea, because it would
> > > expose the funtion to disable verifier in a way much readily accessible
> > > through one of grub command line interface, grub.cfg and grubenv file so
> > > that the intention have to be very carefully reviewed here.
> >
> > I think it should be safe because even if somebody is doing nasty things
> > with disabling the TPM verifier they can be easily detected or will lead
> > to early boot failures when the TPM is used to store secrets. Of course
> > there is small chance somebody disables the TPM verifier during platform
> > initialization/installation. However, this should be also easily to
> > detect due to, e.g., lack of measurements.
>
> IMHO it is hardly be detectable, there's no clear distinction to it is a
> result of lack of measurement or a compromised system if we know that
> tpm is active and working ...

Sudden lack of measurements should be a red flag and machine should be
treated initially as a compromised. Additionally, how the proposed
variable/switch/... in the GRUB differs from the switch in the BIOS
setup which you mentioned above? The result of their use will be
the same.

> > Anyway, I would extend the
> > GRUB documentation with a note saying that platform
> > initialization/installation should happen in well controlled
> > environment. Just in case...
>
> Yes the initial bootstrapping environemt or process is important, but
> also there's system update which should happen in a well controlled
> environment as well.

Yes, and we can document this as well...

Daniel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

On Thu, Mar 02, 2023 at 07:59:00PM +0100, Daniel Kiper wrote:
> On Tue, Feb 28, 2023 at 11:22:14AM +0800, Michael Chang via Grub-devel wrote:
> > On Thu, Feb 23, 2023 at 02:22:27PM +0100, Daniel Kiper wrote:
> > > Adding James, Stefan and Robbie...
> > >
> > > On Mon, Feb 20, 2023 at 12:57:01PM +0800, Michael Chang via Grub-devel wrote:
> > > > On Tue, Nov 29, 2022 at 04:11:48PM +0100, Daniel Kiper wrote:
> > > > > On Fri, Nov 25, 2022 at 03:00:48PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> > > > > > > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > > > > > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > > > > This helps to prevent out of memory error when reading large files via disabling
> > > > > > > > > > tpm device as verifier has to read all content into memory in one chunk to
> > > > > > > > > > measure the hash and extend to tpm.
> > > > > > > > >
> > > > > > > > > How does this patch help when the TPM is present in the system?
> > > > > > > >
> > > > > > > > If the firmware menu offers option to disable TPM device, then this
> > > > > > > > patch can be useful to get around 'out of memory error' through
> > > > > > > > disabling TPM device from firmware in order to make tpm verifier won't
> > > > > > > > be in the way of reading huge files.
> > > > > > > >
> > > > > > > > This is essentially a compromised solution as long as tpm module can be
> > > > > > > > a built-in module in signed image and at the same time user may come
> > > > > > > > across the need to open huge files, for eg, loopback mount in grub for
> > > > > > > > the rescue image. In this case they could be opted in to disable tpm
> > > > > > > > device from firmware to proceed if they run into out of memory or other
> > > > > > > > (slow) reading issues.
> > > > > > >
> > > > > > > I think I would prefer something similar to this [1] patch. Of course
> > > > > > > if [1] is not enough...
> > > > > >
> > > > > > The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all
> > > > > > incoming files, which gets loaded into memory in its entirety as an
> > > > > > duplicated copy to disk files. The overhead is too huge to some low
> > > > > > profile hardwares with smaller memory or when the boot path has to cover
> > > > > > very large files, hence the out of memory error.
> > > > > >
> > > > > > I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
> > > > > > measures and extends file intergrity. But we ought to avoid the overhead
> > > > > > when TPM device is not present or disabled by the user.
> > > > > >
> > > > > > The patch [1] seems to deal with the tpm error which prevents a file
> > > > > > from being opened, which is orthogonal to the memory allocation issue in
> > > > > > the common verifier before tpm doing measurement.
> > > > >
> > > > > This is what I expected. So, that is why I said "Of course if [1] is not
> > > > > enough..."... :-) Now I think it would be nice if TPM verifier is
> > > > > disabled when the TPM device is broken/disabled/... and/or somebody sets
> > > > > a separate environemnt variable in the GRUB. WDYT?
> > > >
> > > > I'm not sure if a separate environment a good idea, because it would
> > > > expose the funtion to disable verifier in a way much readily accessible
> > > > through one of grub command line interface, grub.cfg and grubenv file so
> > > > that the intention have to be very carefully reviewed here.
> > >
> > > I think it should be safe because even if somebody is doing nasty things
> > > with disabling the TPM verifier they can be easily detected or will lead
> > > to early boot failures when the TPM is used to store secrets. Of course
> > > there is small chance somebody disables the TPM verifier during platform
> > > initialization/installation. However, this should be also easily to
> > > detect due to, e.g., lack of measurements.
> >
> > IMHO it is hardly be detectable, there's no clear distinction to it is a
> > result of lack of measurement or a compromised system if we know that
> > tpm is active and working ...
> 
> Sudden lack of measurements should be a red flag and machine should be
> treated initially as a compromised. Additionally, how the proposed
> variable/switch/... in the GRUB differs from the switch in the BIOS
> setup which you mentioned above? The result of their use will be
> the same.

The off switch in bios may completely shut down the tpm device, so it
won't be usable at all to the user who wants to actively disable it
there. On the other hand the swich in grub is in a context where tpm is
still in action, the missing measurement could lead to a comprehension
of compromised system and it can break root of trust, killing the most
idea of using a tpm to attest integrity of a booted system. 

So instead of making grub bypass tpm become possible, I think user
should always go for bios to disable tpm if running into OOM issue upon
executing memory consuming commands like loopback, initrd etc, in
particular after grub upgrade to add tpm module in small memory systems. 

Thanks,
Michael

> 
> > > Anyway, I would extend the
> > > GRUB documentation with a note saying that platform
> > > initialization/installation should happen in well controlled
> > > environment. Just in case...
> >
> > Yes the initial bootstrapping environemt or process is important, but
> > also there's system update which should happen in a well controlled
> > environment as well.
> 
> Yes, and we can document this as well...
> 
> Daniel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Daniel Kiper]

On Fri, Mar 03, 2023 at 12:18:19PM +0800, Michael Chang wrote:
> On Thu, Mar 02, 2023 at 07:59:00PM +0100, Daniel Kiper wrote:
> > On Tue, Feb 28, 2023 at 11:22:14AM +0800, Michael Chang via Grub-devel wrote:
> > > On Thu, Feb 23, 2023 at 02:22:27PM +0100, Daniel Kiper wrote:
> > > > Adding James, Stefan and Robbie...
> > > >
> > > > On Mon, Feb 20, 2023 at 12:57:01PM +0800, Michael Chang via Grub-devel wrote:
> > > > > On Tue, Nov 29, 2022 at 04:11:48PM +0100, Daniel Kiper wrote:
> > > > > > On Fri, Nov 25, 2022 at 03:00:48PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> > > > > > > > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > > > > > > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > > > > > This helps to prevent out of memory error when reading large files via disabling
> > > > > > > > > > > tpm device as verifier has to read all content into memory in one chunk to
> > > > > > > > > > > measure the hash and extend to tpm.
> > > > > > > > > >
> > > > > > > > > > How does this patch help when the TPM is present in the system?
> > > > > > > > >
> > > > > > > > > If the firmware menu offers option to disable TPM device, then this
> > > > > > > > > patch can be useful to get around 'out of memory error' through
> > > > > > > > > disabling TPM device from firmware in order to make tpm verifier won't
> > > > > > > > > be in the way of reading huge files.
> > > > > > > > >
> > > > > > > > > This is essentially a compromised solution as long as tpm module can be
> > > > > > > > > a built-in module in signed image and at the same time user may come
> > > > > > > > > across the need to open huge files, for eg, loopback mount in grub for
> > > > > > > > > the rescue image. In this case they could be opted in to disable tpm
> > > > > > > > > device from firmware to proceed if they run into out of memory or other
> > > > > > > > > (slow) reading issues.
> > > > > > > >
> > > > > > > > I think I would prefer something similar to this [1] patch. Of course
> > > > > > > > if [1] is not enough...
> > > > > > >
> > > > > > > The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all
> > > > > > > incoming files, which gets loaded into memory in its entirety as an
> > > > > > > duplicated copy to disk files. The overhead is too huge to some low
> > > > > > > profile hardwares with smaller memory or when the boot path has to cover
> > > > > > > very large files, hence the out of memory error.
> > > > > > >
> > > > > > > I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
> > > > > > > measures and extends file intergrity. But we ought to avoid the overhead
> > > > > > > when TPM device is not present or disabled by the user.
> > > > > > >
> > > > > > > The patch [1] seems to deal with the tpm error which prevents a file
> > > > > > > from being opened, which is orthogonal to the memory allocation issue in
> > > > > > > the common verifier before tpm doing measurement.
> > > > > >
> > > > > > This is what I expected. So, that is why I said "Of course if [1] is not
> > > > > > enough..."... :-) Now I think it would be nice if TPM verifier is
> > > > > > disabled when the TPM device is broken/disabled/... and/or somebody sets
> > > > > > a separate environemnt variable in the GRUB. WDYT?
> > > > >
> > > > > I'm not sure if a separate environment a good idea, because it would
> > > > > expose the funtion to disable verifier in a way much readily accessible
> > > > > through one of grub command line interface, grub.cfg and grubenv file so
> > > > > that the intention have to be very carefully reviewed here.
> > > >
> > > > I think it should be safe because even if somebody is doing nasty things
> > > > with disabling the TPM verifier they can be easily detected or will lead
> > > > to early boot failures when the TPM is used to store secrets. Of course
> > > > there is small chance somebody disables the TPM verifier during platform
> > > > initialization/installation. However, this should be also easily to
> > > > detect due to, e.g., lack of measurements.
> > >
> > > IMHO it is hardly be detectable, there's no clear distinction to it is a
> > > result of lack of measurement or a compromised system if we know that
> > > tpm is active and working ...
> >
> > Sudden lack of measurements should be a red flag and machine should be
> > treated initially as a compromised. Additionally, how the proposed
> > variable/switch/... in the GRUB differs from the switch in the BIOS
> > setup which you mentioned above? The result of their use will be
> > the same.
>
> The off switch in bios may completely shut down the tpm device, so it
> won't be usable at all to the user who wants to actively disable it
> there. On the other hand the swich in grub is in a context where tpm is
> still in action, the missing measurement could lead to a comprehension
> of compromised system and it can break root of trust, killing the most
> idea of using a tpm to attest integrity of a booted system.

I am still not convinced. Using one or another lead to the same result,
lack of measurements. Am I missing something?

> So instead of making grub bypass tpm become possible, I think user
> should always go for bios to disable tpm if running into OOM issue upon
> executing memory consuming commands like loopback, initrd etc, in
> particular after grub upgrade to add tpm module in small memory systems.

Did you try latest GRUB upstream master branch? It has significant changes
in mm code which should solve your problems. Or at least in some cases...

Daniel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

On Fri, Mar 03, 2023 at 07:19:56PM +0100, Daniel Kiper wrote:
> On Fri, Mar 03, 2023 at 12:18:19PM +0800, Michael Chang wrote:
> > On Thu, Mar 02, 2023 at 07:59:00PM +0100, Daniel Kiper wrote:
> > > On Tue, Feb 28, 2023 at 11:22:14AM +0800, Michael Chang via Grub-devel wrote:
> > > > On Thu, Feb 23, 2023 at 02:22:27PM +0100, Daniel Kiper wrote:
> > > > > Adding James, Stefan and Robbie...
> > > > >
> > > > > On Mon, Feb 20, 2023 at 12:57:01PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > On Tue, Nov 29, 2022 at 04:11:48PM +0100, Daniel Kiper wrote:
> > > > > > > On Fri, Nov 25, 2022 at 03:00:48PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > > On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> > > > > > > > > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > > > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > > > > > > > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote:
> > > > > > > > > > > > This helps to prevent out of memory error when reading large files via disabling
> > > > > > > > > > > > tpm device as verifier has to read all content into memory in one chunk to
> > > > > > > > > > > > measure the hash and extend to tpm.
> > > > > > > > > > >
> > > > > > > > > > > How does this patch help when the TPM is present in the system?
> > > > > > > > > >
> > > > > > > > > > If the firmware menu offers option to disable TPM device, then this
> > > > > > > > > > patch can be useful to get around 'out of memory error' through
> > > > > > > > > > disabling TPM device from firmware in order to make tpm verifier won't
> > > > > > > > > > be in the way of reading huge files.
> > > > > > > > > >
> > > > > > > > > > This is essentially a compromised solution as long as tpm module can be
> > > > > > > > > > a built-in module in signed image and at the same time user may come
> > > > > > > > > > across the need to open huge files, for eg, loopback mount in grub for
> > > > > > > > > > the rescue image. In this case they could be opted in to disable tpm
> > > > > > > > > > device from firmware to proceed if they run into out of memory or other
> > > > > > > > > > (slow) reading issues.
> > > > > > > > >
> > > > > > > > > I think I would prefer something similar to this [1] patch. Of course
> > > > > > > > > if [1] is not enough...
> > > > > > > >
> > > > > > > > The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all
> > > > > > > > incoming files, which gets loaded into memory in its entirety as an
> > > > > > > > duplicated copy to disk files. The overhead is too huge to some low
> > > > > > > > profile hardwares with smaller memory or when the boot path has to cover
> > > > > > > > very large files, hence the out of memory error.
> > > > > > > >
> > > > > > > > I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
> > > > > > > > measures and extends file intergrity. But we ought to avoid the overhead
> > > > > > > > when TPM device is not present or disabled by the user.
> > > > > > > >
> > > > > > > > The patch [1] seems to deal with the tpm error which prevents a file
> > > > > > > > from being opened, which is orthogonal to the memory allocation issue in
> > > > > > > > the common verifier before tpm doing measurement.
> > > > > > >
> > > > > > > This is what I expected. So, that is why I said "Of course if [1] is not
> > > > > > > enough..."... :-) Now I think it would be nice if TPM verifier is
> > > > > > > disabled when the TPM device is broken/disabled/... and/or somebody sets
> > > > > > > a separate environemnt variable in the GRUB. WDYT?
> > > > > >
> > > > > > I'm not sure if a separate environment a good idea, because it would
> > > > > > expose the funtion to disable verifier in a way much readily accessible
> > > > > > through one of grub command line interface, grub.cfg and grubenv file so
> > > > > > that the intention have to be very carefully reviewed here.
> > > > >
> > > > > I think it should be safe because even if somebody is doing nasty things
> > > > > with disabling the TPM verifier they can be easily detected or will lead
> > > > > to early boot failures when the TPM is used to store secrets. Of course
> > > > > there is small chance somebody disables the TPM verifier during platform
> > > > > initialization/installation. However, this should be also easily to
> > > > > detect due to, e.g., lack of measurements.
> > > >
> > > > IMHO it is hardly be detectable, there's no clear distinction to it is a
> > > > result of lack of measurement or a compromised system if we know that
> > > > tpm is active and working ...
> > >
> > > Sudden lack of measurements should be a red flag and machine should be
> > > treated initially as a compromised. Additionally, how the proposed
> > > variable/switch/... in the GRUB differs from the switch in the BIOS
> > > setup which you mentioned above? The result of their use will be
> > > the same.
> >
> > The off switch in bios may completely shut down the tpm device, so it
> > won't be usable at all to the user who wants to actively disable it
> > there. On the other hand the swich in grub is in a context where tpm is
> > still in action, the missing measurement could lead to a comprehension
> > of compromised system and it can break root of trust, killing the most
> > idea of using a tpm to attest integrity of a booted system.
> 
> I am still not convinced. Using one or another lead to the same result,
> lack of measurements. Am I missing something?

Unless it is necessary for something to really work out, "convenience is
the enemy of security" could be my (lame) excuse that I didn't want to
have that in the beginning if firmware already had offered the same
functionality to the user.

It can be added later any time someone has found it beneficial.

> 
> > So instead of making grub bypass tpm become possible, I think user
> > should always go for bios to disable tpm if running into OOM issue upon
> > executing memory consuming commands like loopback, initrd etc, in
> > particular after grub upgrade to add tpm module in small memory systems.
> 
> Did you try latest GRUB upstream master branch? It has significant changes
> in mm code which should solve your problems. Or at least in some cases...

Yes I did. And they are working satisfactorily well in that many out of
memory related problems were solved. However, depending on the available
memory and usage of grub, chances are still there that the memory can be
completely exhasuted or to a limit where too little left to the booted
kernel that it cannot relocate itself.

As many distributions start to making tpm as builtin to signed grub
image, this can help in the worse case that people can opt in to disable
tpm from the firwmare settings and continue as if they were before.

Thanks,
Michael

> 
> Daniel

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Stefan Berger]

On 10/7/22 01:37, Michael Chang via Grub-devel wrote:
> This helps to prevent out of memory error when reading large files via disabling
> tpm device as verifier has to read all content into memory in one chunk to
> measure the hash and extend to tpm.

For ibmvtpm driver support this change here would be need. Can you merge it into your patch once the ibmvtpm driver is in the repo?

diff --git a/grub-core/commands/ieee1275/ibmvtpm.c b/grub-core/commands/ieee1275/ibmvtpm.c
index 239942d27..e01759c17 100644
--- a/grub-core/commands/ieee1275/ibmvtpm.c
+++ b/grub-core/commands/ieee1275/ibmvtpm.c
@@ -135,16 +135,6 @@ grub_err_t
  grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
                   const char *description)
  {
-  /*
-   * Call tpm_init() 'late' rather than from GRUB_MOD_INIT() so that device nodes
-   * can be found.
-   */
-  grub_err_t err = tpm_init ();
-
-  /* Absence of a TPM isn't a failure. */
-  if (err != GRUB_ERR_NONE)
-    return GRUB_ERR_NONE;
-
    grub_dprintf ("tpm", "log_event, pcr = %d, size = 0x%" PRIxGRUB_SIZE ", %s\n",
                 pcr, size, description);

@@ -153,3 +143,13 @@ grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,

    return GRUB_ERR_NONE;
  }
+
+int
+grub_tpm_present (void)
+{
+  /*
+   * Call tpm_init() 'late' rather than from GRUB_MOD_INIT() so that device nodes
+   * can be found.
+   */
+  return tpm_init() == GRUB_ERR_NONE;
+}


Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

> diff --git a/grub-core/commands/tpm.c b/grub-core/commands/tpm.c
> index 2052c36ea..cb8ed6b94 100644
> --- a/grub-core/commands/tpm.c
> +++ b/grub-core/commands/tpm.c
> @@ -86,10 +86,14 @@ struct grub_file_verifier grub_tpm_verifier = {
>   
>   GRUB_MOD_INIT (tpm)
>   {
> +  if (!grub_tpm_present())
> +    return;

Even though this now calls grub_tpm_present() from GRUB_MOD_INIT() and I have this comment in the code
above, it does seem to call it late enough in the initialization sequence so that whatever discovered
'device nodes' before your GRUB_MOD_INIT() is called, enables the ibmvtpm driver to see the device nodes.

Per my tests powerpc grub now measures and logs for PCR 8 and 9 correctly and also extends PCRs 8 & 9.
I hope that nothing will change this initialization order because there seems to be little control over it.

    Stefan

>     grub_verifier_register (&grub_tpm_verifier);
>   }
>   
>   GRUB_MOD_FINI (tpm)
>   {
> +  if (!grub_tpm_present())
> +    return;
>     grub_verifier_unregister (&grub_tpm_verifier);
>   }
> diff --git a/include/grub/tpm.h b/include/grub/tpm.h
> index 5c285cbc5..c19fcbd0a 100644
> --- a/include/grub/tpm.h
> +++ b/include/grub/tpm.h
> @@ -36,4 +36,5 @@
>   
>   grub_err_t grub_tpm_measure (unsigned char *buf, grub_size_t size,
>   			     grub_uint8_t pcr, const char *description);
> +int grub_tpm_present (void);
>   #endif

Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

On Thu, Feb 16, 2023 at 01:02:00PM -0500, Stefan Berger wrote:
> 
> 
> On 10/7/22 01:37, Michael Chang via Grub-devel wrote:
> > This helps to prevent out of memory error when reading large files via disabling
> > tpm device as verifier has to read all content into memory in one chunk to
> > measure the hash and extend to tpm.
> 
> For ibmvtpm driver support this change here would be need. Can you merge it into your patch once the ibmvtpm driver is in the repo?
> 
> diff --git a/grub-core/commands/ieee1275/ibmvtpm.c b/grub-core/commands/ieee1275/ibmvtpm.c
> index 239942d27..e01759c17 100644
> --- a/grub-core/commands/ieee1275/ibmvtpm.c
> +++ b/grub-core/commands/ieee1275/ibmvtpm.c
> @@ -135,16 +135,6 @@ grub_err_t
>  grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
>                   const char *description)
>  {
> -  /*
> -   * Call tpm_init() 'late' rather than from GRUB_MOD_INIT() so that device nodes
> -   * can be found.
> -   */
> -  grub_err_t err = tpm_init ();
> -
> -  /* Absence of a TPM isn't a failure. */
> -  if (err != GRUB_ERR_NONE)
> -    return GRUB_ERR_NONE;
> -
>    grub_dprintf ("tpm", "log_event, pcr = %d, size = 0x%" PRIxGRUB_SIZE ", %s\n",
>                 pcr, size, description);
> 
> @@ -153,3 +143,13 @@ grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
> 
>    return GRUB_ERR_NONE;
>  }
> +
> +int
> +grub_tpm_present (void)
> +{
> +  /*
> +   * Call tpm_init() 'late' rather than from GRUB_MOD_INIT() so that device nodes
> +   * can be found.
> +   */
> +  return tpm_init() == GRUB_ERR_NONE;
> +}
> 
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> 
> > diff --git a/grub-core/commands/tpm.c b/grub-core/commands/tpm.c
> > index 2052c36ea..cb8ed6b94 100644
> > --- a/grub-core/commands/tpm.c
> > +++ b/grub-core/commands/tpm.c
> > @@ -86,10 +86,14 @@ struct grub_file_verifier grub_tpm_verifier = {
> >   GRUB_MOD_INIT (tpm)
> >   {
> > +  if (!grub_tpm_present())
> > +    return;
> 
> Even though this now calls grub_tpm_present() from GRUB_MOD_INIT() and I have this comment in the code
> above, it does seem to call it late enough in the initialization sequence so that whatever discovered
> 'device nodes' before your GRUB_MOD_INIT() is called, enables the ibmvtpm driver to see the device nodes.

Thanks to the clarification. I think it is worth to keep this comment in
the patched hunk so the reader won't get confused by the comment
followed in grub_tpm_present.

> 
> Per my tests powerpc grub now measures and logs for PCR 8 and 9 correctly and also extends PCRs 8 & 9.
> I hope that nothing will change this initialization order because there seems to be little control over it.

I will merge you change and submit a new version.

Thanks.
Michael

> 
>    Stefan
> 
> >     grub_verifier_register (&grub_tpm_verifier);
> >   }
> >   GRUB_MOD_FINI (tpm)
> >   {
> > +  if (!grub_tpm_present())
> > +    return;
> >     grub_verifier_unregister (&grub_tpm_verifier);
> >   }
> > diff --git a/include/grub/tpm.h b/include/grub/tpm.h
> > index 5c285cbc5..c19fcbd0a 100644
> > --- a/include/grub/tpm.h
> > +++ b/include/grub/tpm.h
> > @@ -36,4 +36,5 @@
> >   grub_err_t grub_tpm_measure (unsigned char *buf, grub_size_t size,
> >   			     grub_uint8_t pcr, const char *description);
> > +int grub_tpm_present (void);
> >   #endif