* [PATCH 0/3] Miscellaneous fixes of efi_tcg2
@ 2021-09-03 1:55 Masahisa Kojima
2021-09-03 1:55 ` [PATCH 1/3] efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api Masahisa Kojima
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Masahisa Kojima @ 2021-09-03 1:55 UTC (permalink / raw)
To: Heinrich Schuchardt, Alexander Graf, Ilias Apalodimas,
Simon Glass, AKASHI Takahiro, u-boot
Arm is preparing the BBSR(Base Boot Security Requirement)
compiance test tool.
This tool covers Measured Boot, it checks TCG EFI Protocol compliance.
This patch series fix the failure items detected by this
BBSR compliance test tool.
Masahisa Kojima (3):
efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api
efi_loader: fix boot_service_capability_min calculation
efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check
include/efi_tcg2.h | 4 +++-
lib/efi_loader/efi_tcg2.c | 21 ++++++++++++++++++++-
2 files changed, 23 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api 2021-09-03 1:55 [PATCH 0/3] Miscellaneous fixes of efi_tcg2 Masahisa Kojima @ 2021-09-03 1:55 ` Masahisa Kojima 2021-09-03 6:25 ` Ilias Apalodimas 2021-09-03 1:55 ` [PATCH 2/3] efi_loader: fix boot_service_capability_min calculation Masahisa Kojima 2021-09-03 1:55 ` [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check Masahisa Kojima 2 siblings, 1 reply; 12+ messages in thread From: Masahisa Kojima @ 2021-09-03 1:55 UTC (permalink / raw) To: Heinrich Schuchardt, Alexander Graf, Ilias Apalodimas, Simon Glass, AKASHI Takahiro, u-boot TCG EFI Protocol Specification defines the required parameter checking and return value for each API. This commit adds the missing parameter check and fixes the wrong return value to comply the specification. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- lib/efi_loader/efi_tcg2.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 35e69b9112..c4e9f61fd6 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -708,6 +708,18 @@ efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this, EFI_ENTRY("%p, %u, %p, %p, %p", this, log_format, event_log_location, event_log_last_entry, event_log_truncated); + if (!this || !event_log_location || !event_log_last_entry || + !event_log_truncated) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + /* Only support TPMV2 */ + if (log_format != TCG2_EVENT_LOG_FORMAT_TCG_2) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) { event_log_location = NULL; @@ -965,6 +977,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags, data_to_hash_len, (void **)&nt); if (ret != EFI_SUCCESS) { log_err("Not a valid PE-COFF file\n"); + ret = EFI_UNSUPPORTED; goto out; } ret = tcg2_hash_pe_image((void *)(uintptr_t)data_to_hash, @@ -1038,9 +1051,15 @@ efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this, { efi_status_t ret; + if (!this || !active_pcr_banks) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + EFI_ENTRY("%p, %p", this, active_pcr_banks); ret = __get_active_pcr_banks(active_pcr_banks); +out: return EFI_EXIT(ret); } -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api 2021-09-03 1:55 ` [PATCH 1/3] efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api Masahisa Kojima @ 2021-09-03 6:25 ` Ilias Apalodimas 0 siblings, 0 replies; 12+ messages in thread From: Ilias Apalodimas @ 2021-09-03 6:25 UTC (permalink / raw) To: Masahisa Kojima Cc: Heinrich Schuchardt, Alexander Graf, Simon Glass, AKASHI Takahiro, U-Boot Mailing List Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> On Fri, 3 Sept 2021 at 04:54, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > > TCG EFI Protocol Specification defines the required parameter > checking and return value for each API. > This commit adds the missing parameter check and > fixes the wrong return value to comply the specification. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > lib/efi_loader/efi_tcg2.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index 35e69b9112..c4e9f61fd6 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -708,6 +708,18 @@ efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this, > EFI_ENTRY("%p, %u, %p, %p, %p", this, log_format, event_log_location, > event_log_last_entry, event_log_truncated); > > + if (!this || !event_log_location || !event_log_last_entry || > + !event_log_truncated) { > + ret = EFI_INVALID_PARAMETER; > + goto out; > + } > + > + /* Only support TPMV2 */ > + if (log_format != TCG2_EVENT_LOG_FORMAT_TCG_2) { > + ret = EFI_INVALID_PARAMETER; > + goto out; > + } > + > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) { > event_log_location = NULL; > @@ -965,6 +977,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags, > data_to_hash_len, (void **)&nt); > if (ret != EFI_SUCCESS) { > log_err("Not a valid PE-COFF file\n"); > + ret = EFI_UNSUPPORTED; > goto out; > } > ret = tcg2_hash_pe_image((void *)(uintptr_t)data_to_hash, > @@ -1038,9 +1051,15 @@ efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this, > { > efi_status_t ret; > > + if (!this || !active_pcr_banks) { > + ret = EFI_INVALID_PARAMETER; > + goto out; > + } > + > EFI_ENTRY("%p, %p", this, active_pcr_banks); > ret = __get_active_pcr_banks(active_pcr_banks); > > +out: > return EFI_EXIT(ret); > } > > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] efi_loader: fix boot_service_capability_min calculation 2021-09-03 1:55 [PATCH 0/3] Miscellaneous fixes of efi_tcg2 Masahisa Kojima 2021-09-03 1:55 ` [PATCH 1/3] efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api Masahisa Kojima @ 2021-09-03 1:55 ` Masahisa Kojima 2021-09-03 6:19 ` Ilias Apalodimas 2021-09-03 7:01 ` Heinrich Schuchardt 2021-09-03 1:55 ` [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check Masahisa Kojima 2 siblings, 2 replies; 12+ messages in thread From: Masahisa Kojima @ 2021-09-03 1:55 UTC (permalink / raw) To: Heinrich Schuchardt, Alexander Graf, Ilias Apalodimas, Simon Glass, AKASHI Takahiro, u-boot TCG EFI Protocol Specification requires to the input ProtocolCapability.Size < size of the EFI_TCG2_BOOT_SERVICE_CAPABILITY up to and including the vendor ID field. Current implementation does different calculation, let's fix it. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- include/efi_tcg2.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h index b6b958da51..45788d55d5 100644 --- a/include/efi_tcg2.h +++ b/include/efi_tcg2.h @@ -127,8 +127,8 @@ struct efi_tcg2_boot_service_capability { efi_tcg_event_algorithm_bitmap active_pcr_banks; }; +/* up to and including the vendor ID(manufacture_id) field */ #define boot_service_capability_min \ - sizeof(struct efi_tcg2_boot_service_capability) - \ offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks) #define TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03 "Spec ID Event03" -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] efi_loader: fix boot_service_capability_min calculation 2021-09-03 1:55 ` [PATCH 2/3] efi_loader: fix boot_service_capability_min calculation Masahisa Kojima @ 2021-09-03 6:19 ` Ilias Apalodimas 2021-09-03 7:01 ` Heinrich Schuchardt 1 sibling, 0 replies; 12+ messages in thread From: Ilias Apalodimas @ 2021-09-03 6:19 UTC (permalink / raw) To: Masahisa Kojima Cc: Heinrich Schuchardt, Alexander Graf, Simon Glass, AKASHI Takahiro, U-Boot Mailing List Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> On Fri, 3 Sept 2021 at 04:54, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > > TCG EFI Protocol Specification requires to the input > ProtocolCapability.Size < size of the EFI_TCG2_BOOT_SERVICE_CAPABILITY > up to and including the vendor ID field. > Current implementation does different calculation, let's fix it. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > include/efi_tcg2.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h > index b6b958da51..45788d55d5 100644 > --- a/include/efi_tcg2.h > +++ b/include/efi_tcg2.h > @@ -127,8 +127,8 @@ struct efi_tcg2_boot_service_capability { > efi_tcg_event_algorithm_bitmap active_pcr_banks; > }; > > +/* up to and including the vendor ID(manufacture_id) field */ > #define boot_service_capability_min \ > - sizeof(struct efi_tcg2_boot_service_capability) - \ > offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks) > > #define TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03 "Spec ID Event03" > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] efi_loader: fix boot_service_capability_min calculation 2021-09-03 1:55 ` [PATCH 2/3] efi_loader: fix boot_service_capability_min calculation Masahisa Kojima 2021-09-03 6:19 ` Ilias Apalodimas @ 2021-09-03 7:01 ` Heinrich Schuchardt 2021-09-03 7:24 ` Masahisa Kojima 1 sibling, 1 reply; 12+ messages in thread From: Heinrich Schuchardt @ 2021-09-03 7:01 UTC (permalink / raw) To: Masahisa Kojima Cc: AKASHI Takahiro, Alexander Graf, Ilias Apalodimas, Simon Glass, u-boot On 9/3/21 3:55 AM, Masahisa Kojima wrote: > TCG EFI Protocol Specification requires to the input > ProtocolCapability.Size < size of the EFI_TCG2_BOOT_SERVICE_CAPABILITY > up to and including the vendor ID field. > Current implementation does different calculation, let's fix it. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > include/efi_tcg2.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h > index b6b958da51..45788d55d5 100644 > --- a/include/efi_tcg2.h > +++ b/include/efi_tcg2.h > @@ -127,8 +127,8 @@ struct efi_tcg2_boot_service_capability { > efi_tcg_event_algorithm_bitmap active_pcr_banks; > }; > > +/* up to and including the vendor ID(manufacture_id) field */ Thank you for fixing the issue. There is just a typo above: %s/manufacture_id/manufacturer_id/ I can fix that when merging > #define boot_service_capability_min \ Constants should be capitalized. This can be corrected in a future patch. Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > - sizeof(struct efi_tcg2_boot_service_capability) - \ > offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks) > > #define TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03 "Spec ID Event03" > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] efi_loader: fix boot_service_capability_min calculation 2021-09-03 7:01 ` Heinrich Schuchardt @ 2021-09-03 7:24 ` Masahisa Kojima 0 siblings, 0 replies; 12+ messages in thread From: Masahisa Kojima @ 2021-09-03 7:24 UTC (permalink / raw) To: Heinrich Schuchardt Cc: AKASHI Takahiro, Alexander Graf, Ilias Apalodimas, Simon Glass, U-Boot Mailing List On Fri, 3 Sept 2021 at 16:01, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 9/3/21 3:55 AM, Masahisa Kojima wrote: > > TCG EFI Protocol Specification requires to the input > > ProtocolCapability.Size < size of the EFI_TCG2_BOOT_SERVICE_CAPABILITY > > up to and including the vendor ID field. > > > Current implementation does different calculation, let's fix it. > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > --- > > include/efi_tcg2.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h > > index b6b958da51..45788d55d5 100644 > > --- a/include/efi_tcg2.h > > +++ b/include/efi_tcg2.h > > @@ -127,8 +127,8 @@ struct efi_tcg2_boot_service_capability { > > efi_tcg_event_algorithm_bitmap active_pcr_banks; > > }; > > > > +/* up to and including the vendor ID(manufacture_id) field */ > > Thank you for fixing the issue. There is just a typo above: > > %s/manufacture_id/manufacturer_id/ > > I can fix that when merging Sorry for my typo, thank you. > > > #define boot_service_capability_min \ > > Constants should be capitalized. This can be corrected in a future patch. Thank you, I will fix it after this patch merged. Thanks, Masahisa Kojima > > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > > - sizeof(struct efi_tcg2_boot_service_capability) - \ > > offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks) > > > > #define TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03 "Spec ID Event03" > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check 2021-09-03 1:55 [PATCH 0/3] Miscellaneous fixes of efi_tcg2 Masahisa Kojima 2021-09-03 1:55 ` [PATCH 1/3] efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api Masahisa Kojima 2021-09-03 1:55 ` [PATCH 2/3] efi_loader: fix boot_service_capability_min calculation Masahisa Kojima @ 2021-09-03 1:55 ` Masahisa Kojima 2021-09-03 6:20 ` Ilias Apalodimas 2021-09-03 6:22 ` Heinrich Schuchardt 2 siblings, 2 replies; 12+ messages in thread From: Masahisa Kojima @ 2021-09-03 1:55 UTC (permalink / raw) To: Heinrich Schuchardt, Alexander Graf, Ilias Apalodimas, Simon Glass, AKASHI Takahiro, u-boot TCG EFI Protocol Specification defines that PCRIndex parameter passed from caller must be 0 to 23. TPM2_MAX_PCRS is currently used to check the range of PCRIndex, but TPM2_MAX_PCRS is tpm2 device dependent and may have larger value. This commit newly adds EFI_TCG2_MAX_PCR_INDEX macro, it is used to check the range of PCRIndex parameter. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- include/efi_tcg2.h | 2 ++ lib/efi_loader/efi_tcg2.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h index 45788d55d5..b647361d44 100644 --- a/include/efi_tcg2.h +++ b/include/efi_tcg2.h @@ -28,6 +28,8 @@ #define EFI_TCG2_EXTEND_ONLY 0x0000000000000001 #define PE_COFF_IMAGE 0x0000000000000010 +#define EFI_TCG2_MAX_PCR_INDEX 23 + /* Algorithm Registry */ #define EFI_TCG2_BOOT_HASH_ALG_SHA1 0x00000001 #define EFI_TCG2_BOOT_HASH_ALG_SHA256 0x00000002 diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index c4e9f61fd6..b268a02976 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -958,7 +958,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags, goto out; } - if (efi_tcg_event->header.pcr_index > TPM2_MAX_PCRS) { + if (efi_tcg_event->header.pcr_index > EFI_TCG2_MAX_PCR_INDEX) { ret = EFI_INVALID_PARAMETER; goto out; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check 2021-09-03 1:55 ` [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check Masahisa Kojima @ 2021-09-03 6:20 ` Ilias Apalodimas 2021-09-03 6:22 ` Heinrich Schuchardt 1 sibling, 0 replies; 12+ messages in thread From: Ilias Apalodimas @ 2021-09-03 6:20 UTC (permalink / raw) To: Masahisa Kojima Cc: Heinrich Schuchardt, Alexander Graf, Simon Glass, AKASHI Takahiro, U-Boot Mailing List Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> On Fri, 3 Sept 2021 at 04:54, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > > TCG EFI Protocol Specification defines that PCRIndex parameter > passed from caller must be 0 to 23. > TPM2_MAX_PCRS is currently used to check the range of PCRIndex, > but TPM2_MAX_PCRS is tpm2 device dependent and may have larger value. > This commit newly adds EFI_TCG2_MAX_PCR_INDEX macro, it is used to > check the range of PCRIndex parameter. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > include/efi_tcg2.h | 2 ++ > lib/efi_loader/efi_tcg2.c | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h > index 45788d55d5..b647361d44 100644 > --- a/include/efi_tcg2.h > +++ b/include/efi_tcg2.h > @@ -28,6 +28,8 @@ > #define EFI_TCG2_EXTEND_ONLY 0x0000000000000001 > #define PE_COFF_IMAGE 0x0000000000000010 > > +#define EFI_TCG2_MAX_PCR_INDEX 23 > + > /* Algorithm Registry */ > #define EFI_TCG2_BOOT_HASH_ALG_SHA1 0x00000001 > #define EFI_TCG2_BOOT_HASH_ALG_SHA256 0x00000002 > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index c4e9f61fd6..b268a02976 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -958,7 +958,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags, > goto out; > } > > - if (efi_tcg_event->header.pcr_index > TPM2_MAX_PCRS) { > + if (efi_tcg_event->header.pcr_index > EFI_TCG2_MAX_PCR_INDEX) { > ret = EFI_INVALID_PARAMETER; > goto out; > } > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check 2021-09-03 1:55 ` [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check Masahisa Kojima 2021-09-03 6:20 ` Ilias Apalodimas @ 2021-09-03 6:22 ` Heinrich Schuchardt 2021-09-03 6:51 ` Ilias Apalodimas 1 sibling, 1 reply; 12+ messages in thread From: Heinrich Schuchardt @ 2021-09-03 6:22 UTC (permalink / raw) To: Masahisa Kojima Cc: Alexander Graf, Ilias Apalodimas, Simon Glass, u-boot, AKASHI Takahiro On 9/3/21 3:55 AM, Masahisa Kojima wrote: > TCG EFI Protocol Specification defines that PCRIndex parameter > passed from caller must be 0 to 23. > TPM2_MAX_PCRS is currently used to check the range of PCRIndex, > but TPM2_MAX_PCRS is tpm2 device dependent and may have larger value. > This commit newly adds EFI_TCG2_MAX_PCR_INDEX macro, it is used to > check the range of PCRIndex parameter. In the U-Boot code we have TPM2_MAX_PCRS hard coded as 32. Can the value be less? > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > include/efi_tcg2.h | 2 ++ > lib/efi_loader/efi_tcg2.c | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h > index 45788d55d5..b647361d44 100644 > --- a/include/efi_tcg2.h > +++ b/include/efi_tcg2.h > @@ -28,6 +28,8 @@ > #define EFI_TCG2_EXTEND_ONLY 0x0000000000000001 > #define PE_COFF_IMAGE 0x0000000000000010 > > +#define EFI_TCG2_MAX_PCR_INDEX 23 > + > /* Algorithm Registry */ > #define EFI_TCG2_BOOT_HASH_ALG_SHA1 0x00000001 > #define EFI_TCG2_BOOT_HASH_ALG_SHA256 0x00000002 > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index c4e9f61fd6..b268a02976 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -958,7 +958,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags, > goto out; > } > > - if (efi_tcg_event->header.pcr_index > TPM2_MAX_PCRS) { If TPM2_MAX_PCRS were device dependent and could be less then 23 we would need a check against both constants. Could you, please, clarify the issue. If TPM2_MAX_PCRS is always greater then 23, please, mention this in the commit message and perhaps add a comment in the code here. Best regards Heinrich > + if (efi_tcg_event->header.pcr_index > EFI_TCG2_MAX_PCR_INDEX) { > ret = EFI_INVALID_PARAMETER; > goto out; > } > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check 2021-09-03 6:22 ` Heinrich Schuchardt @ 2021-09-03 6:51 ` Ilias Apalodimas 2021-09-03 7:11 ` Heinrich Schuchardt 0 siblings, 1 reply; 12+ messages in thread From: Ilias Apalodimas @ 2021-09-03 6:51 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Masahisa Kojima, Alexander Graf, Simon Glass, u-boot, AKASHI Takahiro Hi Heinrich, On Fri, Sep 03, 2021 at 08:22:30AM +0200, Heinrich Schuchardt wrote: > On 9/3/21 3:55 AM, Masahisa Kojima wrote: > > TCG EFI Protocol Specification defines that PCRIndex parameter > > passed from caller must be 0 to 23. > > TPM2_MAX_PCRS is currently used to check the range of PCRIndex, > > but TPM2_MAX_PCRS is tpm2 device dependent and may have larger value. > > This commit newly adds EFI_TCG2_MAX_PCR_INDEX macro, it is used to > > check the range of PCRIndex parameter. > > In the U-Boot code we have TPM2_MAX_PCRS hard coded as 32. Can the value > be less? This is defined in [1] [1] https://trustedcomputinggroup.org/wp-content/uploads/TSS_Overview_Common_v0p9_r10_12june2021.pdf > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > --- > > include/efi_tcg2.h | 2 ++ > > lib/efi_loader/efi_tcg2.c | 2 +- > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h > > index 45788d55d5..b647361d44 100644 > > --- a/include/efi_tcg2.h > > +++ b/include/efi_tcg2.h > > @@ -28,6 +28,8 @@ > > #define EFI_TCG2_EXTEND_ONLY 0x0000000000000001 > > #define PE_COFF_IMAGE 0x0000000000000010 > > > > +#define EFI_TCG2_MAX_PCR_INDEX 23 > > + > > /* Algorithm Registry */ > > #define EFI_TCG2_BOOT_HASH_ALG_SHA1 0x00000001 > > #define EFI_TCG2_BOOT_HASH_ALG_SHA256 0x00000002 > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > > index c4e9f61fd6..b268a02976 100644 > > --- a/lib/efi_loader/efi_tcg2.c > > +++ b/lib/efi_loader/efi_tcg2.c > > @@ -958,7 +958,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags, > > goto out; > > } > > > > - if (efi_tcg_event->header.pcr_index > TPM2_MAX_PCRS) { > > If TPM2_MAX_PCRS were device dependent and could be less then 23 we > would need a check against both constants. > > Could you, please, clarify the issue. If TPM2_MAX_PCRS is always greater > then 23, please, mention this in the commit message and perhaps add a > comment in the code here. You don't need that (I think). If the spec says you have to check against 23, then that's what Kojima-san fixes here. If the device supports less than 23, then the current code as-is will return EFI_DEVICE_ERROR, in case the device has less than 23 PCRs. We could ofc just check against the value we get from GetCapability, which would be correct as well? > > Best regards > > Heinrich > > > + if (efi_tcg_event->header.pcr_index > EFI_TCG2_MAX_PCR_INDEX) { > > ret = EFI_INVALID_PARAMETER; > > goto out; > > } > > [1] https://trustedcomputinggroup.org/wp-content/uploads/TSS_Overview_Common_v0p9_r10_12june2021.pdf Regards /Ilias ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check 2021-09-03 6:51 ` Ilias Apalodimas @ 2021-09-03 7:11 ` Heinrich Schuchardt 0 siblings, 0 replies; 12+ messages in thread From: Heinrich Schuchardt @ 2021-09-03 7:11 UTC (permalink / raw) To: Ilias Apalodimas Cc: Masahisa Kojima, Alexander Graf, Simon Glass, u-boot, AKASHI Takahiro On 9/3/21 8:51 AM, Ilias Apalodimas wrote: > Hi Heinrich, > > On Fri, Sep 03, 2021 at 08:22:30AM +0200, Heinrich Schuchardt wrote: >> On 9/3/21 3:55 AM, Masahisa Kojima wrote: >>> TCG EFI Protocol Specification defines that PCRIndex parameter >>> passed from caller must be 0 to 23. >>> TPM2_MAX_PCRS is currently used to check the range of PCRIndex, >>> but TPM2_MAX_PCRS is tpm2 device dependent and may have larger value. >>> This commit newly adds EFI_TCG2_MAX_PCR_INDEX macro, it is used to >>> check the range of PCRIndex parameter. >> >> In the U-Boot code we have TPM2_MAX_PCRS hard coded as 32. Can the value >> be less? > > This is defined in [1] > [1] https://trustedcomputinggroup.org/wp-content/uploads/TSS_Overview_Common_v0p9_r10_12june2021.pdf > >> >>> >>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> >>> --- >>> include/efi_tcg2.h | 2 ++ >>> lib/efi_loader/efi_tcg2.c | 2 +- >>> 2 files changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h >>> index 45788d55d5..b647361d44 100644 >>> --- a/include/efi_tcg2.h >>> +++ b/include/efi_tcg2.h >>> @@ -28,6 +28,8 @@ >>> #define EFI_TCG2_EXTEND_ONLY 0x0000000000000001 >>> #define PE_COFF_IMAGE 0x0000000000000010 >>> >>> +#define EFI_TCG2_MAX_PCR_INDEX 23 >>> + >>> /* Algorithm Registry */ >>> #define EFI_TCG2_BOOT_HASH_ALG_SHA1 0x00000001 >>> #define EFI_TCG2_BOOT_HASH_ALG_SHA256 0x00000002 >>> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c >>> index c4e9f61fd6..b268a02976 100644 >>> --- a/lib/efi_loader/efi_tcg2.c >>> +++ b/lib/efi_loader/efi_tcg2.c >>> @@ -958,7 +958,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags, >>> goto out; >>> } >>> >>> - if (efi_tcg_event->header.pcr_index > TPM2_MAX_PCRS) { >> >> If TPM2_MAX_PCRS were device dependent and could be less then 23 we >> would need a check against both constants. >> >> Could you, please, clarify the issue. If TPM2_MAX_PCRS is always greater >> then 23, please, mention this in the commit message and perhaps add a >> comment in the code here. > > You don't need that (I think). If the spec says you have to check against > 23, then that's what Kojima-san fixes here. > If the device supports less than 23, then the current code as-is will > return EFI_DEVICE_ERROR, in case the device has less than 23 PCRs. > > We could ofc just check against the value we get from GetCapability, which > would be correct as well? Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> >> Best regards >> >> Heinrich >> >>> + if (efi_tcg_event->header.pcr_index > EFI_TCG2_MAX_PCR_INDEX) { >>> ret = EFI_INVALID_PARAMETER; >>> goto out; >>> } >>> > > [1] https://trustedcomputinggroup.org/wp-content/uploads/TSS_Overview_Common_v0p9_r10_12june2021.pdf > > Regards > /Ilias > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-09-03 7:24 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-03 1:55 [PATCH 0/3] Miscellaneous fixes of efi_tcg2 Masahisa Kojima 2021-09-03 1:55 ` [PATCH 1/3] efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api Masahisa Kojima 2021-09-03 6:25 ` Ilias Apalodimas 2021-09-03 1:55 ` [PATCH 2/3] efi_loader: fix boot_service_capability_min calculation Masahisa Kojima 2021-09-03 6:19 ` Ilias Apalodimas 2021-09-03 7:01 ` Heinrich Schuchardt 2021-09-03 7:24 ` Masahisa Kojima 2021-09-03 1:55 ` [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check Masahisa Kojima 2021-09-03 6:20 ` Ilias Apalodimas 2021-09-03 6:22 ` Heinrich Schuchardt 2021-09-03 6:51 ` Ilias Apalodimas 2021-09-03 7:11 ` Heinrich Schuchardt
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.