Re: [PATCH v4 1/1] cmd: gpt: add eMMC and GPT support
On 7/23/2020 5:00 AM, Rayagonda Kokatanur wrote:
> From: Corneliu Doban
>
> Add eMMC and GPT support.
> - GPT partition list and command to create the GPT added to u-boot
> environment
> - eMMC boot commands added to u-boot environment
> - new gpt commands (enumarate and setenv) that are used by broadcom
> update scripts and boot commands
> - eMMC specific u-boot configurations with environment saved in eMMC
> and GPT support
>
> Signed-off-by: Corneliu Doban
> Signed-off-by: Rayagonda Kokatanur
> ---
> Changes from v3:
> -Address review comments from Simon Glass,
> Return -ve number instead of 1 upon failure,
> Use shorter variable name,
> Modified code to avoid buffer overflow,
> Use if (!strcmp(...)) instead of if (strcmp(...) == 0)
>
> Changes from v2:
> -Address review comments from Simon Glass,
> Check for return value of part_driver_get_count(),
> Don't check return value of part_driver_get(),
> Rewrite part_driver_get() and rename to part_driver_get_first(),
> Use env_set_ulong() whereever applicable,
>
> -Address review comments from Michael Nazzareno Trimarchi,
> Add new function to set all env vriables,
>
> Changes from v1:
> -Address review comments from Simon Glass,
> Correct function comments,
> Check for return value,
> Add helper function in part.h
>
> cmd/gpt.c | 161 +
> include/part.h | 29 +
> 2 files changed, 190 insertions(+)
>
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index df759416c8..2626992e59 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> +
> +/**
> + * gpt_setenv_part_variables() - setup partition environmental variables
> + *
> + * Setup the gpt_partition_name, gpt_partition_entry, gpt_partition_addr
> + * and gpt_partition_size environment variables.
> + *
> + * @pinfo: pointer to disk partition
> + * @i: partition entry
> + *
> + * @Return: '0' on success and -ENOENT on failure
> + */
> +static int gpt_setenv_part_variables(struct disk_partition *pinfo, int i)
> +{
> + int ret;
> +
> + ret = env_set_ulong("gpt_partition_addr", pinfo->start);
> + if (ret)
> + goto fail;
You should use env_set_hex() instead of env_set_ulong() for following reasons:
1. commands (ex: mmc read/write etc.) that use gpt_partition_addr and
gpt_partition_size expect them to be in hex values.
2. maintain compatibility with existing usage in the upstream code base
https://gitlab.denx.de/u-boot/u-boot/-/blob/master/include/configs/bcm_ns3.h
ex:
"gpt setenv mmc ${sd_device_number} ${fit_image};"\
"mmc read ${fit_image_loadaddr} ${gpt_partition_addr} "\
> +
> + ret = env_set_ulong("gpt_partition_size", pinfo->size);
> + if (ret)
> + goto fail;
You should use env_set_hex() instead of env_set_ulong() for same above reasons.
Best Regards,
Thiru
Re: [PATCH] net: ftgmac100: Add support for board specific PHY interface address
Hi Tom/Joe, Is this going to be applied to u-boot/next or u-boot-net? Best Regards, Thiru On 8/17/2020 5:08 PM, Thirupathaiah Annapureddy wrote: > ftgmac100 driver is using hard-coded PHY interface address of zero. > Each board can have different PHY interface address (phy_addr). > This commit modifies the driver to make use of board specific address > by leveraging CONFIG_PHY_ADDR. > > Signed-off-by: Thirupathaiah Annapureddy > --- > drivers/net/ftgmac100.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c > index 5676a5b3ba..00bda24f1f 100644 > --- a/drivers/net/ftgmac100.c > +++ b/drivers/net/ftgmac100.c > @@ -551,6 +551,10 @@ static int ftgmac100_probe(struct udevice *dev) > priv->max_speed = pdata->max_speed; > priv->phy_addr = 0; > > +#ifdef CONFIG_PHY_ADDR > + priv->phy_addr = CONFIG_PHY_ADDR; > +#endif > + > ret = clk_enable_bulk(>clks); > if (ret) > goto out; >
Re: [RFC PATCH 1/1] image: add anti rollback protection for FIT Images
Hi Tom,
Please see my comment(s) in-line.
On 9/15/2020 6:40 AM, Tom Rini wrote:
> On Mon, Sep 14, 2020 at 11:18:25PM -0700, Thirupathaiah Annapureddy wrote:
>> Hi Simon,
>>
>> Thanks for the review.
>>
>> On 9/6/2020 6:43 PM, Simon Glass wrote:
>>>>
>>>> diff --git a/Kconfig b/Kconfig
>>>> index 883e3f71d0..3959a6592c 100644
>>>> --- a/Kconfig
>>>> +++ b/Kconfig
>>>> @@ -533,6 +533,15 @@ config FIT_CIPHER
>>>> Enable the feature of data ciphering/unciphering in the tool
>>>> mkimage
>>>> and in the u-boot support of the FIT image.
>>>>
>>>> +config FIT_ARBP
>>>
>>> How about using ROLLBACK instead of ARBP. It is easier to understand.Looks
>>> good to me. I will change it in the next version of the patch.
>>
>>>> +{
>>>> + uint8_t type;
>>>> + uint32_t image_arbvn;
>>>> + uint32_t plat_arbvn = 0;
>>>
>>> Those three can be uint.
>> fit_image_get_type() returns type as uint8_t.
>> I can change it for the other two variables.
>>
>>>> static int fit_config_verify_sig(const void *fit, int conf_noffset,
>>>> const void *sig_blob, int sig_offset)
>>>> {
>>>> @@ -401,6 +472,14 @@ static int fit_config_verify_sig(const void *fit, int
>>>> conf_noffset,
>>>> goto error;
>>>> }
>>>>
>>>> +#if !defined(USE_HOSTCC)
>>>
>>> Do we need this £ifdef, or can we rely on IMAGE_ENABLE_ARBP?
>> I believe we can rely on just IMAGE_ENABLE_ARBP.
>>
>>>> #define FIT_LOAD_PROP "load"
>>>> +#define FIT_ARBVN_PROP "arbvn"
>>>
>>> ROLLBACK / "rollback"
>> I will fix it in the next version.
>>
>>>
>>>>
>>>> /* configuration node */
>>>> #define FIT_KERNEL_PROP"kernel"
>>>> @@ -1085,6 +1086,7 @@ int fit_image_get_data_size_unciphered(const void
>>>> *fit, int noffset,
>>>>size_t *data_size);
>>>> int fit_image_get_data_and_size(const void *fit, int noffset,
>>>> const void **data, size_t *size);
>>>> +int fit_image_get_arbvn(const void *fit, int noffset, uint32_t *arbvn);
>>>
>>> Please add a full function comment
>> comment was added before the function definition to be consistent
>> with other functions.
>>
>>>> +int board_get_arbvn(uint8_t ih_type, uint32_t *arbvn);
>>>
>>> This needs a driver since the rollback counter may be implemented by a
>>> TPM or anything.
>> Board specific hooks can leverage TPM library functions in that case.
>> May I know why a driver is needed?
>
> Sorry for not getting in to this series sooner. One thing that I think
> would be very helpful is to see is a full demonstration on say a
> Raspberry Pi. I know I have a TPM2 module that supports Pi sitting
> around here. I assume you've also tested this on some HW platform.
>
We test patches on our own hardware. But I agree demonstration of this
feature on more widely available hardware is useful. I will include
board (ex: Raspberry Pi) specific changes in the next version of the patch
series.
Best Regards,
Thiru
Re: [RFC PATCH 0/1] Anti rollback protection for FIT Images
On 9/7/2020 11:15 PM, Rasmus Villemoes wrote: > On 02/09/2020 09.58, Rasmus Villemoes wrote: >> On 01/09/2020 22.48, Thirupathaiah Annapureddy wrote: >>> Anti rollback protection is required when there is a need to retire >>> previous versions of FIT images due to security flaws in them. >>> Currently U-Boot Verified boot does not have rollback protection to >>> protect against known security flaws. >> >> This is definitely something we've had on our todo-list/wishlist. But we >> haven't had the time to sit down and work out the semantics and >> implementation, so thanks for doing this. > > ... > >> The board callbacks would simply be given a pointer to the data part of >> that node; that would make the versioning scheme rather flexible instead >> of being limited to a single monotonically increasing u32 (hence also >> the comparison logic should be in the board callbacks, instead of a >> "get/set" interface). > > Oh, and another reason for having the board callbacks being responsible > for the Yay/Nay verdict is that that makes it possible to hook up a gpio > that can be used to say "ignore rollback version check" - immensely > useful during development, and might also come in handy for the end > products. This is not a good idea from security point of view as that would lead to physical present attacks. > > Rasmus >
Re: [RFC PATCH 1/1] image: add anti rollback protection for FIT Images
Hi Simon,
Thanks for the review.
On 9/6/2020 6:43 PM, Simon Glass wrote:
>>
>> diff --git a/Kconfig b/Kconfig
>> index 883e3f71d0..3959a6592c 100644
>> --- a/Kconfig
>> +++ b/Kconfig
>> @@ -533,6 +533,15 @@ config FIT_CIPHER
>> Enable the feature of data ciphering/unciphering in the tool
>> mkimage
>> and in the u-boot support of the FIT image.
>>
>> +config FIT_ARBP
>
> How about using ROLLBACK instead of ARBP. It is easier to understand.Looks
> good to me. I will change it in the next version of the patch.
>> +{
>> + uint8_t type;
>> + uint32_t image_arbvn;
>> + uint32_t plat_arbvn = 0;
>
> Those three can be uint.
fit_image_get_type() returns type as uint8_t.
I can change it for the other two variables.
>> static int fit_config_verify_sig(const void *fit, int conf_noffset,
>> const void *sig_blob, int sig_offset)
>> {
>> @@ -401,6 +472,14 @@ static int fit_config_verify_sig(const void *fit, int
>> conf_noffset,
>> goto error;
>> }
>>
>> +#if !defined(USE_HOSTCC)
>
> Do we need this £ifdef, or can we rely on IMAGE_ENABLE_ARBP?
I believe we can rely on just IMAGE_ENABLE_ARBP.
>> #define FIT_LOAD_PROP "load"
>> +#define FIT_ARBVN_PROP "arbvn"
>
> ROLLBACK / "rollback"
I will fix it in the next version.
>
>>
>> /* configuration node */
>> #define FIT_KERNEL_PROP"kernel"
>> @@ -1085,6 +1086,7 @@ int fit_image_get_data_size_unciphered(const void
>> *fit, int noffset,
>>size_t *data_size);
>> int fit_image_get_data_and_size(const void *fit, int noffset,
>> const void **data, size_t *size);
>> +int fit_image_get_arbvn(const void *fit, int noffset, uint32_t *arbvn);
>
> Please add a full function comment
comment was added before the function definition to be consistent
with other functions.
>> +int board_get_arbvn(uint8_t ih_type, uint32_t *arbvn);
>
> This needs a driver since the rollback counter may be implemented by a
> TPM or anything.
Board specific hooks can leverage TPM library functions in that case.
May I know why a driver is needed?
> If you want to use the board, add a new
> get_rollback() to UCLASS_BOARD (board.h). Or you could create a new
> UCLASS_SECURITY which includes these two API calls.
I explored the option of using UCLASS_BOARD. But it does not have "set"
interfaces and the "id" parameter used in "get" functions seem to be
board specific. We can look into the option of UCLASS_SECURITY for these
two API calls.
>
> Also please update the vboot test to add a check for rollback.
Yes, will do in the next version of the patch series.
Best Regards,
Thiru
Re: [RFC PATCH 0/1] Anti rollback protection for FIT Images
Hi Rasmus,
Thanks for the review. Please see in-line:
On 9/2/2020 12:58 AM, Rasmus Villemoes wrote:
> On 01/09/2020 22.48, Thirupathaiah Annapureddy wrote:
>> Anti rollback protection is required when there is a need to retire
>> previous versions of FIT images due to security flaws in them.
>> Currently U-Boot Verified boot does not have rollback protection to
>> protect against known security flaws.
>
> This is definitely something we've had on our todo-list/wishlist. But we
> haven't had the time to sit down and work out the semantics and
> implementation, so thanks for doing this.
>
> I think most of this cover letter should be in the commit log instead.
Yes, will do in the formal patch proposal.
>
>> This RFC introduces a proposal to add anti-rollback protection for
>> FIT images. This protection feature prevents U-Boot from accepting
>> an image if it has ever successfully loaded an image with a larger
>> anti-rollback version number.
>>
>> Each sub-image node inside /images node has an
>> anti-rollback version number(arbvn) similar to rollback_index in
>> Android Verified Boot. This version number is part of signed data and
>> it is incremented as security flaws are discovered and fixed.
>> U-Boot stores the last seen arbvn for a given image type in platform
>> specific tamper-evident storage.
>
> Hmm. What is the purpose of per-image-type version numbers? That seems
> to be wrong. For example, we use the the "loadables" property in the
> U-Boot FIT to get the SPL to load some firmware blob to a known memory
> address where U-Boot proper then knows it to be. If we happened to have
> two such blobs for different pieces of hardware, they'd have the same
> "type" in the FIT image, but that doesn't mean they should follow the
> same versioning.
I received similar feedback internally to enhance the board specific hooks
to take sub-image node name as another input argument. Board specific code
can use this additional input to distinguish different images of the same
type.
>
> The way I imagined rollback protection to work (but I really haven't
> given this too much thought) was to have an extra property in the
> configuration:
>
> configurations {
> default = "conf-1";
> conf-1 {
> description = "...";
> kernel = "kernel-1";
> fdt = "fdt-1";
> ramdisk = "ramdisk-1";
> arvbn = "arvbn"; /* <-- */
>
> hash-1 {
> algo = "sha1";
> };
> signature-foo {
> algo = "sha1,rsa2048";
> sign-images = "kernel", "fdt", "ramdisk",
> "arvbn";
> key-name-hint = "foo";
> };
> };
> };
>
> with arvbn simply being a small extra "image" node
>
> arvbn {
> description = "BSP version";
> data = <0x02 0x03 0x01>;
> }
>
> (which should of course not be loaded to memory). This requires use of
> signed configurations rather than individually signed images, but that's
> anyway required for proper security.
The current proposal applies to both signed images and signed configurations.
>
> The board callbacks would simply be given a pointer to the data part of
> that node; that would make the versioning scheme rather flexible instead
> of being limited to a single monotonically increasing u32 (hence also
> the comparison logic should be in the board callbacks, instead of a
The comparison logic should be in the common code to enforce the checks in
a consistent way.
> "get/set" interface). For example, one could have a version composed as
> $major.$minor;$security, the board logic could prevent both downgrading
> to another major version and another security version. I.e., suppose
> we've had several BSP releases (in chronological order):
A BSP could be composed of multiple images and each of them will have their
own normal versioning scheme. Anti-rollback version number(arbvn) is an
additional complimentary version that can be incremented as
security flaws are discovered and fixed in a given image. This does not replace
existing versioning scheme for a given image.
>
> 1.0;0
> 1.1;0
> 2.0;0
> 1.2;0
> 2.1;0
>
> 1.3;1
> 2.2;1
> 2.3;1
> 1.4;1
>
> 2.4;2
> 1.5;2
>
> With this, a us
[RFC PATCH 0/1] Anti rollback protection for FIT Images
Anti rollback protection is required when there is a need to retire
previous versions of FIT images due to security flaws in them.
Currently U-Boot Verified boot does not have rollback protection to
protect against known security flaws.
This RFC introduces a proposal to add anti-rollback protection for
FIT images. This protection feature prevents U-Boot from accepting
an image if it has ever successfully loaded an image with a larger
anti-rollback version number.
Each sub-image node inside /images node has an
anti-rollback version number(arbvn) similar to rollback_index in
Android Verified Boot. This version number is part of signed data and
it is incremented as security flaws are discovered and fixed.
U-Boot stores the last seen arbvn for a given image type in platform
specific tamper-evident storage.
As part of signature verification, U-Boot enfroces arvbn based
protection if enabled. arvbn stored in secure storage is validated with
arbvn in the sub-image node. If the counter in the FIT image is lower than
the counter in platform secure storage, image validation has failed
i.e. verified boot failed. If both counters match or the image counter is
higher than that in the platform secure storage, the image validation is
successful. In the higher case, U-Boot stores the new counter in platform
secure storage.
Pseudo code is as follows:
ret = board_get_arbvn(type, _arbvn);
...
if (image_arbvn < plat_arbvn) {
return -EPERM;
} else if (image_arbvn > plat_arbvn) {
ret = board_set_arbvn(type, image_arbvn);
return ret;
} else {
return 0;
}
The following board specific hooks are required to get/set arbvn
from platform specific tamper-evident storage.
int board_get_arbvn(uint8_t ih_type, uint32_t *arbvn);
int board_set_arbvn(uint8_t ih_type, uint32_t arbvn);
As an example, consider this FIT:
/ {
images {
kernel-1 {
data =
arbvn = <1>;
hash-1 {
algo = "sha1";
value = <...kernel hash 1...>
};
};
fdt-1 {
data = ;
hash-1 {
algo = "sha1";
value = <...fdt hash 1...>
};
};
};
configurations {
default = "conf-1";
conf-1 {
kernel = "kernel-1";
fdt = "fdt-1";
signature-1 {
algo = "sha1,rsa2048";
sign-images = "fdt", "kernel";
value = <...conf 1 signature...>;
};
};
};
};
In the above example, kernel-1 image has an arbvn of 1.
if plat_arbvn is 1, the system will boot with this FIT image.
if plat_arbvn is 2 or more, U-Boot will prevent the system from booting
with this FIT image.
Thirupathaiah Annapureddy (1):
image: add anti rollback protection for FIT Images
Kconfig| 9 +
common/image-fit-sig.c | 79 ++
common/image-fit.c | 24 +
include/image.h| 23
4 files changed, 135 insertions(+)
--
2.25.2
[RFC PATCH 1/1] image: add anti rollback protection for FIT Images
Anti rollback protection is required when there is a need to retire
previous versions of FIT images due to security flaws in them.
Currently U-Boot Verified boot does not have rollback protection to
protect against known security flaws.
This change adds anti-rollback protection for FIT images.
Signed-off-by: Thirupathaiah Annapureddy
---
Kconfig| 9 +
common/image-fit-sig.c | 79 ++
common/image-fit.c | 24 +
include/image.h| 23
4 files changed, 135 insertions(+)
diff --git a/Kconfig b/Kconfig
index 883e3f71d0..3959a6592c 100644
--- a/Kconfig
+++ b/Kconfig
@@ -533,6 +533,15 @@ config FIT_CIPHER
Enable the feature of data ciphering/unciphering in the tool mkimage
and in the u-boot support of the FIT image.
+config FIT_ARBP
+ bool "Enable Anti rollback version check for FIT images"
+ depends on FIT_SIGNATURE
+ default n
+ help
+ Enable this to activate anti-rollback protection. This is required
+ when a platform needs to retire previous versions of FIT images due to
+ security flaws in in them.
+
config FIT_VERBOSE
bool "Show verbose messages when FIT images fail"
help
diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
index cc1967109e..5103508894 100644
--- a/common/image-fit-sig.c
+++ b/common/image-fit-sig.c
@@ -78,6 +78,35 @@ struct image_region *fit_region_make_list(const void *fit,
return region;
}
+#if !defined(USE_HOSTCC)
+static int fit_image_verify_arbvn(const void *fit, int image_noffset)
+{
+ uint8_t type;
+ uint32_t image_arbvn;
+ uint32_t plat_arbvn = 0;
+ int ret;
+
+ ret = fit_image_get_arbvn(fit, image_noffset, _arbvn);
+ if (ret)
+ return 0;
+
+ ret = fit_image_get_type(fit, image_noffset, );
+ if (ret)
+ return 0;
+
+ ret = board_get_arbvn(type, _arbvn);
+
+ if (image_arbvn < plat_arbvn) {
+ return -EPERM;
+ } else if (image_arbvn > plat_arbvn) {
+ ret = board_set_arbvn(type, image_arbvn);
+ return ret;
+ }
+
+ return 0;
+}
+#endif
+
static int fit_image_setup_verify(struct image_sign_info *info,
const void *fit, int noffset,
int required_keynode, char **err_msgp)
@@ -181,6 +210,14 @@ static int fit_image_verify_sig(const void *fit, int
image_noffset,
goto error;
}
+#if !defined(USE_HOSTCC)
+ if (IMAGE_ENABLE_ARBP && verified) {
+ ret = fit_image_verify_arbvn(fit, image_noffset);
+ if (ret)
+ verified = 0;
+ }
+#endif
+
return verified ? 0 : -EPERM;
error:
@@ -370,6 +407,40 @@ static int fit_config_check_sig(const void *fit, int
noffset,
return 0;
}
+#if !defined(USE_HOSTCC)
+static int fit_config_verify_arbvn(const void *fit, int conf_noffset,
+ int sig_offset)
+{
+ static const char default_list[] = FIT_KERNEL_PROP "\0"
+ FIT_FDT_PROP;
+ int ret, len;
+ const char *prop, *iname, *end;
+ int image_noffset;
+
+ /* If there is "sign-images" property, use that */
+ prop = fdt_getprop(fit, sig_offset, "sign-images", );
+ if (!prop) {
+ prop = default_list;
+ len = sizeof(default_list);
+ }
+
+ /* Locate the images */
+ end = prop + len;
+ for (iname = prop; iname < end; iname += strlen(iname) + 1) {
+ image_noffset = fit_conf_get_prop_node(fit, conf_noffset,
+ iname);
+ if (image_noffset < 0)
+ return -ENOENT;
+
+ ret = fit_image_verify_arbvn(fit, image_noffset);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+#endif
+
static int fit_config_verify_sig(const void *fit, int conf_noffset,
const void *sig_blob, int sig_offset)
{
@@ -401,6 +472,14 @@ static int fit_config_verify_sig(const void *fit, int
conf_noffset,
goto error;
}
+#if !defined(USE_HOSTCC)
+ if (IMAGE_ENABLE_ARBP && verified) {
+ ret = fit_config_verify_arbvn(fit, conf_noffset, noffset);
+ if (ret)
+ verified = 0;
+ }
+#endif
+
if (verified)
return 0;
diff --git a/common/image-fit.c b/common/image-fit.c
index d54eff9033..97029853b9 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1025,6 +1025,30 @@ int fit_image_get_data_and_size(const void *fit, int
noffset,
return ret;
}
+/**
+ * Get 'arbvn' property from a given image node.
+ *
+ * @
[PATCH v2] arm: dts: fix ast2500-evb inclusion for the correct soc family
Include ast2500-evb.dtb for CONFIG_ASPEED_AST2500 instead of for all aspeed targets. ast2400 is based on ARM926EJ-S processor (ARMv5-architecture). ast2500 is based on ARM1176JZS processor (ARMv6-architecture). ast2600 is based on Cortex A7 processor (ARMv7-A architecture). Each of the above SOC is using a different ARM CPU(s) with different ARM architecture revision. It is not possible to support all 3 of these families in a single binary. So there is no need to build ast2500-evb.dtb for other SOC families. Signed-off-by: Thirupathaiah Annapureddy --- Changes in v2: - Incorporated the feedback from Tom Rini and Ryan Chen. arch/arm/dts/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 7e29b9096b..33d40a05f9 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -938,7 +938,7 @@ dtb-$(CONFIG_ARCH_BCM6858) += \ dtb-$(CONFIG_TARGET_BCMNS3) += ns3-board.dtb -dtb-$(CONFIG_ARCH_ASPEED) += ast2500-evb.dtb +dtb-$(CONFIG_ASPEED_AST2500) += ast2500-evb.dtb dtb-$(CONFIG_ARCH_STI) += stih410-b2260.dtb -- 2.25.2
Re: [PATCH] arm: dts: fix ast2500-evb inclusion for correct target
On 8/20/2020 5:03 PM, Tom Rini wrote: > On Thu, Aug 20, 2020 at 02:09:55PM -0700, Thirupathaiah Annapureddy wrote: >> Hi Ryan, >> >> Thanks for reviewing the patch. Please see my comment(s): >> >> On 8/19/2020 7:06 PM, Ryan Chen wrote: >>> Hi >>>> -Original Message- >>>> From: Thirupathaiah Annapureddy [mailto:thir...@linux.microsoft.com] >>>> Sent: Thursday, August 20, 2020 8:16 AM >>>> To: u-boot@lists.denx.de >>>> Cc: Maxim Sloyko ; Marek Vasut ; >>>> ChiaWei Wang ; Ryan Chen >>>> >>>> Subject: Re: [PATCH] arm: dts: fix ast2500-evb inclusion for correct target >>>> >>>> Adding Ryan and Chiawei to the list. >>>> >>>> On 8/17/2020 5:53 PM, Thirupathaiah Annapureddy wrote: >>>>> Include ast2500-evb.dtb for CONFIG_TARGET_EVB_AST2500 instead of for >>>>> all aspeed targets. >>> >>> There should not have to many Kconfig for ASPEED platform. >> When you build U-Boot, you have to provide a platform specific defconfig >> as the target. ex: evb-ast2500_defconfig. >> defconfig specifies the platform specific device tree file. >> ex: CONFIG_DEFAULT_DEVICE_TREE="ast2500-evb" >> >> I do not see any reason to make other device trees (ex: ast2600a0-evb.dtb) >> when we are building for evb-ast2500. > > It keeps the Makefile logic clearer and makes future moves towards more > platforms in a single binary easier if we just build all of the dtb > files. > ast2400 is based on ARM926EJ-S processor (ARMv5-architecture). ast2500 is based on ARM1176JZS processor (ARMv6-architecture). ast2600 is based on Cortex A7 processor (ARMv7-A architecture). Each of the above SOC is using a different ARM CPU(s) with different ARM architecture revision. Is it possible for single binary to support multiple platforms based on above SOCs?
Re: [PATCH] arm: dts: fix ast2500-evb inclusion for correct target
Hi Ryan, Thanks for reviewing the patch. Please see my comment(s): On 8/19/2020 7:06 PM, Ryan Chen wrote: > Hi >> -Original Message- >> From: Thirupathaiah Annapureddy [mailto:thir...@linux.microsoft.com] >> Sent: Thursday, August 20, 2020 8:16 AM >> To: u-boot@lists.denx.de >> Cc: Maxim Sloyko ; Marek Vasut ; >> ChiaWei Wang ; Ryan Chen >> >> Subject: Re: [PATCH] arm: dts: fix ast2500-evb inclusion for correct target >> >> Adding Ryan and Chiawei to the list. >> >> On 8/17/2020 5:53 PM, Thirupathaiah Annapureddy wrote: >>> Include ast2500-evb.dtb for CONFIG_TARGET_EVB_AST2500 instead of for >>> all aspeed targets. > > There should not have to many Kconfig for ASPEED platform. When you build U-Boot, you have to provide a platform specific defconfig as the target. ex: evb-ast2500_defconfig. defconfig specifies the platform specific device tree file. ex: CONFIG_DEFAULT_DEVICE_TREE="ast2500-evb" I do not see any reason to make other device trees (ex: ast2600a0-evb.dtb) when we are building for evb-ast2500. > I prefer use following to build all all ASPEED platform. Like following. > dtb-$(CONFIG_ARCH_ASPEED) += \ > ast2400-evb.dtb \ > ast2500-evb.dtb \ > ast2600a0-evb.dtb \ > ast2600a0-ncsi.dtb \ > ast2600a1-evb.dtb \ > ast2600a1-ncsi.dtb \ > ast2600-fpga.dtb \ > ast2600-rainier.dtb \ > ast2600-slt.dtb \ > ast2600-tacoma.dtb > >>> >>> Signed-off-by: Thirupathaiah Annapureddy >> --- >>> arch/arm/dts/Makefile | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index >>> 7e29b9096b..d019f26983 100644 >>> --- a/arch/arm/dts/Makefile >>> +++ b/arch/arm/dts/Makefile >>> @@ -938,7 +938,7 @@ dtb-$(CONFIG_ARCH_BCM6858) += \ >>> >>> dtb-$(CONFIG_TARGET_BCMNS3) += ns3-board.dtb >>> >>> -dtb-$(CONFIG_ARCH_ASPEED) += ast2500-evb.dtb >>> +dtb-$(CONFIG_TARGET_EVB_AST2500) += ast2500-evb.dtb >>> >>> dtb-$(CONFIG_ARCH_STI) += stih410-b2260.dtb >>> >>> > Best Regards, Thiru
Re: [PATCH] arm: dts: fix ast2500-evb inclusion for correct target
Adding Ryan and Chiawei to the list. On 8/17/2020 5:53 PM, Thirupathaiah Annapureddy wrote: > Include ast2500-evb.dtb for CONFIG_TARGET_EVB_AST2500 instead of > for all aspeed targets. > > Signed-off-by: Thirupathaiah Annapureddy --- > arch/arm/dts/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > index 7e29b9096b..d019f26983 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -938,7 +938,7 @@ dtb-$(CONFIG_ARCH_BCM6858) += \ > > dtb-$(CONFIG_TARGET_BCMNS3) += ns3-board.dtb > > -dtb-$(CONFIG_ARCH_ASPEED) += ast2500-evb.dtb > +dtb-$(CONFIG_TARGET_EVB_AST2500) += ast2500-evb.dtb > > dtb-$(CONFIG_ARCH_STI) += stih410-b2260.dtb > >
[PATCH] arm: dts: fix ast2500-evb inclusion for correct target
Include ast2500-evb.dtb for CONFIG_TARGET_EVB_AST2500 instead of for all aspeed targets. Signed-off-by: Thirupathaiah Annapureddy --- arch/arm/dts/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 7e29b9096b..d019f26983 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -938,7 +938,7 @@ dtb-$(CONFIG_ARCH_BCM6858) += \ dtb-$(CONFIG_TARGET_BCMNS3) += ns3-board.dtb -dtb-$(CONFIG_ARCH_ASPEED) += ast2500-evb.dtb +dtb-$(CONFIG_TARGET_EVB_AST2500) += ast2500-evb.dtb dtb-$(CONFIG_ARCH_STI) += stih410-b2260.dtb -- 2.25.2
[PATCH] include: phy: fix NULL pointer check in phy_write()
phy_write() uses bus->write() instead of bus->read(). This means NULL
pointer pre-check needs to happen on bus->write instead of bus->read.
Signed-off-by: Thirupathaiah Annapureddy
---
include/phy.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/phy.h b/include/phy.h
index 1dbbf65111..cbdb10d6fc 100644
--- a/include/phy.h
+++ b/include/phy.h
@@ -205,7 +205,7 @@ static inline int phy_write(struct phy_device *phydev, int
devad, int regnum,
{
struct mii_dev *bus = phydev->bus;
- if (!bus || !bus->read) {
+ if (!bus || !bus->write) {
debug("%s: No bus configured\n", __func__);
return -1;
}
--
2.25.2
[PATCH] net: ftgmac100: Add support for board specific PHY interface address
ftgmac100 driver is using hard-coded PHY interface address of zero. Each board can have different PHY interface address (phy_addr). This commit modifies the driver to make use of board specific address by leveraging CONFIG_PHY_ADDR. Signed-off-by: Thirupathaiah Annapureddy --- drivers/net/ftgmac100.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c index 5676a5b3ba..00bda24f1f 100644 --- a/drivers/net/ftgmac100.c +++ b/drivers/net/ftgmac100.c @@ -551,6 +551,10 @@ static int ftgmac100_probe(struct udevice *dev) priv->max_speed = pdata->max_speed; priv->phy_addr = 0; +#ifdef CONFIG_PHY_ADDR + priv->phy_addr = CONFIG_PHY_ADDR; +#endif + ret = clk_enable_bulk(>clks); if (ret) goto out; -- 2.25.2
[PATCH v3 1/3] vboot: add DTB policy for supporting multiple required conf keys
Currently FIT image must be signed by all required conf keys. This means
Verified Boot fails if there is a signature verification failure
using any required key in U-Boot DTB.
This patch introduces a new policy in DTB that can be set to any required
conf key. This means if verified boot passes with one of the required
keys, U-Boot will continue the OS hand off.
There were prior attempts to address this:
https://lists.denx.de/pipermail/u-boot/2019-April/366047.html
The above patch was failing "make tests".
https://lists.denx.de/pipermail/u-boot/2020-January/396629.html
Signed-off-by: Thirupathaiah Annapureddy
Reviewed-by: Simon Glass
---
Changes in v3:
- Replaced 'u-boot' with 'U-Boot' in commit message.
- Added an explicit print message to indicate that no required signature
was verified.
Changes in v2:
- Modify fit_config_verify_required_sigs() to process required-mode
policy variable in U-boot DTB.
common/image-fit-sig.c | 32 +---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
index cc1967109e..5401d9411b 100644
--- a/common/image-fit-sig.c
+++ b/common/image-fit-sig.c
@@ -416,6 +416,10 @@ int fit_config_verify_required_sigs(const void *fit, int
conf_noffset,
{
int noffset;
int sig_node;
+ int verified = 0;
+ int reqd_sigs = 0;
+ bool reqd_policy_all = true;
+ const char *reqd_mode;
/* Work out what we need to verify */
sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME);
@@ -425,6 +429,14 @@ int fit_config_verify_required_sigs(const void *fit, int
conf_noffset,
return 0;
}
+ /* Get required-mode policy property from DTB */
+ reqd_mode = fdt_getprop(sig_blob, sig_node, "required-mode", NULL);
+ if (reqd_mode && !strcmp(reqd_mode, "any"))
+ reqd_policy_all = false;
+
+ debug("%s: required-mode policy set to '%s'\n", __func__,
+ reqd_policy_all ? "all" : "any");
+
fdt_for_each_subnode(noffset, sig_blob, sig_node) {
const char *required;
int ret;
@@ -433,15 +445,29 @@ int fit_config_verify_required_sigs(const void *fit, int
conf_noffset,
NULL);
if (!required || strcmp(required, "conf"))
continue;
+
+ reqd_sigs++;
+
ret = fit_config_verify_sig(fit, conf_noffset, sig_blob,
noffset);
if (ret) {
- printf("Failed to verify required signature '%s'\n",
- fit_get_name(sig_blob, noffset, NULL));
- return ret;
+ if (reqd_policy_all) {
+ printf("Failed to verify required signature
'%s'\n",
+ fit_get_name(sig_blob, noffset, NULL));
+ return ret;
+ }
+ } else {
+ verified++;
+ if (!reqd_policy_all)
+ break;
}
}
+ if (reqd_sigs && !verified) {
+ printf("Failed to verify 'any' of the required signature(s)\n");
+ return -EPERM;
+ }
+
return 0;
}
--
2.25.2
[PATCH v3 2/3] test: vboot: add tests for multiple required keys
This patch adds vboot tests to verify the support for multiple
required keys using new required-mode DTB policy.
This patch also fixes existing test where dev
key is assumed to be marked as not required, although
it is marked as required.
Note that this patch re-added sign_fit_norequire().
sign_fit_norequire() was removed as part of the following:
commit b008677daf2a ("test: vboot: Fix pylint errors").
This patch leverages sign_fit_norequire() to fix the
existing bug.
Signed-off-by: Thirupathaiah Annapureddy
Reviewed-by: Simon Glass
---
Changes in v3:
- Modified commit message to reference earlier commit the right way.
Changes in v2:
- Added tests to cover any or all required keys policy.
test/py/tests/test_vboot.py | 46 +++--
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 6b998cfd70..e45800d94c 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -126,6 +126,23 @@ def test_vboot(u_boot_console, sha_algo, padding,
sign_options, required):
cons.log.action('%s: Sign images' % sha_algo)
util.run_and_log(cons, args)
+def sign_fit_norequire(sha_algo, options):
+"""Sign the FIT
+
+Signs the FIT and writes the signature into it. It also writes the
+public key into the dtb. It does not mark key as 'required' in dtb.
+
+Args:
+sha_algo: Either 'sha1' or 'sha256', to select the algorithm to
+use.
+options: Options to provide to mkimage.
+"""
+args = [mkimage, '-F', '-k', tmpdir, '-K', dtb, fit]
+if options:
+args += options.split(' ')
+cons.log.action('%s: Sign images' % sha_algo)
+util.run_and_log(cons, args)
+
def replace_fit_totalsize(size):
"""Replace FIT header's totalsize with something greater.
@@ -279,15 +296,40 @@ def test_vboot(u_boot_console, sha_algo, padding,
sign_options, required):
# Build the FIT with dev key (keys NOT required). This adds the
# signature into sandbox-u-boot.dtb, NOT marked 'required'.
make_fit('sign-configs-%s%s.its' % (sha_algo, padding))
-sign_fit(sha_algo, sign_options)
+sign_fit_norequire(sha_algo, sign_options)
# So now sandbox-u-boot.dtb two signatures, for the prod and dev keys.
# Only the prod key is set as 'required'. But FIT we just built has
-# a dev signature only (sign_fit() overwrites the FIT).
+# a dev signature only (sign_fit_norequire() overwrites the FIT).
# Try to boot the FIT with dev key. This FIT should not be accepted by
# U-Boot because the prod key is required.
run_bootm(sha_algo, 'required key', '', False)
+# Build the FIT with dev key (keys required) and sign it. This puts the
+# signature into sandbox-u-boot.dtb, marked 'required'.
+make_fit('sign-configs-%s%s.its' % (sha_algo, padding))
+sign_fit(sha_algo, sign_options)
+
+# Set the required-mode policy to "any".
+# So now sandbox-u-boot.dtb two signatures, for the prod and dev keys.
+# Both the dev and prod key are set as 'required'. But FIT we just
built has
+# a dev signature only (sign_fit() overwrites the FIT).
+# Try to boot the FIT with dev key. This FIT should be accepted by
+# U-Boot because the dev key is required and policy is "any" required
key.
+util.run_and_log(cons, 'fdtput -t s %s /signature required-mode any' %
+ (dtb))
+run_bootm(sha_algo, 'multi required key', 'dev+', True)
+
+# Set the required-mode policy to "all".
+# So now sandbox-u-boot.dtb two signatures, for the prod and dev keys.
+# Both the dev and prod key are set as 'required'. But FIT we just
built has
+# a dev signature only (sign_fit() overwrites the FIT).
+# Try to boot the FIT with dev key. This FIT should not be accepted by
+# U-Boot because the prod key is required and policy is "all" required
key
+util.run_and_log(cons, 'fdtput -t s %s /signature required-mode all' %
+ (dtb))
+run_bootm(sha_algo, 'multi required key', '', False)
+
cons = u_boot_console
tmpdir = cons.config.result_dir + '/'
datadir = cons.config.source_dir + '/test/py/tests/vboot/'
--
2.25.2
[PATCH v3 3/3] doc: verified-boot: add required-mode information
Add documentation about 'required-mode' property in /signature node in U-Boot's control FDT. Signed-off-by: Thirupathaiah Annapureddy Reviewed-by: Simon Glass --- Changes in v3: - Added commit description to address checkpatch warning. Changes in v2: - New. doc/uImage.FIT/signature.txt | 14 ++ 1 file changed, 14 insertions(+) diff --git a/doc/uImage.FIT/signature.txt b/doc/uImage.FIT/signature.txt index d4afd755e9..a3455889ed 100644 --- a/doc/uImage.FIT/signature.txt +++ b/doc/uImage.FIT/signature.txt @@ -386,6 +386,20 @@ that might be used by the target needs to be signed with 'required' keys. This happens automatically as part of a bootm command when FITs are used. +For Signed Configurations, the default verification behavior can be changed by +the following optional property in /signature node in U-Boot's control FDT. + +- required-mode: Valid values are "any" to allow verified boot to succeed if +the selected configuration is signed by any of the 'required' keys, and "all" +to allow verified boot to succeed if the selected configuration is signed by +all of the 'required' keys. + +This property can be added to a binary device tree using fdtput as shown in +below examples:: + + fdtput -t s control.dtb /signature required-mode any + fdtput -t s control.dtb /signature required-mode all + Enabling FIT Verification - -- 2.25.2
[PATCH v3 0/3] Add support for multiple required keys
This patch series adds the support for multiple required keys in U-Boot DTB with test support. Changes in v3: - Replaced 'u-boot' with 'U-Boot' in commit messages. - Added an explicit print message to indicate that no required signature was verified. Changes in v2 (thanks for the feedback Simon and Rasmus): - Introduce a policy variable in U-boot DTB to control whether any or all required keys must have signed configuration. - Added tests to cover any or all required keys policy. - Updated signature.txt to include required-mode policy information. Thirupathaiah Annapureddy (3): vboot: add DTB policy for supporting multiple required conf keys test: vboot: add tests for multiple required keys doc: verified-boot: add required-mode information common/image-fit-sig.c | 32 ++--- doc/uImage.FIT/signature.txt | 14 +++ test/py/tests/test_vboot.py | 46 ++-- 3 files changed, 87 insertions(+), 5 deletions(-) -- 2.25.2
Re: [PATCH v2 3/3] doc: verified-boot: add required-mode information
On 7/28/2020 11:58 AM, Simon Glass wrote: > Hi Thirupathaiah, > > On Fri, 17 Jul 2020 at 21:20, Thirupathaiah Annapureddy > wrote: >> >> Signed-off-by: Thirupathaiah Annapureddy >> --- >> >> Changes in v2: >> - New >> >> doc/uImage.FIT/signature.txt | 14 ++ >> 1 file changed, 14 insertions(+) >> > > Reviewed-by: Simon Glass > > But I think we need a new mkimage option to set the required-mode Is it okay if I do mkimage option change as part of a different patch/ patch series? > > >> diff --git a/doc/uImage.FIT/signature.txt b/doc/uImage.FIT/signature.txt >> index d4afd755e9..a3455889ed 100644 >> --- a/doc/uImage.FIT/signature.txt >> +++ b/doc/uImage.FIT/signature.txt >> @@ -386,6 +386,20 @@ that might be used by the target needs to be signed >> with 'required' keys. >> >> This happens automatically as part of a bootm command when FITs are used. >> >> +For Signed Configurations, the default verification behavior can be changed >> by >> +the following optional property in /signature node in U-Boot's control FDT. >> + >> +- required-mode: Valid values are "any" to allow verified boot to succeed if >> +the selected configuration is signed by any of the 'required' keys, and >> "all" >> +to allow verified boot to succeed if the selected configuration is signed by >> +all of the 'required' keys. >> + >> +This property can be added to a binary device tree using fdtput as shown in >> +below examples:: >> + >> + fdtput -t s control.dtb /signature required-mode any >> + fdtput -t s control.dtb /signature required-mode all >> + >> >> Enabling FIT Verification >> - >> -- >> 2.25.2 >>
[PATCH v2 2/3] test: vboot: add tests for multiple required keys
This patch adds vboot tests to verify the support for multiple
required keys using new required-mode DTB policy.
This patch also fixes existing test where dev
key is assumed to be marked as not required, although
it is marked as required.
Note that this patch re-added sign_fit_norequire().
sign_fit_norequire() was removed as part of the commit:
b008677daf2a9dc0335260c7c4e24390487fe0ca
(test: vboot: Fix pylint errors)
This patch leverages sign_fit_norequire() to fix the
existing bug.
Signed-off-by: Thirupathaiah Annapureddy
---
Changes in v2:
- Added tests to cover any or all required keys policy.
test/py/tests/test_vboot.py | 46 +++--
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 6b998cfd70..e45800d94c 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -126,6 +126,23 @@ def test_vboot(u_boot_console, sha_algo, padding,
sign_options, required):
cons.log.action('%s: Sign images' % sha_algo)
util.run_and_log(cons, args)
+def sign_fit_norequire(sha_algo, options):
+"""Sign the FIT
+
+Signs the FIT and writes the signature into it. It also writes the
+public key into the dtb. It does not mark key as 'required' in dtb.
+
+Args:
+sha_algo: Either 'sha1' or 'sha256', to select the algorithm to
+use.
+options: Options to provide to mkimage.
+"""
+args = [mkimage, '-F', '-k', tmpdir, '-K', dtb, fit]
+if options:
+args += options.split(' ')
+cons.log.action('%s: Sign images' % sha_algo)
+util.run_and_log(cons, args)
+
def replace_fit_totalsize(size):
"""Replace FIT header's totalsize with something greater.
@@ -279,15 +296,40 @@ def test_vboot(u_boot_console, sha_algo, padding,
sign_options, required):
# Build the FIT with dev key (keys NOT required). This adds the
# signature into sandbox-u-boot.dtb, NOT marked 'required'.
make_fit('sign-configs-%s%s.its' % (sha_algo, padding))
-sign_fit(sha_algo, sign_options)
+sign_fit_norequire(sha_algo, sign_options)
# So now sandbox-u-boot.dtb two signatures, for the prod and dev keys.
# Only the prod key is set as 'required'. But FIT we just built has
-# a dev signature only (sign_fit() overwrites the FIT).
+# a dev signature only (sign_fit_norequire() overwrites the FIT).
# Try to boot the FIT with dev key. This FIT should not be accepted by
# U-Boot because the prod key is required.
run_bootm(sha_algo, 'required key', '', False)
+# Build the FIT with dev key (keys required) and sign it. This puts the
+# signature into sandbox-u-boot.dtb, marked 'required'.
+make_fit('sign-configs-%s%s.its' % (sha_algo, padding))
+sign_fit(sha_algo, sign_options)
+
+# Set the required-mode policy to "any".
+# So now sandbox-u-boot.dtb two signatures, for the prod and dev keys.
+# Both the dev and prod key are set as 'required'. But FIT we just
built has
+# a dev signature only (sign_fit() overwrites the FIT).
+# Try to boot the FIT with dev key. This FIT should be accepted by
+# U-Boot because the dev key is required and policy is "any" required
key.
+util.run_and_log(cons, 'fdtput -t s %s /signature required-mode any' %
+ (dtb))
+run_bootm(sha_algo, 'multi required key', 'dev+', True)
+
+# Set the required-mode policy to "all".
+# So now sandbox-u-boot.dtb two signatures, for the prod and dev keys.
+# Both the dev and prod key are set as 'required'. But FIT we just
built has
+# a dev signature only (sign_fit() overwrites the FIT).
+# Try to boot the FIT with dev key. This FIT should not be accepted by
+# U-Boot because the prod key is required and policy is "all" required
key
+util.run_and_log(cons, 'fdtput -t s %s /signature required-mode all' %
+ (dtb))
+run_bootm(sha_algo, 'multi required key', '', False)
+
cons = u_boot_console
tmpdir = cons.config.result_dir + '/'
datadir = cons.config.source_dir + '/test/py/tests/vboot/'
--
2.25.2
[PATCH v2 3/3] doc: verified-boot: add required-mode information
Signed-off-by: Thirupathaiah Annapureddy --- Changes in v2: - New doc/uImage.FIT/signature.txt | 14 ++ 1 file changed, 14 insertions(+) diff --git a/doc/uImage.FIT/signature.txt b/doc/uImage.FIT/signature.txt index d4afd755e9..a3455889ed 100644 --- a/doc/uImage.FIT/signature.txt +++ b/doc/uImage.FIT/signature.txt @@ -386,6 +386,20 @@ that might be used by the target needs to be signed with 'required' keys. This happens automatically as part of a bootm command when FITs are used. +For Signed Configurations, the default verification behavior can be changed by +the following optional property in /signature node in U-Boot's control FDT. + +- required-mode: Valid values are "any" to allow verified boot to succeed if +the selected configuration is signed by any of the 'required' keys, and "all" +to allow verified boot to succeed if the selected configuration is signed by +all of the 'required' keys. + +This property can be added to a binary device tree using fdtput as shown in +below examples:: + + fdtput -t s control.dtb /signature required-mode any + fdtput -t s control.dtb /signature required-mode all + Enabling FIT Verification - -- 2.25.2
[PATCH v2 0/3] Add support for multiple required keys
This patch series adds the support for multiple required keys in U-Boot DTB with test support. Changes in v2 (thanks for the feedback Simon and Rasmus): - Introduce a policy variable in U-boot DTB to control whether any or all required keys must have signed configuration. - Added tests to cover any or all required keys policy. - Updated signature.txt to include required-mode policy information. Thirupathaiah Annapureddy (3): vboot: add DTB policy for supporting multiple required conf keys test: vboot: add tests for multiple required keys doc: verified-boot: add required-mode information common/image-fit-sig.c | 30 --- doc/uImage.FIT/signature.txt | 14 +++ test/py/tests/test_vboot.py | 46 ++-- 3 files changed, 85 insertions(+), 5 deletions(-) -- 2.25.2
[PATCH v2 1/3] vboot: add DTB policy for supporting multiple required conf keys
Currently FIT image must be signed by all required conf keys. This means
Verified Boot fails if there is a signature verification failure
using any required key in U-boot DTB.
This patch introduces a new policy in DTB that can be set to any required
conf key. This means if verified boot passes with one of the required
keys, u-boot will continue the OS hand off.
There were prior attempts to address this:
https://lists.denx.de/pipermail/u-boot/2019-April/366047.html
The above patch was failing "make tests".
https://lists.denx.de/pipermail/u-boot/2020-January/396629.html
Signed-off-by: Thirupathaiah Annapureddy
---
Changes in v2:
- Modify fit_config_verify_required_sigs() to process required-mode
policy variable in U-boot DTB.
common/image-fit-sig.c | 30 +++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
index cc1967109e..d9b1c93a9b 100644
--- a/common/image-fit-sig.c
+++ b/common/image-fit-sig.c
@@ -416,6 +416,10 @@ int fit_config_verify_required_sigs(const void *fit, int
conf_noffset,
{
int noffset;
int sig_node;
+ int verified = 0;
+ int reqd_sigs = 0;
+ bool reqd_policy_all = true;
+ const char *reqd_mode;
/* Work out what we need to verify */
sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME);
@@ -425,6 +429,14 @@ int fit_config_verify_required_sigs(const void *fit, int
conf_noffset,
return 0;
}
+ /* Get required-mode policy property from DTB */
+ reqd_mode = fdt_getprop(sig_blob, sig_node, "required-mode", NULL);
+ if (reqd_mode && !strcmp(reqd_mode, "any"))
+ reqd_policy_all = false;
+
+ debug("%s: required-mode policy set to '%s'\n", __func__,
+ reqd_policy_all ? "all" : "any");
+
fdt_for_each_subnode(noffset, sig_blob, sig_node) {
const char *required;
int ret;
@@ -433,15 +445,27 @@ int fit_config_verify_required_sigs(const void *fit, int
conf_noffset,
NULL);
if (!required || strcmp(required, "conf"))
continue;
+
+ reqd_sigs++;
+
ret = fit_config_verify_sig(fit, conf_noffset, sig_blob,
noffset);
if (ret) {
- printf("Failed to verify required signature '%s'\n",
- fit_get_name(sig_blob, noffset, NULL));
- return ret;
+ if (reqd_policy_all) {
+ printf("Failed to verify required signature
'%s'\n",
+ fit_get_name(sig_blob, noffset, NULL));
+ return ret;
+ }
+ } else {
+ verified++;
+ if (!reqd_policy_all)
+ break;
}
}
+ if (reqd_sigs && !verified)
+ return -EPERM;
+
return 0;
}
--
2.25.2
Re: [PATCH 1/2] vboot: add support for multiple required keys
Hi Simon, Thanks a lot for reviewing the patch. I would appreciate if you could clarify the following in-line questions: On 6/29/2020 10:31 AM, Simon Glass wrote: > Hi Thirupathaiah, > > > On Mon, 29 Jun 2020 at 11:26, Simon Glass wrote: >> >> Hi Thirupathaiah, >> >> On Thu, 25 Jun 2020 at 09:51, Thirupathaiah Annapureddy >> wrote: >>> >>> Currently Verified Boot fails if there is a signature verification failure >>> using required key in U-boot DTB. This patch adds support for multiple >>> required keys. This means if verified boot passes with one of the required >>> keys, u-boot will continue the OS hand off. >>> >>> There was a prior attempt to resolve this with the following patch: >>> https://lists.denx.de/pipermail/u-boot/2019-April/366047.html >>> The above patch was failing "make tests". >>> >>> Signed-off-by: Thirupathaiah Annapureddy >>> --- >>> common/image-fit-sig.c | 12 +++- >>> 1 file changed, 11 insertions(+), 1 deletion(-) > > One more thing...this patch is changing the policy. I assume you are referring to the policy of conf signing with all required keys instead of just one. I just wanted to double check. However I did not see any test in test_vboot.py for verifying this policy. So I thought signing with all required keys is not by design and it is an unintended bug. Could you please clarify on this? > > I think we need a new string property in the DTB alongside the > 'required' properly, that indicates whether the image must be signed > with all required keys, or just one. > > Regards, > Simon > Best Regards, Thiru
[PATCH 0/2] Add support for multiple required keys
This patch series adds the support for multiple required keys in U-Boot DTB with test support. Thirupathaiah Annapureddy (2): vboot: add support for multiple required keys test: vboot: add a test for multiple required keys common/image-fit-sig.c | 12 +++- test/py/tests/test_vboot.py | 33 +++-- 2 files changed, 42 insertions(+), 3 deletions(-) -- 2.17.1
[PATCH 1/2] vboot: add support for multiple required keys
Currently Verified Boot fails if there is a signature verification failure
using required key in U-boot DTB. This patch adds support for multiple
required keys. This means if verified boot passes with one of the required
keys, u-boot will continue the OS hand off.
There was a prior attempt to resolve this with the following patch:
https://lists.denx.de/pipermail/u-boot/2019-April/366047.html
The above patch was failing "make tests".
Signed-off-by: Thirupathaiah Annapureddy
---
common/image-fit-sig.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
index cc1967109e..4d25d4c541 100644
--- a/common/image-fit-sig.c
+++ b/common/image-fit-sig.c
@@ -416,6 +416,8 @@ int fit_config_verify_required_sigs(const void *fit, int
conf_noffset,
{
int noffset;
int sig_node;
+ int verified = 0;
+ int reqd_sigs = 0;
/* Work out what we need to verify */
sig_node = fdt_subnode_offset(sig_blob, 0, FIT_SIG_NODENAME);
@@ -433,15 +435,23 @@ int fit_config_verify_required_sigs(const void *fit, int
conf_noffset,
NULL);
if (!required || strcmp(required, "conf"))
continue;
+
+ reqd_sigs++;
+
ret = fit_config_verify_sig(fit, conf_noffset, sig_blob,
noffset);
if (ret) {
printf("Failed to verify required signature '%s'\n",
fit_get_name(sig_blob, noffset, NULL));
- return ret;
+ } else {
+ verified = 1;
+ break;
}
}
+ if (reqd_sigs && !verified)
+ return -EPERM;
+
return 0;
}
--
2.17.1
[PATCH 2/2] test: vboot: add a test for multiple required keys
Add a test to verify the support for multiple required
keys. This patch also fixes existing test where dev
key is assumed to be marked as not required, although
it is marked as required.
Note that this patch re-added sign_fit_norequire().
sign_fit_norequire() was removed as part of the following:
commit b008677daf2a ("test: vboot: Fix pylint errors").
This patch leverages sign_fit_norequire() to fix the
existing bug.
Signed-off-by: Thirupathaiah Annapureddy
---
test/py/tests/test_vboot.py | 33 +++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 6b998cfd70..9773ee3509 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -126,6 +126,23 @@ def test_vboot(u_boot_console, sha_algo, padding,
sign_options, required):
cons.log.action('%s: Sign images' % sha_algo)
util.run_and_log(cons, args)
+def sign_fit_norequire(sha_algo, options):
+"""Sign the FIT
+
+Signs the FIT and writes the signature into it. It also writes the
+public key into the dtb. It does not mark key as 'required' in dtb.
+
+Args:
+sha_algo: Either 'sha1' or 'sha256', to select the algorithm to
+use.
+options: Options to provide to mkimage.
+"""
+args = [mkimage, '-F', '-k', tmpdir, '-K', dtb, fit]
+if options:
+args += options.split(' ')
+cons.log.action('%s: Sign images' % sha_algo)
+util.run_and_log(cons, args)
+
def replace_fit_totalsize(size):
"""Replace FIT header's totalsize with something greater.
@@ -279,15 +296,27 @@ def test_vboot(u_boot_console, sha_algo, padding,
sign_options, required):
# Build the FIT with dev key (keys NOT required). This adds the
# signature into sandbox-u-boot.dtb, NOT marked 'required'.
make_fit('sign-configs-%s%s.its' % (sha_algo, padding))
-sign_fit(sha_algo, sign_options)
+sign_fit_norequire(sha_algo, sign_options)
# So now sandbox-u-boot.dtb two signatures, for the prod and dev keys.
# Only the prod key is set as 'required'. But FIT we just built has
-# a dev signature only (sign_fit() overwrites the FIT).
+# a dev signature only (sign_fit_norequire() overwrites the FIT).
# Try to boot the FIT with dev key. This FIT should not be accepted by
# U-Boot because the prod key is required.
run_bootm(sha_algo, 'required key', '', False)
+# Build the FIT with dev key (keys required) and sign it. This puts the
+# signature into sandbox-u-boot.dtb, marked 'required'.
+make_fit('sign-configs-%s%s.its' % (sha_algo, padding))
+sign_fit(sha_algo, sign_options)
+
+# So now sandbox-u-boot.dtb two signatures, for the prod and dev keys.
+# Both the dev and prod key are set as 'required'. But FIT we just
built has
+# a dev signature only (sign_fit() overwrites the FIT).
+# Try to boot the FIT with dev key. This FIT should be accepted by
+# U-Boot because the dev key is required.
+run_bootm(sha_algo, 'multi required key', '', True)
+
cons = u_boot_console
tmpdir = cons.config.result_dir + '/'
datadir = cons.config.source_dir + '/test/py/tests/vboot/'
--
2.17.1
Re: [PATCH v3 1/1] cmd: bootefi: Parse reserved-memory node from DT
On 3/17/2020 4:10 AM, Heinrich Schuchardt wrote: > On 3/17/20 9:06 AM, Patrick DELAUNAY wrote: >> Hi, >> >>> From: U-Boot On Behalf Of Heinrich Schuchardt >>> Sent: dimanche 15 mars 2020 13:03 >>> > > It makes sense to keep these coding stretches in sync. > > @Thirupathaiah > * I did not find any example in the Linux code where a status for > a subnode of /reserved-memory was specified. Where did you > observe reserved memory not having status="okay"? A child node of /reserved-memory may be declared in a SOC specific dtsi file. But a given board's dts file may chose to disable this i.e. it does not need to reserve this memory. > * Does Linux check the status? yes, it checks. __fdt_scan_reserved_mem() -> of_fdt_device_is_available() Best Regards, Thiru
[PATCH] menu: add support for client defined statusline function
Currently displaying status line is done in a weak function
menu_display_statusline().
bootmenu.c overrides the weak default function.
It calls menu_default_choice() and interprets the data as
struct bootmenu_entry.
pxe boot also uses common menu code for pxe menus.
If there is a system that enables both bootmenu and pxe,
menu_display_statusline() defined in bootmenu.c will be called
and it will interpret struct pxe_label as struct bootmenu_entry.
This leads to data aborts and pxe menu corruptions.
This patch adds support for client defined statusline function
to resolve the above bug.
Signed-off-by: Thirupathaiah Annapureddy
---
cmd/bootmenu.c | 61 +
cmd/pxe_utils.c | 2 +-
common/menu.c | 13 +++
include/menu.h | 2 +-
4 files changed, 41 insertions(+), 37 deletions(-)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 3dc2c854ac..f1562883f5 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -365,6 +365,34 @@ cleanup:
return NULL;
}
+static void menu_display_statusline(struct menu *m)
+{
+ struct bootmenu_entry *entry;
+ struct bootmenu_data *menu;
+
+ if (menu_default_choice(m, (void *)) < 0)
+ return;
+
+ menu = entry->menu;
+
+ printf(ANSI_CURSOR_POSITION, 1, 1);
+ puts(ANSI_CLEAR_LINE);
+ printf(ANSI_CURSOR_POSITION, 2, 1);
+ puts(" *** U-Boot Boot Menu ***");
+ puts(ANSI_CLEAR_LINE_TO_END);
+ printf(ANSI_CURSOR_POSITION, 3, 1);
+ puts(ANSI_CLEAR_LINE);
+
+ /* First 3 lines are bootmenu header + 2 empty lines between entries */
+ printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
+ puts(ANSI_CLEAR_LINE);
+ printf(ANSI_CURSOR_POSITION, menu->count + 6, 1);
+ puts(" Press UP/DOWN to move, ENTER to select");
+ puts(ANSI_CLEAR_LINE_TO_END);
+ printf(ANSI_CURSOR_POSITION, menu->count + 7, 1);
+ puts(ANSI_CLEAR_LINE);
+}
+
static void bootmenu_show(int delay)
{
int init = 0;
@@ -396,8 +424,9 @@ static void bootmenu_show(int delay)
if (!bootmenu)
return;
- menu = menu_create(NULL, bootmenu->delay, 1, bootmenu_print_entry,
- bootmenu_choice_entry, bootmenu);
+ menu = menu_create(NULL, bootmenu->delay, 1, menu_display_statusline,
+ bootmenu_print_entry, bootmenu_choice_entry,
+ bootmenu);
if (!menu) {
bootmenu_destroy(bootmenu);
return;
@@ -445,34 +474,6 @@ cleanup:
#endif
}
-void menu_display_statusline(struct menu *m)
-{
- struct bootmenu_entry *entry;
- struct bootmenu_data *menu;
-
- if (menu_default_choice(m, (void *)) < 0)
- return;
-
- menu = entry->menu;
-
- printf(ANSI_CURSOR_POSITION, 1, 1);
- puts(ANSI_CLEAR_LINE);
- printf(ANSI_CURSOR_POSITION, 2, 1);
- puts(" *** U-Boot Boot Menu ***");
- puts(ANSI_CLEAR_LINE_TO_END);
- printf(ANSI_CURSOR_POSITION, 3, 1);
- puts(ANSI_CLEAR_LINE);
-
- /* First 3 lines are bootmenu header + 2 empty lines between entries */
- printf(ANSI_CURSOR_POSITION, menu->count + 5, 1);
- puts(ANSI_CLEAR_LINE);
- printf(ANSI_CURSOR_POSITION, menu->count + 6, 1);
- puts(" Press UP/DOWN to move, ENTER to select");
- puts(ANSI_CLEAR_LINE_TO_END);
- printf(ANSI_CURSOR_POSITION, menu->count + 7, 1);
- puts(ANSI_CLEAR_LINE);
-}
-
#ifdef CONFIG_AUTOBOOT_MENU_SHOW
int menu_show(int bootdelay)
{
diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
index 53af04d7dc..c244bfb10d 100644
--- a/cmd/pxe_utils.c
+++ b/cmd/pxe_utils.c
@@ -1237,7 +1237,7 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
* Create a menu and add items for all the labels.
*/
m = menu_create(cfg->title, DIV_ROUND_UP(cfg->timeout, 10),
- cfg->prompt, label_print, NULL, NULL);
+ cfg->prompt, NULL, label_print, NULL, NULL);
if (!m)
return NULL;
diff --git a/common/menu.c b/common/menu.c
index 7b66d199a9..5fb2ffbd06 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -36,6 +36,7 @@ struct menu {
int timeout;
char *title;
int prompt;
+ void (*display_statusline)(struct menu *);
void (*item_data_print)(void *);
char *(*item_choice)(void *);
void *item_choice_data;
@@ -106,10 +107,6 @@ static inline void *menu_item_destroy(struct menu *m,
return NULL;
}
-__weak void menu_display_statusline(struct menu *m)
-{
-}
-
/*
* Display a menu so the user can make a choice of an item. First display its
* title, if any, and then each item in the menu.
@@ -120,7 +117,8 @@ static inline void menu_display(struct menu *m)
puts(m->title);
[PATCH] Revert "dm: fdt: scan for devices under /firmware too"
Subnodes under "/firmware" node are scanned twice in
dm_scan_fdt_node() and dm_extended_scan_fdt().
This patch removes the scanning in dm_scan_fdt_node() to
avoid double scanning.
This reverts commit 747558d014577526bf2e8d9fe9ca748fdbf75d8a.
Signed-off-by: Thirupathaiah Annapureddy
---
drivers/core/root.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c
index e85643819e..cb695e933a 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -254,15 +254,9 @@ static int dm_scan_fdt_node(struct udevice *parent, const
void *blob,
for (offset = fdt_first_subnode(blob, offset);
offset > 0;
offset = fdt_next_subnode(blob, offset)) {
- const char *node_name = fdt_get_name(blob, offset, NULL);
-
- /*
-* The "chosen" and "firmware" nodes aren't devices
-* themselves but may contain some:
-*/
- if (!strcmp(node_name, "chosen") ||
- !strcmp(node_name, "firmware")) {
- pr_debug("parsing subnodes of \"%s\"\n", node_name);
+ /* "chosen" node isn't a device itself but may contain some: */
+ if (!strcmp(fdt_get_name(blob, offset, NULL), "chosen")) {
+ pr_debug("parsing subnodes of \"chosen\"\n");
err = dm_scan_fdt_node(parent, blob, offset,
pre_reloc_only);
@@ -279,7 +273,8 @@ static int dm_scan_fdt_node(struct udevice *parent, const
void *blob,
pre_reloc_only);
if (err && !ret) {
ret = err;
- debug("%s: ret=%d\n", node_name, ret);
+ debug("%s: ret=%d\n", fdt_get_name(blob, offset, NULL),
+ ret);
}
}
--
2.24.1
Re: [PATCH] image: fdt: check "status" of "/reserved-memory" subnodes
Thank You Simon for the review.
May I know what are the next steps in making forward progress on this?
Best Regards,
Thiru
On 1/7/2020 12:33 AM, Simon Goldschmidt wrote:
> On Tue, Jan 7, 2020 at 7:21 AM Thirupathaiah Annapureddy
> wrote:
>>
>> boot_fdt_add_mem_rsv_regions() scans the subnodes of
>> "/reserved-memory" and adds them to reserved lmb regions.
>> Currently this scanning does not take into "status" property.
>> Even if the subnode is disabled, it gets added to the
>> reserved lmb regions.
>>
>> This patch checks the "status" property before adding it
>> to reserved lmb regions.
>>
>> Signed-off-by: Thirupathaiah Annapureddy
>
> Reviewed-by: Simon Goldschmidt
>
>> ---
>> common/image-fdt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/image-fdt.c b/common/image-fdt.c
>> index 48388488d9..cf13d655c0 100644
>> --- a/common/image-fdt.c
>> +++ b/common/image-fdt.c
>> @@ -122,7 +122,7 @@ void boot_fdt_add_mem_rsv_regions(struct lmb *lmb, void
>> *fdt_blob)
>> /* check if this subnode has a reg property */
>> ret = fdt_get_resource(fdt_blob, subnode, "reg", 0,
>>);
>> - if (!ret) {
>> + if (!ret && fdtdec_get_is_enabled(fdt_blob,
>> subnode)) {
>> addr = res.start;
>> size = res.end - res.start + 1;
>> boot_fdt_reserve_region(lmb, addr, size);
>> --
>> 2.24.1
>>
Re: [PATCH] tpm2: ftpm: A driver for firmware TPM running inside TEE
Hi All,
May I know what are the next steps in making forward progress on this?
Best Regards,
Thiru
On 1/12/2020 11:34 PM, Thirupathaiah Annapureddy wrote:
> Add a driver for a firmware TPM running inside TEE.
>
> Documentation of the firmware TPM:
> https://www.microsoft.com/en-us/research/publication/ftpm-software-implementation-tpm-chip/
>
> Implementation of the firmware TPM:
> https://github.com/Microsoft/ms-tpm-20-ref/tree/master/Samples/ARM32-FirmwareTPM
>
> Signed-off-by: Thirupathaiah Annapureddy
> ---
> drivers/tpm/Kconfig | 6 +
> drivers/tpm/Makefile| 1 +
> drivers/tpm/tpm2_ftpm_tee.c | 250
> drivers/tpm/tpm2_ftpm_tee.h | 35 +
> 4 files changed, 292 insertions(+)
> create mode 100644 drivers/tpm/tpm2_ftpm_tee.c
> create mode 100644 drivers/tpm/tpm2_ftpm_tee.h
>
> diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
> index 94629dffd2..81bbffc50d 100644
> --- a/drivers/tpm/Kconfig
> +++ b/drivers/tpm/Kconfig
> @@ -145,6 +145,12 @@ config TPM2_TIS_SPI
> to the device using the standard TPM Interface Specification (TIS)
> protocol.
>
> +config TPM2_FTPM_TEE
> + bool "TEE based fTPM Interface"
> + depends on TEE && OPTEE && TPM_V2
> + help
> + This driver supports firmware TPM running in TEE.
> +
> endif # TPM_V2
>
> endmenu
> diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
> index 94c337b8ed..b1be3feac8 100644
> --- a/drivers/tpm/Makefile
> +++ b/drivers/tpm/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_TPM_ST33ZP24_SPI) += tpm_tis_st33zp24_spi.o
>
> obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o
> obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o
> +obj-$(CONFIG_TPM2_FTPM_TEE) += tpm2_ftpm_tee.o
> diff --git a/drivers/tpm/tpm2_ftpm_tee.c b/drivers/tpm/tpm2_ftpm_tee.c
> new file mode 100644
> index 00..4b79d4ada0
> --- /dev/null
> +++ b/drivers/tpm/tpm2_ftpm_tee.c
> @@ -0,0 +1,250 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation
> + *
> + * Authors:
> + * Thirupathaiah Annapureddy
> + *
> + * Description:
> + * Device Driver for a firmware TPM as described here:
> + *
> https://www.microsoft.com/en-us/research/publication/ftpm-software-implementation-tpm-chip/
> + *
> + * A reference implementation is available here:
> + *
> https://github.com/microsoft/ms-tpm-20-ref/tree/master/Samples/ARM32-FirmwareTPM/optee_ta/fTPM
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include "tpm_tis.h"
> +#include "tpm2_ftpm_tee.h"
> +
> +/**
> + * ftpm_tee_transceive() - send fTPM commands and retrieve fTPM response.
> + * @sendbuf - address of the data to send, byte by byte
> + * @send_size - length of the data to send
> + * @recvbuf - address where to read the response, byte by byte.
> + * @recv_len - pointer to the size of buffer
> + *
> + * Return:
> + * In case of success, returns 0.
> + * On failure, -errno
> + */
> +static int ftpm_tee_transceive(struct udevice *dev, const u8 *sendbuf,
> + size_t send_size, u8 *recvbuf,
> + size_t *recv_len)
> +{
> + struct ftpm_tee_private *context = dev_get_priv(dev);
> + int rc = 0;
> + size_t resp_len;
> + u8 *resp_buf;
> + struct tpm_output_header *resp_header;
> + struct tee_invoke_arg transceive_args;
> + struct tee_param command_params[4];
> + struct tee_shm *shm;
> +
> + if (send_size > MAX_COMMAND_SIZE) {
> + debug("%s:send_size=%zd exceeds MAX_COMMAND_SIZE\n",
> + __func__, send_size);
> + return -EIO;
> + }
> +
> + shm = context->shm;
> + memset(_args, 0, sizeof(transceive_args));
> + memset(command_params, 0, sizeof(command_params));
> +
> + /* Invoke FTPM_OPTEE_TA_SUBMIT_COMMAND function of fTPM TA */
> + transceive_args = (struct tee_invoke_arg) {
> + .func = FTPM_OPTEE_TA_SUBMIT_COMMAND,
> + .session = context->session,
> + };
> +
> + /* Fill FTPM_OPTEE_TA_SUBMIT_COMMAND parameters */
> + /* request */
> + command_params[0] = (struct tee_param) {
> + .attr = TEE_PARAM_ATTR_TYPE_MEMREF_INPUT,
> + .u.memref = {
> + .shm = shm,
> + .size = send_size,
> + .shm_offs = 0,
> + },
> + };
> + memset(command_params[0].u.memref.shm->addr, 0,
> + (MAX_COMMAND_SIZE + MAX_RESPONSE_SIZE));
> + memcpy(c
[PATCH] tpm2: ftpm: A driver for firmware TPM running inside TEE
Add a driver for a firmware TPM running inside TEE.
Documentation of the firmware TPM:
https://www.microsoft.com/en-us/research/publication/ftpm-software-implementation-tpm-chip/
Implementation of the firmware TPM:
https://github.com/Microsoft/ms-tpm-20-ref/tree/master/Samples/ARM32-FirmwareTPM
Signed-off-by: Thirupathaiah Annapureddy
---
drivers/tpm/Kconfig | 6 +
drivers/tpm/Makefile| 1 +
drivers/tpm/tpm2_ftpm_tee.c | 250
drivers/tpm/tpm2_ftpm_tee.h | 35 +
4 files changed, 292 insertions(+)
create mode 100644 drivers/tpm/tpm2_ftpm_tee.c
create mode 100644 drivers/tpm/tpm2_ftpm_tee.h
diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
index 94629dffd2..81bbffc50d 100644
--- a/drivers/tpm/Kconfig
+++ b/drivers/tpm/Kconfig
@@ -145,6 +145,12 @@ config TPM2_TIS_SPI
to the device using the standard TPM Interface Specification (TIS)
protocol.
+config TPM2_FTPM_TEE
+ bool "TEE based fTPM Interface"
+ depends on TEE && OPTEE && TPM_V2
+ help
+ This driver supports firmware TPM running in TEE.
+
endif # TPM_V2
endmenu
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile
index 94c337b8ed..b1be3feac8 100644
--- a/drivers/tpm/Makefile
+++ b/drivers/tpm/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_TPM_ST33ZP24_SPI) += tpm_tis_st33zp24_spi.o
obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o
obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o
+obj-$(CONFIG_TPM2_FTPM_TEE) += tpm2_ftpm_tee.o
diff --git a/drivers/tpm/tpm2_ftpm_tee.c b/drivers/tpm/tpm2_ftpm_tee.c
new file mode 100644
index 00..4b79d4ada0
--- /dev/null
+++ b/drivers/tpm/tpm2_ftpm_tee.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation
+ *
+ * Authors:
+ * Thirupathaiah Annapureddy
+ *
+ * Description:
+ * Device Driver for a firmware TPM as described here:
+ *
https://www.microsoft.com/en-us/research/publication/ftpm-software-implementation-tpm-chip/
+ *
+ * A reference implementation is available here:
+ *
https://github.com/microsoft/ms-tpm-20-ref/tree/master/Samples/ARM32-FirmwareTPM/optee_ta/fTPM
+ */
+
+#include
+#include
+#include
+#include
+
+#include "tpm_tis.h"
+#include "tpm2_ftpm_tee.h"
+
+/**
+ * ftpm_tee_transceive() - send fTPM commands and retrieve fTPM response.
+ * @sendbuf - address of the data to send, byte by byte
+ * @send_size - length of the data to send
+ * @recvbuf - address where to read the response, byte by byte.
+ * @recv_len - pointer to the size of buffer
+ *
+ * Return:
+ * In case of success, returns 0.
+ * On failure, -errno
+ */
+static int ftpm_tee_transceive(struct udevice *dev, const u8 *sendbuf,
+ size_t send_size, u8 *recvbuf,
+ size_t *recv_len)
+{
+ struct ftpm_tee_private *context = dev_get_priv(dev);
+ int rc = 0;
+ size_t resp_len;
+ u8 *resp_buf;
+ struct tpm_output_header *resp_header;
+ struct tee_invoke_arg transceive_args;
+ struct tee_param command_params[4];
+ struct tee_shm *shm;
+
+ if (send_size > MAX_COMMAND_SIZE) {
+ debug("%s:send_size=%zd exceeds MAX_COMMAND_SIZE\n",
+ __func__, send_size);
+ return -EIO;
+ }
+
+ shm = context->shm;
+ memset(_args, 0, sizeof(transceive_args));
+ memset(command_params, 0, sizeof(command_params));
+
+ /* Invoke FTPM_OPTEE_TA_SUBMIT_COMMAND function of fTPM TA */
+ transceive_args = (struct tee_invoke_arg) {
+ .func = FTPM_OPTEE_TA_SUBMIT_COMMAND,
+ .session = context->session,
+ };
+
+ /* Fill FTPM_OPTEE_TA_SUBMIT_COMMAND parameters */
+ /* request */
+ command_params[0] = (struct tee_param) {
+ .attr = TEE_PARAM_ATTR_TYPE_MEMREF_INPUT,
+ .u.memref = {
+ .shm = shm,
+ .size = send_size,
+ .shm_offs = 0,
+ },
+ };
+ memset(command_params[0].u.memref.shm->addr, 0,
+ (MAX_COMMAND_SIZE + MAX_RESPONSE_SIZE));
+ memcpy(command_params[0].u.memref.shm->addr, sendbuf, send_size);
+
+ /* response */
+ command_params[1] = (struct tee_param) {
+ .attr = TEE_PARAM_ATTR_TYPE_MEMREF_INOUT,
+ .u.memref = {
+ .shm = shm,
+ .size = MAX_RESPONSE_SIZE,
+ .shm_offs = MAX_COMMAND_SIZE,
+ },
+ };
+
+ rc = tee_invoke_func(context->tee_dev, _args, 4,
+ command_params);
+ if ((rc < 0) || (transceive_args.ret != 0)) {
+ debug("%s:SUBMIT_COMMAND invoke error: 0x%x\n",
+ __func__, transceive_args.ret);
+ retu
Re: [PATCH] Revert "dm: core: Scan "/firmware" node by default"
On 1/8/2020 9:39 AM, Simon Glass wrote:
> Hi T,
>
> On Tue, 7 Jan 2020 at 20:24, Thirupathaiah Annapureddy
> wrote:
>>
>> Subnodes under "/firmware" node are scanned twice in dm_scan_fdt_node
>> and dm_extended_scan_fdt. This patch removes the double scanning.
>>
>> This reverts commit 1712ca21924bff678f19fd3141f435408d23bdbf.
>>
>> Signed-off-by: Thirupathaiah Annapureddy
>> ---
>> drivers/core/root.c | 8 +---
>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/drivers/core/root.c b/drivers/core/root.c
>> index e85643819e..daf7c61190 100644
>> --- a/drivers/core/root.c
>> +++ b/drivers/core/root.c
>> @@ -342,14 +342,8 @@ int dm_extended_scan_fdt(const void *blob, bool
>> pre_reloc_only)
>> }
>>
>> ret = dm_scan_fdt_ofnode_path("/clocks", pre_reloc_only);
>> - if (ret) {
>> - debug("scan for /clocks failed: %d\n", ret);
>> - return ret;
>> - }
>> -
>> - ret = dm_scan_fdt_ofnode_path("/firmware", pre_reloc_only);
>> if (ret)
>> - debug("scan for /firmware failed: %d\n", ret);
>> + debug("scan for /clocks failed: %d\n", ret);
>>
>> return ret;
>> }
>> --
>> 2.24.1
>>
>
> I actually think it would be better to remove this from
> dm_scan_fdt_node() since that function looks for f
>
Hi Simon,
Thanks. I will address your comment and send new patch.
My rationale for removing the scanning from dm_extended_scan_fdt() was
based on the order of the scan. Why do you think removing the scan from
dm_extended_scan_fdt() is better?
--Thiru
[PATCH] Revert "dm: core: Scan "/firmware" node by default"
Subnodes under "/firmware" node are scanned twice in dm_scan_fdt_node
and dm_extended_scan_fdt. This patch removes the double scanning.
This reverts commit 1712ca21924bff678f19fd3141f435408d23bdbf.
Signed-off-by: Thirupathaiah Annapureddy
---
drivers/core/root.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c
index e85643819e..daf7c61190 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -342,14 +342,8 @@ int dm_extended_scan_fdt(const void *blob, bool
pre_reloc_only)
}
ret = dm_scan_fdt_ofnode_path("/clocks", pre_reloc_only);
- if (ret) {
- debug("scan for /clocks failed: %d\n", ret);
- return ret;
- }
-
- ret = dm_scan_fdt_ofnode_path("/firmware", pre_reloc_only);
if (ret)
- debug("scan for /firmware failed: %d\n", ret);
+ debug("scan for /clocks failed: %d\n", ret);
return ret;
}
--
2.24.1
[PATCH] image: fdt: check "status" of "/reserved-memory" subnodes
boot_fdt_add_mem_rsv_regions() scans the subnodes of
"/reserved-memory" and adds them to reserved lmb regions.
Currently this scanning does not take into "status" property.
Even if the subnode is disabled, it gets added to the
reserved lmb regions.
This patch checks the "status" property before adding it
to reserved lmb regions.
Signed-off-by: Thirupathaiah Annapureddy
---
common/image-fdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/image-fdt.c b/common/image-fdt.c
index 48388488d9..cf13d655c0 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -122,7 +122,7 @@ void boot_fdt_add_mem_rsv_regions(struct lmb *lmb, void
*fdt_blob)
/* check if this subnode has a reg property */
ret = fdt_get_resource(fdt_blob, subnode, "reg", 0,
);
- if (!ret) {
+ if (!ret && fdtdec_get_is_enabled(fdt_blob, subnode)) {
addr = res.start;
size = res.end - res.start + 1;
boot_fdt_reserve_region(lmb, addr, size);
--
2.24.1
