[edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Roth, Michael via groups.io]

The struct used for GHCB-based page-state change requests uses a 40-bit
bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
64-bit address. However, anything beyond 40-bits simply gets shifted off
when doing this, which will cause issues when dealing with 1TB+
addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
prior to shifting it by PAGE_SHIFT.

Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate system RAM")
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
v2:
  - use EFI_PHYSICAL_ADDRESS instead of UINT64 (Gerd)

 .../BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c    | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
index 85eb41585b..46c6682760 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
@@ -78,13 +78,14 @@ PvalidateRange (
   IN  BOOLEAN                     Validate

   )

 {

-  UINTN  Address, RmpPageSize, Ret, i;

+  UINTN                 RmpPageSize, Ret, i;

+  EFI_PHYSICAL_ADDRESS  Address;

 

   for ( ; StartIndex <= EndIndex; StartIndex++) {

     //

     // Get the address and the page size from the Info.

     //

-    Address     = Info->Entry[StartIndex].GuestFrameNumber << EFI_PAGE_SHIFT;

+    Address     = ((EFI_PHYSICAL_ADDRESS)Info->Entry[StartIndex].GuestFrameNumber) << EFI_PAGE_SHIFT;

     RmpPageSize = Info->Entry[StartIndex].PageSize;

 

     Ret = AsmPvalidate (RmpPageSize, Validate, Address);

-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111285): https://edk2.groups.io/g/devel/message/111285
Mute This Topic: https://groups.io/mt/102610323/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Gerd Hoffmann]

On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
> The struct used for GHCB-based page-state change requests uses a 40-bit
> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
> 64-bit address. However, anything beyond 40-bits simply gets shifted off
> when doing this, which will cause issues when dealing with 1TB+
> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
> prior to shifting it by PAGE_SHIFT.
> 
> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate system RAM")
> Signed-off-by: Michael Roth <michael.roth@amd.com>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111305): https://edk2.groups.io/g/devel/message/111305
Mute This Topic: https://groups.io/mt/102610323/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Laszlo Ersek]

(+Liming +Mike)

On 11/16/23 10:01, Gerd Hoffmann wrote:
> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
>> The struct used for GHCB-based page-state change requests uses a 40-bit
>> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
>> 64-bit address. However, anything beyond 40-bits simply gets shifted off
>> when doing this, which will cause issues when dealing with 1TB+
>> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
>> prior to shifting it by PAGE_SHIFT.
>>
>> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate system RAM")
>> Signed-off-by: Michael Roth <michael.roth@amd.com>
> 
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> take care,
>   Gerd

Is this hard feature freeze material?

Do we have a BZ?

Also, the patch looks garbled to me on-list (superfluous line breaks).
Michael's git setup may not be perfect. Michael, can you compare
gitconfigs with Tom?

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111345): https://edk2.groups.io/g/devel/message/111345
Mute This Topic: https://groups.io/mt/102610323/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Gerd Hoffmann]

On Fri, Nov 17, 2023 at 10:16:10AM +0100, Laszlo Ersek wrote:
> (+Liming +Mike)
> 
> On 11/16/23 10:01, Gerd Hoffmann wrote:
> > On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
> >> The struct used for GHCB-based page-state change requests uses a 40-bit
> >> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
> >> 64-bit address. However, anything beyond 40-bits simply gets shifted off
> >> when doing this, which will cause issues when dealing with 1TB+
> >> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
> >> prior to shifting it by PAGE_SHIFT.
> >>
> >> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate system RAM")
> >> Signed-off-by: Michael Roth <michael.roth@amd.com>
> > 
> > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> > 
> > take care,
> >   Gerd
> 
> Is this hard feature freeze material?

It is a clear bugfix, so IMHO it qualifies.

> Also, the patch looks garbled to me on-list (superfluous line breaks).

Patch applies fine here.  I see mutt breaking the long line, but
that is just the local display rendering, the mail good.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111396): https://edk2.groups.io/g/devel/message/111396
Mute This Topic: https://groups.io/mt/102610323/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Laszlo Ersek]

On 11/17/23 12:42, Gerd Hoffmann wrote:
> On Fri, Nov 17, 2023 at 10:16:10AM +0100, Laszlo Ersek wrote:
>> (+Liming +Mike)
>>
>> On 11/16/23 10:01, Gerd Hoffmann wrote:
>>> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
>>>> The struct used for GHCB-based page-state change requests uses a 40-bit
>>>> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
>>>> 64-bit address. However, anything beyond 40-bits simply gets shifted off
>>>> when doing this, which will cause issues when dealing with 1TB+
>>>> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
>>>> prior to shifting it by PAGE_SHIFT.
>>>>
>>>> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate system RAM")
>>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>>>
>>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
>>>
>>> take care,
>>>   Gerd
>>
>> Is this hard feature freeze material?
> 
> It is a clear bugfix, so IMHO it qualifies.
> 
>> Also, the patch looks garbled to me on-list (superfluous line breaks).
> 
> Patch applies fine here.  I see mutt breaking the long line, but
> that is just the local display rendering, the mail good.

Can you check the raw message? I did that and it seems broken.
Superfluous newlines. I see *double* CRLFs.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111415): https://edk2.groups.io/g/devel/message/111415
Mute This Topic: https://groups.io/mt/102610323/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] 回复: [edk2-202311] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[gaoliming via groups.io]

Laszlo and Gerd:
  I agree this is a critical bug fix. If it plans to catch this stable tag, I am OK. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Laszlo Ersek
> 发送时间: 2023年11月18日 5:39
> 收件人: devel@edk2.groups.io; kraxel@redhat.com
> 抄送: Michael Roth <michael.roth@amd.com>; Ray Ni <ray.ni@intel.com>;
> Erdem Aktas <erdemaktas@google.com>; Jiewen Yao
> <jiewen.yao@intel.com>; Min Xu <min.m.xu@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Liming Gao (Byosoft address)
> <gaoliming@byosoft.com.cn>; Michael Kinney <michael.d.kinney@intel.com>
> 主题: Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address
> overflow during PVALIDATE
> 
> On 11/17/23 12:42, Gerd Hoffmann wrote:
> > On Fri, Nov 17, 2023 at 10:16:10AM +0100, Laszlo Ersek wrote:
> >> (+Liming +Mike)
> >>
> >> On 11/16/23 10:01, Gerd Hoffmann wrote:
> >>> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
> >>>> The struct used for GHCB-based page-state change requests uses a
> 40-bit
> >>>> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
> >>>> 64-bit address. However, anything beyond 40-bits simply gets shifted off
> >>>> when doing this, which will cause issues when dealing with 1TB+
> >>>> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
> >>>> prior to shifting it by PAGE_SHIFT.
> >>>>
> >>>> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to
> validate system RAM")
> >>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
> >>>
> >>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> >>>
> >>> take care,
> >>>   Gerd
> >>
> >> Is this hard feature freeze material?
> >
> > It is a clear bugfix, so IMHO it qualifies.
> >
> >> Also, the patch looks garbled to me on-list (superfluous line breaks).
> >
> > Patch applies fine here.  I see mutt breaking the long line, but
> > that is just the local display rendering, the mail good.
> 
> Can you check the raw message? I did that and it seems broken.
> Superfluous newlines. I see *double* CRLFs.
> 
> Laszlo
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111426): https://edk2.groups.io/g/devel/message/111426
Mute This Topic: https://groups.io/mt/102698691/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Gerd Hoffmann]

On Fri, Nov 17, 2023 at 10:39:13PM +0100, Laszlo Ersek wrote:
> On 11/17/23 12:42, Gerd Hoffmann wrote:
> > On Fri, Nov 17, 2023 at 10:16:10AM +0100, Laszlo Ersek wrote:
> >> (+Liming +Mike)
> >>
> >> On 11/16/23 10:01, Gerd Hoffmann wrote:
> >>> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
> >>>> The struct used for GHCB-based page-state change requests uses a 40-bit
> >>>> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
> >>>> 64-bit address. However, anything beyond 40-bits simply gets shifted off
> >>>> when doing this, which will cause issues when dealing with 1TB+
> >>>> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
> >>>> prior to shifting it by PAGE_SHIFT.
> >>>>
> >>>> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate system RAM")
> >>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
> >>>
> >>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> >>>
> >>> take care,
> >>>   Gerd
> >>
> >> Is this hard feature freeze material?
> > 
> > It is a clear bugfix, so IMHO it qualifies.
> > 
> >> Also, the patch looks garbled to me on-list (superfluous line breaks).
> > 
> > Patch applies fine here.  I see mutt breaking the long line, but
> > that is just the local display rendering, the mail good.
> 
> Can you check the raw message? I did that and it seems broken.
> Superfluous newlines. I see *double* CRLFs.

Hmm, everything looks fine here, and 'git am' accepts the mail without
problems.  Pushed a branch:

https://github.com/kraxel/edk2/commits/b4/v2-20231115-michael-roth-ovmfpkg-memencryptsevlib-fix-address-overflow-during-pvalidate

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111462): https://edk2.groups.io/g/devel/message/111462
Mute This Topic: https://groups.io/mt/102610323/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Laszlo Ersek]

On 11/20/23 08:55, Gerd Hoffmann wrote:
> On Fri, Nov 17, 2023 at 10:39:13PM +0100, Laszlo Ersek wrote:
>> On 11/17/23 12:42, Gerd Hoffmann wrote:
>>> On Fri, Nov 17, 2023 at 10:16:10AM +0100, Laszlo Ersek wrote:
>>>> (+Liming +Mike)
>>>>
>>>> On 11/16/23 10:01, Gerd Hoffmann wrote:
>>>>> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
>>>>>> The struct used for GHCB-based page-state change requests uses a 40-bit
>>>>>> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
>>>>>> 64-bit address. However, anything beyond 40-bits simply gets shifted off
>>>>>> when doing this, which will cause issues when dealing with 1TB+
>>>>>> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
>>>>>> prior to shifting it by PAGE_SHIFT.
>>>>>>
>>>>>> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate system RAM")
>>>>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>>>>>
>>>>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
>>>>>
>>>>> take care,
>>>>>   Gerd
>>>>
>>>> Is this hard feature freeze material?
>>>
>>> It is a clear bugfix, so IMHO it qualifies.
>>>
>>>> Also, the patch looks garbled to me on-list (superfluous line breaks).
>>>
>>> Patch applies fine here.  I see mutt breaking the long line, but
>>> that is just the local display rendering, the mail good.
>>
>> Can you check the raw message? I did that and it seems broken.
>> Superfluous newlines. I see *double* CRLFs.
> 
> Hmm, everything looks fine here, and 'git am' accepts the mail without
> problems.  Pushed a branch:
> 
> https://github.com/kraxel/edk2/commits/b4/v2-20231115-michael-roth-ovmfpkg-memencryptsevlib-fix-address-overflow-during-pvalidate

Thanks. I hope that suffices for Liming to merge the patch.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111588): https://edk2.groups.io/g/devel/message/111588
Mute This Topic: https://groups.io/mt/102610323/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Laszlo Ersek]

On 11/20/23 08:55, Gerd Hoffmann wrote:
> On Fri, Nov 17, 2023 at 10:39:13PM +0100, Laszlo Ersek wrote:
>> On 11/17/23 12:42, Gerd Hoffmann wrote:
>>> On Fri, Nov 17, 2023 at 10:16:10AM +0100, Laszlo Ersek wrote:
>>>> (+Liming +Mike)
>>>>
>>>> On 11/16/23 10:01, Gerd Hoffmann wrote:
>>>>> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
>>>>>> The struct used for GHCB-based page-state change requests uses a 40-bit
>>>>>> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
>>>>>> 64-bit address. However, anything beyond 40-bits simply gets shifted off
>>>>>> when doing this, which will cause issues when dealing with 1TB+
>>>>>> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
>>>>>> prior to shifting it by PAGE_SHIFT.
>>>>>>
>>>>>> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate system RAM")
>>>>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>>>>>
>>>>> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
>>>>>
>>>>> take care,
>>>>>   Gerd
>>>>
>>>> Is this hard feature freeze material?
>>>
>>> It is a clear bugfix, so IMHO it qualifies.
>>>
>>>> Also, the patch looks garbled to me on-list (superfluous line breaks).
>>>
>>> Patch applies fine here.  I see mutt breaking the long line, but
>>> that is just the local display rendering, the mail good.
>>
>> Can you check the raw message? I did that and it seems broken.
>> Superfluous newlines. I see *double* CRLFs.
> 
> Hmm, everything looks fine here, and 'git am' accepts the mail without
> problems.  Pushed a branch:
> 
> https://github.com/kraxel/edk2/commits/b4/v2-20231115-michael-roth-ovmfpkg-memencryptsevlib-fix-address-overflow-during-pvalidate

This branch contains whitespace damage. The new lines coming from the
patch are terminated with LF, not CRLF.

(Doesn't matter much, just wanted to clarify that mutt wasn't doing the
right thing on your end. The patch, as posted, does contain multiple
CRLFs, and while mutt seems to mitigate that for you, it overshoots.

Anyway, I'm picking this up now; I've cleaned up the double CRLFs manually.)

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112227): https://edk2.groups.io/g/devel/message/112227
Mute This Topic: https://groups.io/mt/102610323/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Laszlo Ersek]

On 11/16/23 10:01, Gerd Hoffmann wrote:
> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
>> The struct used for GHCB-based page-state change requests uses a 40-bit
>> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
>> 64-bit address. However, anything beyond 40-bits simply gets shifted off
>> when doing this, which will cause issues when dealing with 1TB+
>> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
>> prior to shifting it by PAGE_SHIFT.
>>
>> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate system RAM")
>> Signed-off-by: Michael Roth <michael.roth@amd.com>
> 
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

Merged as commit e8c23d1e27f70dcb2e59010ded6df32374eaa84a, via
<https://github.com/tianocore/edk2/pull/5125>.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112230): https://edk2.groups.io/g/devel/message/112230
Mute This Topic: https://groups.io/mt/102610323/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-