From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) by mx.groups.io with SMTP id smtpd.web11.1723.1662572659607342238 for ; Wed, 07 Sep 2022 10:44:19 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=kzQ0aRsu; spf=pass (domain: gmail.com, ip: 209.85.214.178, mailfrom: kuqin12@gmail.com) Received: by mail-pl1-f178.google.com with SMTP id iw17so8673230plb.0 for ; Wed, 07 Sep 2022 10:44:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:from:references:cc:to:content-language:subject :user-agent:mime-version:date:message-id:from:to:cc:subject:date; bh=elS++iu5uIZRX80CWmTdPoFS7ajjrTt+lyyBFhxpiBI=; b=kzQ0aRsu252vEhwfNNyAhPRx4ccndhlhRR18NMoTiyemi4Bz0uJwFKPbTew4HLByNI NpBabw9WovhFY3Z+17txO3QVaC2DmfDK4/Ce+hicg4tl6b/XN3Nq3ssPEiiazJw4XLD7 9zhgasrlPkkH9zWTgZaRiuTDcbPR560nhWXBnRoRySPwbTGktYshK57Rj9SlClxoCsRu +kj49nr26qMxGIyClzd2ytxQ/MuMuBEdBOgooEq2+A3RHUQRD8TcTWZlqWlqoi9hwE1B HbfCKa6lFUiPBeh2JEvHX4LPVPi4BjwImHtgH8+LOrSGCXTN8kaRdPz4ulB4T4V19sN1 VaOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:from:references:cc:to:content-language:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date; bh=elS++iu5uIZRX80CWmTdPoFS7ajjrTt+lyyBFhxpiBI=; b=oIh/q/qsFDR9SWbm53+4ACq+zZfBAsO8Ie/e8fg3tqM+iZtp0xj4PzFyI1alUL83Os aKbYaK9tF0+Mnm1HHehj94S3f67YBBgI9BxK75ZMj6MJUzKbmuh2vV7rpsciwy2pINuk iKje1yYeqHGP9FSlpB7Ks8097F+V0x03XuJDHgwGZuA1lNR1Q93kw2D2ZKlhQFgVJqZv QBq/5dHpINUrQGCsKnhkgckIHKqK0XfrcOrio0QWbQYodK35rgNlQdGvF+FkrI09vcMO 4WR4/LqX71A03aHwXHL6KlerVKbFtKjusQBEKd5UPfHD8Oh/XylhYmRnlLvpISWlVBkA 6mow== X-Gm-Message-State: ACgBeo3SoJuwMMekLqGLHkorbrEP6mRVkEj1pOhim0mlMSohc+1bL8xU giER2MsXMof7nUjNGaWfCrI= X-Google-Smtp-Source: AA6agR6dYPj8zl3C9BP8xC9FzyV7GVxpu9VW2BVW1i6d6DnbaTjHRTL8N5VxcJR3aDbaSIC0YJpyyA== X-Received: by 2002:a17:903:1ce:b0:16e:f510:6666 with SMTP id e14-20020a17090301ce00b0016ef5106666mr4874809plh.158.1662572658889; Wed, 07 Sep 2022 10:44:18 -0700 (PDT) Return-Path: Received: from ?IPV6:2001:4898:d8:33:910:98d:6048:94ec? ([2001:4898:80e8:2:8930:98d:6048:94ec]) by smtp.gmail.com with ESMTPSA id x7-20020a623107000000b0052e6c058bccsm12830952pfx.61.2022.09.07.10.44.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 07 Sep 2022 10:44:18 -0700 (PDT) Message-ID: <26aec07a-3731-75eb-6e68-bfd719df12a9@gmail.com> Date: Wed, 7 Sep 2022 10:44:17 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 Subject: Re: [edk2-devel] [PATCH v1 0/2] MM communicate functionality in variable policy To: Ard Biesheuvel Cc: edk2-devel-groups-io , Jian J Wang , Liming Gao , Hao A Wu , Leif Lindholm , Ard Biesheuvel , Bret Barkelew , Michael Kubacki References: <16BC2C06E438B403.26361@groups.io> <6fcb7f10-c113-1c9d-2b4d-264b81ed6ad1@gmail.com> From: "Kun Qin" In-Reply-To: Content-Type: multipart/alternative; boundary="------------JtGtJ9v0jFyR8iEIukbDRLV0" Content-Language: en-US --------------JtGtJ9v0jFyR8iEIukbDRLV0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Ard, No worries. Liming has helped to merge these patches here: MM communicate functionality in variable policy by lgao4 · Pull Request #2443 · tianocore/edk2 (github.com) . Thanks for checking back. Regards, Kun On 9/7/2022 1:27 AM, Ard Biesheuvel wrote: > On Mon, 13 Dec 2021 at 19:23, Kun Qin wrote: >> Hi ArmPkg and MdeModulePkg maintainers, >> >> Now that the hard freeze is lifted, could you please provide some >> feedback on these patches when you have a chance? >> > My apologies for not responding to these changes. > > If these patches are still relevant, please rebase and resend them. > > >> On 12/06/2021 10:41, Ard Biesheuvel wrote: >>> On Mon, 6 Dec 2021 at 19:35, Kun Qin 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 >>>>> Cc: Liming Gao >>>>> Cc: Hao A Wu >>>>> Cc: Leif Lindholm >>>>> Cc: Ard Biesheuvel >>>>> Cc: Bret Barkelew >>>>> Cc: Michael Kubacki >>>>> >>>>> 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(-) >>>>> --------------JtGtJ9v0jFyR8iEIukbDRLV0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi Ard,

No worries. Liming has helped to merge these patches here: MM communicate functionality in variable policy by lgao4 · Pull Request #2443 · tianocore/edk2 (github.com). Thanks for checking back.

Regards,
Kun

On 9/7/2022 1:27 AM, Ard Biesheuvel wrote:
On Mon, 13 Dec 2021 at 19:23, Kun Qin <kuqin12@gmail.com> wrote:

Hi ArmPkg and MdeModulePkg maintainers,

Now that the hard freeze is lifted, could you please provide some
feedback on these patches when you have a chance?


My apologies for not responding to these changes.

If these patches are still relevant, please rebase and resend them.



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(-)
--------------JtGtJ9v0jFyR8iEIukbDRLV0--