From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web11.4186.1594292695570339859 for ; Thu, 09 Jul 2020 04:04:55 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=fQEcPIs1; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594292694; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=I05zkEK1+zhL/cvij7NkNmqiIRWLv9NDQHNu3PckkFc=; b=fQEcPIs1RZC4Tk+cP4TBb5qnAHdfinl1nZ+9rQskRCRGQH+Vi2oWv0xgsAjTk+No+pHh9w AbOIJFxtfbvbuCTWJzYMIq+zlWSy0isV5Wgp5iAiWVL8VxPmz6FJA8CgN0av9K8ITBIOf7 QWdNzciKbJxySiWze6oAjlFIK8j1ARk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-198-O3SjJuemOTO7QRCXXpDKgg-1; Thu, 09 Jul 2020 07:04:50 -0400 X-MC-Unique: O3SjJuemOTO7QRCXXpDKgg-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A3BBA18FF660; Thu, 9 Jul 2020 11:04:49 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-104.ams2.redhat.com [10.36.114.104]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1FAA279231; Thu, 9 Jul 2020 11:04:47 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v5 3/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098) To: devel@edk2.groups.io, guomin.jiang@intel.com Cc: Michael Kubacki , Eric Dong , Ray Ni , Rahul Kumar References: <20200709015645.336-1-guomin.jiang@intel.com> <20200709015645.336-4-guomin.jiang@intel.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 9 Jul 2020 13:04:47 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200709015645.336-4-guomin.jiang@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 07/09/20 03:56, Guomin Jiang wrote: > From: Michael Kubacki > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614 > > Moves the GDT and IDT to permanent memory in a memory discovered > callback. This is done to ensure the GDT and IDT authenticated in > pre-memory is not fetched from outside a verified location after > the permanent memory transition. > > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Rahul Kumar > Signed-off-by: Michael Kubacki > Reviewed-by: Laszlo Ersek > --- > UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 1 + > UefiCpuPkg/CpuMpPei/CpuMpPei.h | 12 +++++++++++ > UefiCpuPkg/CpuMpPei/CpuMpPei.c | 37 ++++++++++++++++++++++++++++++++ > UefiCpuPkg/CpuMpPei/CpuPaging.c | 12 +++++++++-- > 4 files changed, 60 insertions(+), 2 deletions(-) sorry, a question -- the subject line and the commit message reference the IDT, not just the GDT. But the code handles the GDT only. Is this intentional? Should the code handle the IDT, or should we remove the IDT mentions from the subject and the commit message? Thanks Laszlo > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf > index caead3ce34d4..f4d11b861f77 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf > @@ -63,6 +63,7 @@ [Pcd] > gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList ## SOMETIMES_CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize ## SOMETIMES_CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ## SOMETIMES_CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes ## CONSUMES > > [Depex] > TRUE > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > index 7d5c527d6006..309478cbe14c 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > @@ -397,6 +397,18 @@ SecPlatformInformation2 ( > OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2 > ); > > +/** > + Migrates the Global Descriptor Table (GDT) to permanent memory. > + > + @retval EFI_SUCCESS The GDT was migrated successfully. > + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory. > + > +**/ > +EFI_STATUS > +MigrateGdt ( > + VOID > + ); > + > /** > Initializes MP and exceptions handlers. > > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c > index 07ccbe7c6a91..d07540cf7471 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c > @@ -429,6 +429,43 @@ GetGdtr ( > AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer); > } > > +/** > + Migrates the Global Descriptor Table (GDT) to permanent memory. > + > + @retval EFI_SUCCESS The GDT was migrated successfully. > + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory. > + > +**/ > +EFI_STATUS > +MigrateGdt ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + UINTN GdtBufferSize; > + IA32_DESCRIPTOR Gdtr; > + VOID *GdtBuffer; > + > + AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr); > + GdtBufferSize = sizeof (IA32_SEGMENT_DESCRIPTOR) -1 + Gdtr.Limit + 1; > + > + Status = PeiServicesAllocatePool ( > + GdtBufferSize, > + &GdtBuffer > + ); > + ASSERT (GdtBuffer != NULL); > + if (EFI_ERROR (Status)) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_SEGMENT_DESCRIPTOR)); > + CopyMem (GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1); > + Gdtr.Base = (UINTN) GdtBuffer; > + AsmWriteGdtr (&Gdtr); > + > + return EFI_SUCCESS; > +} > + > /** > Initializes CPU exceptions handlers for the sake of stack switch requirement. > > diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c > index a462e7ee1e38..3bf0574b34c6 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c > +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c > @@ -602,8 +602,16 @@ MemoryDiscoveredPpiNotifyCallback ( > IN VOID *Ppi > ) > { > - EFI_STATUS Status; > - BOOLEAN InitStackGuard; > + EFI_STATUS Status; > + BOOLEAN InitStackGuard; > + BOOLEAN InterruptState; > + > + if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) { > + InterruptState = SaveAndDisableInterrupts (); > + Status = MigrateGdt (); > + ASSERT_EFI_ERROR (Status); > + SetInterruptState (InterruptState); > + } > > // > // Paging must be setup first. Otherwise the exception TSS setup during MP >