回复: [edk2-devel] [PATCH v1 0/2] MM communicate functionality in variable policy

["gaoliming" <gaoliming@byosoft.com.cn>]

Kun:
  Does this change impact current behavior? Seemly, there is no strict check on MessageLength.

Thanks
Liming
> -----邮件原件-----
> 发件人: Kun Qin <kuqin12@gmail.com>
> 发送时间: 2021年12月7日 2:47
> 收件人: Ard Biesheuvel <ardb@kernel.org>
> 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Jian J Wang
> <jian.j.wang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Hao A
> Wu <hao.a.wu@intel.com>; Leif Lindholm <leif@nuviainc.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>; Michael Kubacki
> <michael.kubacki@microsoft.com>
> 主题: Re: [edk2-devel] [PATCH v1 0/2] MM communicate functionality in
> variable policy
> 
> Thanks for the information, Ard. I just meant to plan ahead so that I
> can work on the feedback for these patches, if any.
> 
> I can ping back the thread again once the stable tag is created.
> 
> Regards,
> Kun
> 
> On 12/06/2021 10:41, Ard Biesheuvel wrote:
> > On Mon, 6 Dec 2021 at 19:35, Kun Qin <kuqin12@gmail.com> wrote:
> >>
> >> Hi ArmPkg and MdeModulePkg maintainers,
> >>
> >> It has been a week since the patches were sent. Could you please review
> >> the changes and let me know if there is any feedback? Any input is
> >> appreciated.
> >>
> >
> > As far as I know, we are still in hard freeze for the upcoming stable tag.
> >
> >>
> >> On 11/29/2021 16:39, Kun Qin via groups.io wrote:
> >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3709
> >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751
> >>>
> >>> Currently, setups with variable policy operations used together with MM
> >>> communicate from ArmPkg could fail with `EFI_INVALID_PARAMETER`.
> This was
> >>> due to the errors from 2 following aspects:
> >>>
> >>> 1. For variable policy implementations in MdeModulePkg, the DXE
> runtime
> >>> agent would communicate to MM to disable, register or query policies.
> >>> However, during these operations, the MessageLength calculation is
> >>> including MM communicate header. This could lead to MM agent read
> data
> >>> across the given buffer boundary and/or trigger other errors.
> >>>
> >>> 2. On the other hand, current MM communicate routine from ArmPkg
> would
> >>> fail the function if the input message length does not equal to input
> >>> buffer size.
> >>>
> >>> As defined in PI specification, the `CommSize`, when as input, should
> >>> stand for "The size of the data buffer being passed in", which would mean
> >>> the maximal number of bytes `CommBuffer` can hold. In turn, the value of
> >>> this input parameter can be used for MM handlers to determine whether
> the
> >>> output data is too large to fit in this buffer. Enforcing the incoming
> >>> buffer to hold exactly the number of used bytes mismatches with the PI
> >>> spec description.
> >>>
> >>> This change fix MessageLength field calculation from variable policy and
> >>> updated input argument inspections from MM communicate routine in
> ArmPkg
> >>> to match PI spec descriptions.
> >>>
> >>> Patch v1 branch:
> https://github.com/kuqin12/edk2/tree/mm_communicate_check
> >>>
> >>> Cc: Jian J Wang <jian.j.wang@intel.com>
> >>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >>> Cc: Hao A Wu <hao.a.wu@intel.com>
> >>> Cc: Leif Lindholm <leif@nuviainc.com>
> >>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> >>> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> >>>
> >>> Kun Qin (2):
> >>>     MdeModulePkg: VariableSmmRuntimeDxe: Fix Variable Policy
> Message
> >>>       Length
> >>>     ArmPkg: MmCommunicationDxe: Update MM communicate input
> arguments
> >>>       checks
> >>>
> >>>    ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> | 44 ++++++++++++--------
> >>>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c |
> 10 ++---
> >>>    2 files changed, 32 insertions(+), 22 deletions(-)
> >>>