| Message ID | 20210903015552.17180-4-masahisa.kojima@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Miscellaneous fixes of efi_tcg2 | expand |
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 >
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; > } >
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
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 >
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; }
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(-) -- 2.17.1