Re: [PATCH V2 06/50] MdeModulePkg/CapsuleLib: Add ProcessCapsules() API.

["Yao, Jiewen" <jiewen.yao@intel.com>]

HI Sean
We choose to process capsule twice purposely - for security consideration, as I mentioned in the comment section.
We did design review in detail in Intel technical sync meeting. And it is agreed by Mike Kinney and Vincent Zimmer.

To resolve your concern:

1)      For example windows capsule update will stage multiple capsules at once.  If it mixes capsules from both stages and you use memory to preserve capsule contents you will lose your non system capsule because of the reboot.
[Jiewen] That is good feedback.
I think we can hold the reset request in first process and defer that to second process.


2)      For capsules that are not FMP or update capsules but capsules being requested to be put in the system table you will still need to process them even though the boot mode should not be BOOT_ON_FLASH_UPDATE.
[Jiewen] In current EDKII, as long as there is in capsule, the platform sets BOOT_ON_FLASH_UPDATE.
That is done before my patch, I believe.
Of course we can do enhancement to always check capsule_hob, no matter what boot mode is.
But it is not related to process capsule twice.

Thank you
Yao Jiewen


From: Sean Brogan [mailto:sean.brogan@microsoft.com]
Sent: Tuesday, October 11, 2016 5:33 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH V2 06/50] MdeModulePkg/CapsuleLib: Add ProcessCapsules() API.

Comment about calling ProcessCapsules twice will break in some scenarios.  For example windows capsule update will stage multiple capsules at once.  If it mixes capsules from both stages and you use memory to preserve capsule contents you will lose your non system capsule because of the reboot.

2nd - For capsules that are not FMP or update capsules but capsules being requested to be put in the system table you will still need to process them even though the boot mode should not be BOOT_ON_FLASH_UPDATE.

Thanks
Sean


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiewen Yao
> Sent: Friday, September 30, 2016 5:21 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Feng Tian
> <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>; Liming Gao
> <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Subject: [edk2] [PATCH V2 06/50] MdeModulePkg/CapsuleLib: Add
> ProcessCapsules() API.
>
> ProcessCapsules() API can be used by platform BDS to process all capsules.
>
> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>>
> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
> Cc: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Reviewed-by: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
> ---
>  MdeModulePkg/Include/Library/CapsuleLib.h | 45 ++++++++++++++++++--
>  1 file changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Library/CapsuleLib.h
> b/MdeModulePkg/Include/Library/CapsuleLib.h
> index 487cb0f..659c077 100644
> --- a/MdeModulePkg/Include/Library/CapsuleLib.h
> +++ b/MdeModulePkg/Include/Library/CapsuleLib.h
> @@ -2,7 +2,7 @@
>
>    This library class defines a set of interfaces for how to process capsule image
> updates.
>
> -Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials are licensed and made available
> under  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at @@ -20,7 +20,9 @@ WITHOUT
> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
>    The firmware checks whether the capsule image is supported
>    by the CapsuleGuid in CapsuleHeader or if there is other specific information
> in
>    the capsule image.
> -
> +
> +  Caution: This function may receive untrusted input.
> +
>    @param  CapsuleHeader    Pointer to the UEFI capsule image to be checked.
>
>    @retval EFI_SUCESS       Input capsule is supported by firmware.
> @@ -35,7 +37,9 @@ SupportCapsuleImage (
>  /**
>    The firmware-specific implementation processes the capsule image
>    if it recognized the format of this capsule image.
> -
> +
> +  Caution: This function may receive untrusted input.
> +
>    @param  CapsuleHeader    Pointer to the UEFI capsule image to be processed.
>
>    @retval EFI_SUCESS       Capsule Image processed successfully.
> @@ -47,4 +51,39 @@ ProcessCapsuleImage (
>    IN EFI_CAPSULE_HEADER *CapsuleHeader
>    );
>
> +/**
> +
> +  This routine is called to process capsules.
> +
> +  Caution: This function may receive untrusted input.
> +
> +  If the current boot mode is NOT BOOT_ON_FLASH_UPDATE, this routine does
> nothing.
> +  If the current boot mode is BOOT_ON_FLASH_UPDATE, the capsules
> + reported in  EFI_HOB_UEFI_CAPSULE are processed. If there is no
> + EFI_HOB_UEFI_CAPSULE, this  routine does nothing.
> +
> +  This routine should be called twice in BDS.
> +  1) The first call must be before EndOfDxe. The system capsules is processed.
> +     If device capsule FMP protocols are exposted at this time, the device
> +     capsules are processed.
> +     Each individual capsule result is recorded in capsule record variable.
> +     System may reset in this function, if reset is required by capsule.
> +
> +  2) The second call must be after EndOfDxe and after ConnectAll, so that all
> +     device capsule FMP protocols are exposed.
> +     The system capsules are skipped. If the device capsules are NOT processed
> +     in first call, they are processed here.
> +     Each individual capsule result is recorded in capsule record variable.
> +     System may reset in this function, if reset is required by capsule.
> +
> +  @retval EFI_SUCCESS             There is no error when processing capsules.
> +  @retval EFI_OUT_OF_RESOURCES    No enough resource to process capsules.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +ProcessCapsules(
> +  VOID
> +  );
> +
>  #endif
> --
> 2.7.4.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel