This is a regression issue caused by 2ec7953d49677142c5f7552e9e3d96fb406ba0c4.
In Decode() function, once mOutBuf is fully filled, Decode() should return.
Current logic misses the checker of mOutBuf after while() loop.
This is a regression issue caused by 041d89bc0f0119df37a5fce1d0f16495ff905089.
In Decode() function, once mOutBuf is fully filled, Decode() should return.
Current logic misses the checker of mOutBuf after while() loop.
Text format for AcpiEx device path in UEFI Spec:
AcpiEx(HID,CID,UID,HIDSTR,CIDSTR,UIDSTR)
AcpiEx(HID|HIDSTR,(CID|CIDSTR,UID|UIDSTR))(Display Only)
When convert device path to text for AcpiEx device path,
current code check AllowShortcuts parameter to convert
the device path to DisplayOnly text format(shorter text
representation) by mistake.
It should check DisplayOnly parameter.
This commit is to fix this issue.
Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Dandan Bi <dandan.bi@intel.com> Reviewed-by: Liming Gao <liming.gao@intel.com>
Eric Dong [Sat, 10 Nov 2018 02:53:41 +0000 (10:53 +0800)]
UefiCpuPkg/PiSmmCpuDxeSmm: Separate semaphore container.
In current implementation, core and package level sync uses same semaphores.
Sharing the semaphore may cause wrong execution order.
For example:
1. Feature A has CPU_FEATURE_CORE_BEFORE dependency with Feature B.
2. Feature C has CPU_FEATURE_PACKAGE_AFTER dependency with Feature B.
The expected feature initialization order is A B C:
A ---- (Core Depends) ----> B ---- (Package Depends) ----> C
For a CPU has 1 package, 2 cores and 4 threads. The feature initialization
order may like below:
In above case, for thread#4, when it syncs in core level, Wait(S4) * 2 isn't
blocked and [B.Init] runs. But [A.Init] hasn't run in thread#3. It's wrong!
Thread#4 should execute [B.Init] after thread#3 executes [A.Init] because B
core level depends on A.
The reason of the wrong execution order is that S4 is released in thread#1
by calling Release (S1, S2, S3, S4) and in thread #4 by calling
Release (S3, S4).
To fix this issue, core level sync and package level sync should use separate
semaphores.
In above example, the S4 released in Release (S1, S2, S3, S4) should not be the
same semaphore as that in Release (S3, S4).
Related BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1311
Cc: Laszlo Ersek <lersek@redhat.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com> Acked-by: Laszlo Ersek <lersek@redhat.com>
Eric Dong [Thu, 8 Nov 2018 01:06:33 +0000 (09:06 +0800)]
UefiCpuPkg/RegisterCpuFeaturesLib: Separate semaphore container.
In current implementation, core and package level sync uses same semaphores.
Sharing the semaphore may cause wrong execution order.
For example:
1. Feature A has CPU_FEATURE_CORE_BEFORE dependency with Feature B.
2. Feature C has CPU_FEATURE_PACKAGE_AFTER dependency with Feature B.
The expected feature initialization order is A B C:
A ---- (Core Depends) ----> B ---- (Package Depends) ----> C
For a CPU has 1 package, 2 cores and 4 threads. The feature initialization
order may like below:
In above case, for thread#4, when it syncs in core level, Wait(S4) * 2 isn't
blocked and [B.Init] runs. But [A.Init] hasn't run in thread#3. It's wrong!
Thread#4 should execute [B.Init] after thread#3 executes [A.Init] because B
core level depends on A.
The reason of the wrong execution order is that S4 is released in thread#1
by calling Release (S1, S2, S3, S4) and in thread #4 by calling
Release (S3, S4).
To fix this issue, core level sync and package level sync should use separate
semaphores.
In above example, the S4 released in Release (S1, S2, S3, S4) should not be the
same semaphore as that in Release (S3, S4).
Related BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1311
Cc: Laszlo Ersek <lersek@redhat.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com> Acked-by: Laszlo Ersek <lersek@redhat.com>
Eric Dong [Fri, 9 Nov 2018 05:20:41 +0000 (13:20 +0800)]
UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order.
V2 changes:
V1 change has regression which caused by change feature order.
V2 changes logic to detect dependence not only for the
neighborhood features. It need to check all features in the list.
V1 Changes:
In current code logic, only adjust feature position if current
CPU feature position not follow the request order. Just like
Feature A need to be executed before feature B, but current
feature A registers after feature B. So code will adjust the
position for feature A, move it to just before feature B. If
the position already met the requirement, code will not adjust
the position.
This logic has issue when met all below cases:
1. feature A has core or package level dependence with feature B.
2. feature A is register before feature B.
3. Also exist other features exist between feature A and B.
Root cause is driver ignores the dependence for this case, so
threads may execute not follow the dependence order.
Fix this issue by change code logic to adjust feature position
for CPU features which has dependence relationship.
Related BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1311
Cc: Laszlo Ersek <lersek@redhat.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ruiyu Ni <Ruiyu.ni@intel.com>
Dandan Bi [Wed, 7 Nov 2018 08:14:22 +0000 (16:14 +0800)]
MdeModulePkg/DisplayEngine: Remove useless NULL ptr check for NewPos
In function UiDisplayMenu, the NewPos ptr which used to point to the
highlight menu entry. It will always point to the menu entry which
need to be highlighted or the gMenuOption menu if the highlight menu
is not found.
So we can remove the NULL ptr check for NewPos in this function.
And add the ASSERT code to avoid if any false positive reports
of NULL pointer dereference issue raised from static analysis.
Cc: Liming Gao <liming.gao@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Hao Wu <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Dandan Bi <dandan.bi@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Liming Gao <liming.gao@intel.com>
Line #3 and Line #6 both use EAX as destination register.
Line #c uses EAX and (EAX).
The output operand "=a" tells GCC that EAX is used for output.
But GCC only assumes that EAX will be used in the very last
instruction.
Per GCC document,
"Use the '&' constraint modifier on all output operands that must
not overlap an input. Otherwise, GCC may allocate the output
operand in the same register as an unrelated input operand, on
the assumption that the assembler code consumes its inputs before
producing outputs. This assumption may be false if the assembler
code actually consists of more than one instruction."
"=&a" should be used to tell GCC not use EAX before the assembly.
Jiewen Yao [Sat, 3 Nov 2018 11:59:01 +0000 (19:59 +0800)]
UefiCpuPkg/SmmCpu: Block access-out only when static paging is used
When static paging is disabled, page table for below 4GB is created
and page table for above 4GB is created dynamically in page fault
handler.
Today's implementation only allow SMM access-out to below types of
memory address no matter static paging is enabled or not:
1. Reserved, run time and ACPI NVS type
2. MMIO
But certain platform feature like RAS may need to access other types
of memory from SMM. Today's code blocks these platforms.
This patch simplifies the policy to only block when static paging
is used so that the static paging can be disabled in these platforms
to meet their SMM access-out need.
Setting PcdCpuSmmStaticPageTable to FALSE can disable the static
paging.
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Eric Dong <eric.dong@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> Acked-by: Laszlo Ersek <lersek@redhat.com>
Setting GICD_IROUTERn.IRM and GICD_IROUTERn.{Aff3,Aff2,Aff1,Aff0}
at the same time is nonsensical (see 8.9.13 in the GICv3 spec, which
says of GICD_IROUTERn.IRM that "When this bit is set to 1,
GICD_IROUTER<n>.{Aff3,Aff2,Aff1,Aff0} are UNKNOWN"). There is also no
guarantee that IRM is implemented (see GICD_TYPER.No1N which indicates
whether the implementation supports this or not).
Let's thus not set this bit, as we want all SPIs to be delivered to the
same CPU, and not be broadcast to all of them.
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ming Huang <ming.huang@linaro.org> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
This issue originates from following patch which allows to enable
paging if PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy
(in addition to PcdSetNxForStack) are set to enable related features.
Due to above change, PcdImageProtectionPolicy will be set to 0 by
default in many platforms, which, in turn, cause following code in
MdeModulePkg\Core\Dxe\Misc\MemoryProtection.c fail the creation of
notify event of CpuArchProtocol.
The solution is removing the if(...) statement so that the notify
event will always be created and registered. This won't cause
unnecessary code execution because, in the notify event handler,
the related PCDs like
PcdImageProtectionPolicy and
PcdDxeNxMemoryProtectionPolicy
will be checked again before doing related jobs.
Cc: Star Zeng <star.zeng@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Leif Lindholm <leif.lindholm@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com>
Jian J Wang [Sat, 3 Nov 2018 05:58:28 +0000 (13:58 +0800)]
MdeModulePkg/Core: fill logic hole in MemoryProtectionCpuArchProtocolNotify
At the end of of MemoryProtectionCpuArchProtocolNotify there's cleanup
code to free resource. But at line 978, 994, 1005 the function returns
directly. This patch use "goto" to replace "return" to make sure the
resource is freed before exit.
1029: CoreCloseEvent (Event);
1030: return;
There's another memory leak after calling gBS->LocateHandleBuffer() in
the same function:
Status = gBS->LocateHandleBuffer (
ByProtocol,
&gEfiLoadedImageProtocolGuid,
NULL,
&NoHandles,
&HandleBuffer
);
HandleBuffer is allocated in above call but never freed. This patch
will also add code to free it.
Cc: Star Zeng <star.zeng@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Leif Lindholm <leif.lindholm@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com>
Star Zeng [Tue, 6 Nov 2018 13:47:39 +0000 (21:47 +0800)]
Revert "XhciDxe: Use common buffer for AsyncInterruptTransfer"
There is concern at the thread
https://lists.01.org/pipermail/edk2-devel/2018-November/031951.html.
And the time point is a little sensitive as it is near edk2-stable201811.
Star Zeng [Tue, 6 Nov 2018 13:45:25 +0000 (21:45 +0800)]
Revert "EhciDxe: Use common buffer for AsyncInterruptTransfer"
There is concern at the thread
https://lists.01.org/pipermail/edk2-devel/2018-November/031951.html.
And the time point is a little sensitive as it is near edk2-stable201811.
The VMWare SVGA display device implemented by Qemu (-vga vmware) uses
an I/O-type BAR which is laid out such that some register offsets are
not aligned to the read/write width with which they are expected to be
accessed. However, we reverted the initialization of VMWare SVGA device,
we don't need such unaligned I/O.
The VMWare SVGA model now -- since commit 104bd1dc70 in QEMU --
falls back to stdvga (that is, Bochs) if we don't setup VMWare SVGA
FIFO.
To simplify QemuVideoDxe, we don't intend to implement the VMWare SVGA
FIFO setup feature. It means our current VMW SVGA driver code is
basically dead. To simplify the problem, we will replace the old
VMWare SVGA driver to Bochs interface. It should work on all QEMU
version.
The first step for using Bochs interface is to revert old driver.
The VMWare SVGA display device implemented by Qemu (-vga vmware) uses
an I/O-type BAR which is laid out such that some register offsets are
not aligned to the read/write width with which they are expected to be
accessed. However, we will revert the initialization of VMWare SVGA
device later, we don't need such unaligned I/O.
Jiaxin Wu [Mon, 5 Nov 2018 06:52:30 +0000 (14:52 +0800)]
ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.
[-c <retry count>] is to define the number of times to transmit request
packets and wait for a response. The default value is 6. But it doesn't
specify the behavior of zero value. Here, The patch is to clear that:
Set to zero also means to use the default value.
Cc: Carsey Jaben <jaben.carsey@intel.com> Cc: Ye Ting <ting.ye@intel.com> Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com> Reviewed-by: Fu Siyuan <siyuan.fu@intel.com> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Fu Siyuan [Tue, 30 Oct 2018 06:49:42 +0000 (14:49 +0800)]
OvmfPkg: Replace obsoleted network drivers from platform DSC/FDF.
V2:
Add missed library instance for NetworkPkg iSCSI driver.
This patch replaces the MdeModulePkg TCP, PXE and iSCSI driver with those
ones in NetworkPkg. These 3 drivers in MdeModulePkg are not being actively
maintained and will be removed from edk2 master soon.
Fu Siyuan [Tue, 30 Oct 2018 06:51:16 +0000 (14:51 +0800)]
Vlv2TbltDevicePkg: Replace obsoleted drivers from platform DSC/FDF.
V2:
Add missed library instance for NetworkPkg iSCSI driver.
This patch replaces the MdeModulePkg TCP, PXE and iSCSI driver with those
ones in NetworkPkg. These 3 drivers in MdeModulePkg are not being actively
maintained and will be removed from edk2 master soon.
Cc: David Wei <david.wei@intel.com> Cc: Mang Guo <mang.guo@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Fu Siyuan <siyuan.fu@intel.com> Reviewed-by: David Wei <david.wei@intel.com>
Fu Siyuan [Tue, 30 Oct 2018 06:47:33 +0000 (14:47 +0800)]
EmulatorPkg: Replace obsoleted network drivers from platform DSC/FDF.
This patch replaces the MdeModulePkg TCP, PXE and iSCSI driver with those
ones in NetworkPkg. These 3 drivers in MdeModulePkg are not being actively
maintained and will be removed from edk2 master soon.
Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Andrew Fish <afish@apple.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Fu Siyuan <siyuan.fu@intel.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Fu Siyuan [Tue, 30 Oct 2018 06:43:37 +0000 (14:43 +0800)]
Nt32Pkg: Replace obsoleted network drivers from NT32 platform DSC/FDF.
This patch replaces the MdeModulePkg TCP, PXE and iSCSI driver with those
ones in NetworkPkg. These 3 drivers in MdeModulePkg are not being actively
maintained and will be removed from edk2 master soon.
Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Hao Wu <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Fu Siyuan <siyuan.fu@intel.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
shenglei [Mon, 5 Nov 2018 03:08:07 +0000 (11:08 +0800)]
CorebootPayloadPkg: Remove EdkShellBinPkg in FDF
Remove EdkShellBinPkg in CorebootPayloadPkg.fdf.
https://bugzilla.tianocore.org/show_bug.cgi?id=1108
v3:Remove FULL_BIN and change SHELL_TYPE from FULL_BIN
to UEFI_BIN.
Cc: Maurice Ma <maurice.ma@intel.com> Cc: Prince Agyeman <prince.agyeman@intel.com> Cc: Benjamin You <benjamin.you@intel.com>
Change-Id: I4db7068a3a1f68a1f6303079b73dc548c9feb2e3
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com> Reviewed-by: Benjamin You <benjamin.you@intel.com>
BobCF [Fri, 2 Nov 2018 05:56:38 +0000 (13:56 +0800)]
BaseTools: Use common cc flag for building PcdValueInit.
V3:
Update the patch to avoid "reduce" function fail.
V2:
Support to extract the common cc flag from a
combined cc flag string. For example
MSFT:*_*_IA32_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES /DDEF_IA32
MSFT:*_*_X64_CC_FLAGS = /DDEF_X64 /D DISABLE_NEW_DEPRECATED_INTERFACES
V1:
Use common cc flags for building PcdValueInit. The common
cc flags include the cc flag which is under common arch and
under all build arches.
Liming Gao [Fri, 2 Nov 2018 15:17:07 +0000 (23:17 +0800)]
BaseTools build_rule.template: Update aslc rule for XCODE tool chain
Update aslc rule to rename the temp output file from .efi to .pecoff.
This change can avoid the conflict .efi file name in output directory.
One is the driver image, another is aslc temp output file.
ShellPkg-ShellApp: Provide fully-qualified path to shell scripts
Provide fully-qualified path to shell scripts
Section 3.6.2 of version 2.2 of the shell specification requires that
the first positional argument (i.e. arg 0) of a shell script resolves
to "the full path name of the script itself."
Ensure that the startup script and any scripts launched by the shell
meet this requirement.
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jim Dailey <jim_dailey@dell.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
ShellPkg-UefiShellLib: Add a function to fully-qualify paths
Add a function to return a clean, fully-qualified version of some path.
This function handles a (possibly "dirty") input path that may or may
not include a file system reference.
If it does not include a file system reference, then if the input path
does not begin with a forward or backward slash, then the input path is
relative to the current working directory of the current file system.
Otherwise, it is an absolute path within the current file system.
If it does include a file system reference, it may be a reference to the
current or some other file system. If the file system reference is not
immediately followed by a forward or backward slash, then the input path
is relative to the current working directory of the given file system.
Otherwise, it is an absolute path within the given file system.
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jim Dailey <jim_dailey@dell.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Dong, Eric [Fri, 2 Nov 2018 08:27:48 +0000 (16:27 +0800)]
UefiCpuPkg/MpInitLib: Rollback old change 2a5997f8.
In some special cases, after BSP sends Init-sipi-sipi signal
AP needs more time to start the Ap procedure. In this case
BSP may think AP has finished its task but in fact AP hasn't began
yet.
Rollback former change to keep the status which only be used
when AP really finished task.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Zhao, ZhiqiangX [Wed, 31 Oct 2018 10:35:47 +0000 (18:35 +0800)]
BaseTools: Check the max size for string PCD.
According to PCD_DATABASE_INIT in
edk2\MdeModulePkg\Include\Guid\PcdDataBaseSignatureGuid.h,
the max size for string PCD should not exceed USHRT_MAX 65535(0xffff).
MdePkg/Base.h: Implement BASE_CR() via OFFSET_OF().
Replace the current NULL pointer dereference to retrieve Field's
offset with a call to OFFSET_OF(). This is implemented via
__builtin_offsetof for GCC and Clang, which eliminates UB caught by
Clang UndefinedBehaviorSanitizer.
Liming Gao [Thu, 1 Nov 2018 14:35:29 +0000 (22:35 +0800)]
BaseTools tools_def.template: Add GCC link script option in ASLDLINK_FLAGS
GCC link script is used to discard the unused section data from ELF image.
ASLDLINK_FLAGS requires it to remove the unnecessary section data, then
GenFw can be used to retrieve the correct data section from ELF image.
Ruiyu Ni [Mon, 22 Oct 2018 09:03:07 +0000 (17:03 +0800)]
MdeModulePkg/PiSmmIpl: Do not reset SMRAM to UC when CPU driver runs
Today's PiSmmIpl implementation initially sets SMRAM to WB to speed
up the SMM core/modules loading before SMM CPU driver runs.
When SMM CPU driver runs, PiSmmIpl resets the SMRAM to UC. It's done
in SmmIplDxeDispatchEventNotify(). COMM_BUFFER_SMM_DISPATCH_RESTART
is returned from SMM core that SMM CPU driver is just dispatched.
Since now the SMRR is widely used to control the SMRAM cache setting.
It's not needed to reset the SMRAM to UC anymore.
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> Cc: Michael Kinney <michael.d.kinney@intel.com>
Current implementation failed to parse the multiple TLS record
messages due to the incorrect pointer of TLS record header. This
patch is to resolve that problem.
Cc: Ye Ting <ting.ye@intel.com> Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com> Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
The LookupFreeMemRegion() in RuntimeMemAllocate.c is used to look-up
free memory region for runtime resource allocation, which was designed
to support runtime authenticated variable service.
The ReqPages in this function is the required pages to be allocated,
which depends on the malloc() call in internal OpenSSL routines. The
direct offset subtractions on ReqPages may bring possible integer
overflow issue.
This patch is to add the extra parameter checks to remove this possible
overflow risk.
Cc: Ye Ting <ting.ye@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Long Qin <qin.long@intel.com> Reviewed-by: Ye Ting <ting.ye@intel.com>
If the contents in 'PreviousFile' and 'File' are the same, call to
"CleanupFileInformation (&PreviousFile);" will free the buffers in 'File'
as well. This will lead to potential memory double free/use after free
issues.
This commit will add additional check to address the above issue.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu <hao.a.wu@intel.com> Reviewed-by: Paulo Alcantara <palcantara@suse.de> Reviewed-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
The content within 'File' is the output data for ResolveSymlink(). This
commit will add checks to ensure the content in 'File' is valid.
Otherwise, possible null pointer dereference issue will occur during the
subsequent usage of the data returned by ResolveSymlink().
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu <hao.a.wu@intel.com> Reviewed-by: Paulo Alcantara <palcantara@suse.de> Reviewed-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
According to the ECMA-167 standard (3rd Edition - June 1997), Section
14.16.1.1, valid values are 1 to 5. All other values will be treated as a
corrupted volume.
This commit will add such check within function ResolveSymlink().
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu <hao.a.wu@intel.com> Reviewed-by: Paulo Alcantara <palcantara@suse.de> Reviewed-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Marvin H?user [Sun, 28 Oct 2018 08:51:23 +0000 (16:51 +0800)]
UefiCpuPkg/PiSmmCpuDxeSmm: Fix ASSERT for success.
Index is initialized to MAX_UINT16 as default failure value, which
is what the ASSERT is supposed to test for. The ASSERT condition
however can never return FALSE for INT16 != int, as due to
Integer Promotion[1], Index is converted to int, which can never
result in -1.
Furthermore, Index is used as a for loop index variable inbetween its
initialization and the ASSERT, so the value is unconditionally
overwritten too.
Fix the ASSERT check to compare Index to its upper boundary, which it
will be equal to if the loop was not broken out of on success.
Marvin H?user [Sun, 28 Oct 2018 08:51:22 +0000 (16:51 +0800)]
UefiCpuPkg/MpInitLib: Fix ASSERT for success.
Index is initialized to MAX_UINT16 as default failure value, which
is what the ASSERT is supposed to test for. The ASSERT condition
however can never return FALSE for INT16 != int, as due to
Integer Promotion[1], Index is converted to int, which can never
result in -1.
Furthermore, Index is used as a for loop index variable inbetween its
initialization and the ASSERT, so the value is unconditionally
overwritten too.
Fix the ASSERT check to compare Index to its upper boundary, which it
will be equal to if the loop was not broken out of on success.
The *MemorySpaceMap assigned with NULL (line 1710) value might be
accessed (line 1726/1730) without any sanity check. Although it won't
happen in practice because of line 1722, we still need to add check
against NULL to make static code analyzer happy.
Ruiyu Ni [Thu, 25 Oct 2018 10:09:46 +0000 (18:09 +0800)]
MdeModulePkg/UsbBusPei: Reject descriptor whose length is bad
Today's implementation doesn't check whether the length of
descriptor is valid before using it.
The patch fixes this issue by syncing the similar fix to UsbBusDxe. 70c3c2370a2aefe71cf0f6c1a1e063f7d74e1d79
*MdeModulePkg/UsbBus: Reject descriptor whose length is bad
Additionally the patch also rejects the data when length is
larger than sizeof (PeiUsbDevice->ConfigurationData).
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com>
Ruiyu Ni [Thu, 25 Oct 2018 09:07:12 +0000 (17:07 +0800)]
MdeModulePkg/UsbBusPei: Fix out-of-bound read access to descriptors
Today's implementation reads the Type/Length field in the USB
descriptors data without checking whether the offset to read is
beyond the data boundary.
The patch fixes this issue by syncing the fix in commit 4c034bf62cbc1f3c5f4b5df25de97f0f528132b2
*MdeModulePkg/UsbBus: Fix out-of-bound read access to descriptors
ParsedBytes in UsbBusPei.GetExpectedDescriptor() is different from
Consumed in UsbBusDxe.UsbCreateDesc().
ParsedBytes is the offset of found descriptor while Consumed is
offset of next descriptor of found one.
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com>
In current code, EhcMonitorAsyncRequests (timer handler) will do
unmap and map operations for AsyncIntTransfers to "Flush data from
PCI controller specific address to mapped system memory address".
EhcMonitorAsyncRequests
EhcFlushAsyncIntMap
PciIo->Unmap
IoMmu->SetAttribute
PciIo->Map
IoMmu->SetAttribute
This may impact the boot performance.
Since the data buffer for EhcMonitorAsyncRequests is internal
buffer, we can allocate common buffer by PciIo->AllocateBuffer
and map the buffer with EfiPciIoOperationBusMasterCommonBuffer,
then the unmap and map operations can be removed.
///
/// Provides both read and write access to system memory by
/// both the processor and a bus master. The buffer is coherent
/// from both the processor's and the bus master's point of view.
///
EfiPciIoOperationBusMasterCommonBuffer,
Test done:
USB KB works normally.
USB disk read/write works normally.
Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Hao Wu <hao.a.wu@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
In current code, XhcMonitorAsyncRequests (timer handler) will do
unmap and map operations for AsyncIntTransfers to "Flush data from
PCI controller specific address to mapped system memory address".
XhcMonitorAsyncRequests
XhcFlushAsyncIntMap
PciIo->Unmap
IoMmu->SetAttribute
PciIo->Map
IoMmu->SetAttribute
This may impact the boot performance.
Since the data buffer for XhcMonitorAsyncRequests is internal
buffer, we can allocate common buffer by PciIo->AllocateBuffer
and map the buffer with EfiPciIoOperationBusMasterCommonBuffer,
then the unmap and map operations can be removed.
///
/// Provides both read and write access to system memory by
/// both the processor and a bus master. The buffer is coherent
/// from both the processor's and the bus master's point of view.
///
EfiPciIoOperationBusMasterCommonBuffer,
Test done:
USB KB works normally.
USB disk read/write works normally.
Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Hao Wu <hao.a.wu@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
When update contents in HiiDatabase, like:
1. Add/update/remove package list
2. Add/update string
3. Add/update image
We should make these operations atomic to prevent the
potential issue that the one update operation with
higher TPL may interrupt another.
This commit is to make the HiiDatabase update behaviors
atomic by adding EfiAcquireLock/EfiReleaseLock function.
Function "HiiGetConfigurationSetting" may be called
when HiiDatabase contents have been updated.
And it is just to call function "HiiGetDatabaseInfo" to
get the contents in HiiDatabase and call function
"HiiGetConfigRespInfo" and get the configuration response
string form HII drivers.
So here we can remove function "HiiGetConfigurationSetting"
and make caller to call "HiiGetDatabaseInfo" and
"HiiGetConfigRespInfo" directly.
And thus it also can distinguish which code blocks are to
operate HiiDatabase contents and which code blocks are not.
Then it's easy to know which code blocks should be atomic
when updating HiiDatabase contents.
Eric Dong [Thu, 25 Oct 2018 06:50:22 +0000 (14:50 +0800)]
UefiCpuPkg/RegisterCpuFeaturesLib: Support combo CPU feature style.
Current code assume only one dependence (before or after) for one
feature. Enhance code logic to support feature has two dependence
(before and after) type.
Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Changes include:
1. Remove extra white space at the end of line.
2. Add comments for the new add function parameter.
3. Update IN OUT tag for function parameter.
Cc: Dandan Bi <dandan.bi@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Jian J Wang [Wed, 24 Oct 2018 04:47:45 +0000 (12:47 +0800)]
MdeModulePkg/Core: add freed-memory guard feature
Freed-memory guard is used to detect UAF (Use-After-Free) memory issue
which is illegal access to memory which has been freed. The principle
behind is similar to pool guard feature, that is we'll turn all pool
memory allocation to page allocation and mark them to be not-present
once they are freed.
This also implies that, once a page is allocated and freed, it cannot
be re-allocated. This will bring another issue, which is that there's
risk that memory space will be used out. To address it, the memory
service add logic to put part (at most 64 pages a time) of freed pages
back into page pool, so that the memory service can still have memory
to allocate, when all memory space have been allocated once. This is
called memory promotion. The promoted pages are always from the eldest
pages which haven been freed.
This feature brings another problem is that memory map descriptors will
be increased enormously (200+ -> 2000+). One of change in this patch
is to update MergeMemoryMap() in file PropertiesTable.c to allow merge
freed pages back into the memory map. Now the number can stay at around
510.
Cc: Star Zeng <star.zeng@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com>
This issue is hidden in current code but exposed by introduction
of freed-memory guard feature due to the fact that the feature
will turn all pool allocation to page allocation.
The solution is moving the memory allocation in CoreGetMemorySpaceMap()
to be out of the GCD memory map lock.
Cc: Star Zeng <star.zeng@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com>
Jian J Wang [Wed, 24 Oct 2018 03:21:04 +0000 (11:21 +0800)]
UefiCpuPkg/CpuDxe: prevent recursive calling of InitializePageTablePool
The freed-memory guard feature will cause a recursive calling
of InitializePageTablePool(). This is due to a fact that
AllocateAlignedPages() is used to allocate page table pool memory.
This function will most likely call gBS->FreePages to free unaligned
pages and then cause another round of page attributes change, like
below (freed pages will be always marked not-present if freed-memory
guard is enabled)
The solution is add a global variable as a lock in page table pool
allocation function and fail any other requests if it has not been
done.
Since this issue will only happen if free-memory guard is enabled,
it won't affect CpuSetMemoryAttributes() in default build of a BIOS.
If free-memory guard is enabled, it only affect the pages
(failed to mark them as not-present) freed in AllocateAlignedPages().
Since those freed pages haven't been used yet (their addresses not
yet exposed to code outside AllocateAlignedPages), it won't compromise
the freed-memory guard feature.
This change will just fail the CpuSetMemoryAttributes() called from
FreePages() but it won't fail the FreePages(). So the error status
won't be propagated upper layer of code.
Cc: Laszlo Ersek <lersek@redhat.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com>
Jian J Wang [Wed, 24 Oct 2018 02:48:37 +0000 (10:48 +0800)]
UefiCpuPkg/CpuDxe: consider freed-memory guard in non-stop mode
Non-stop mode was introduced / explained in commit 8f2613628acf
("MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs",
2018-08-30).
The macro HEAP_GUARD_NONSTOP_MODE was added to CpuDxe in commit dcc026217fdc ("UefiCpuPkg/CpuDxe: implement non-stop mode for uefi",
2018-08-30).
Another instance of the macro HEAP_GUARD_NONSTOP_MODE was added to
PiSmmCpuDxeSmm -- with BIT1|BIT0 replaced with BIT3|BIT2 -- in commit 09afd9a42a7f ("UefiCpuPkg/PiSmmCpuDxeSmm: implement non-stop mode for
SMM", 2018-08-30)
Since the freed-memory guard is for UEFI-only. This patch only updates
HEAP_GUARD_NONSTOP_MODE in "UefiCpuPkg/CpuDxe/CpuDxe.h" (add BIT4).
Cc: Laszlo Ersek <lersek@redhat.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com>
Jian J Wang [Wed, 24 Oct 2018 02:26:07 +0000 (10:26 +0800)]
MdeModulePkg: introduce UEFI freed-memory guard bit in HeapGuard PCD
UAF (Use-After-Free) memory issue is kind of illegal access to memory
which has been freed. It can be detected by a new freed-memory guard
enforced onto freed memory.
BIT4 of following PCD is used to enable the freed-memory guard feature.
Please note this feature is for debug purpose and should not be enabled
in product BIOS, and cannot be enabled with pool/page heap guard at the
same time. It's disabled by default.
Cc: Star Zeng <star.zeng@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Jian J Wang [Wed, 24 Oct 2018 02:14:43 +0000 (10:14 +0800)]
MdeModulePkg: cleanup Heap Guard pool/page type PCD documentation
This cleanup is meant for avoiding misuse of newly introduced BIT4
(UAF detection) of PCD PcdHeapGuardPropertyMask, because it applies
to all types of physical memory. In another words,
PcdHeapGuardPoolType and PcdHeapGuardPageType don't have effect to
the BIT4 of PcdHeapGuardPropertyMask.
Cc: Star Zeng <star.zeng@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Jaben Carsey [Tue, 23 Oct 2018 17:29:22 +0000 (01:29 +0800)]
BaseTools/GenFds: remove MacroDict parameter
The MacroDict parameter goes around in circles through 4 functions without use.
1. GetSectionData calls into GetLeafSection, otherwise doesn?t use MacroDict
2. GetLeafSection calls into GetFileStatement, otherwise doesn?t use MacroDict
3. GetFileStatement calls into GetFilePart, otherwise doesn?t use MacroDict
4. GetFilePart calls into GetSectionData, otherwise doesn?t use MacroDict
Go to 1 and repeat forever.