.../BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
(+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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.