Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Pin Linuxboot to Commit and Separate EDK2 into a Separate Module #1047

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions boards/qemu-linuxboot/qemu-linuxboot.config
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ export CONFIG_LINUX_VERSION=4.14.62
CONFIG_LINUXBOOT_BOARD=qemu
CONFIG_LINUX_CONFIG=config/linux-linuxboot.config

CONFIG_EDK2_PLATFORM=X64

ifeq "$(CONFIG_UROOT)" "y"
CONFIG_BUSYBOX=n
else
Expand Down
22 changes: 22 additions & 0 deletions modules/edk2
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# EDK2 is required by Linuxboot.
modules-$(CONFIG_LINUXBOOT) += edk2

edk2_version := b7c299e4799012e8a20958a68693ab95b7626aa9
edk2_hash := 78f48f46414b019b5fcd2d7049fd79e955d25f2965123d0bbdf7226564db33e5
edk2_tar := edk2-$(edk2_version).tar.gz
edk2_dir := edk2-$(edk2_version)
edk2_url := https://github.com/linuxboot/edk2/archive/$(edk2_version).tar.gz
edk2_depends := $(musl_dep)

# Use musl for compilation to avoid errors with GCC and create a dummy
# .git file so Linuxboot doesn't try to clone EDK2 again.
edk2_configure := \
cd $(build)/$(edk2_dir) \
&& source edksetup.sh \
&& sed -i \
's|DEFINE GCC5_$(CONFIG_EDK2_PLATFORM)_PREFIX = ENV(GCC5_BIN)|DEFINE GCC5_$(CONFIG_EDK2_PLATFORM)_PREFIX = $(CROSS)|' \
Conf/tools_def.txt \
&& touch .git

# EDK2 will be built via the Linuxboot module, so no build target here.
edk2_target := nothing || true
16 changes: 7 additions & 9 deletions modules/linuxboot
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
modules-$(CONFIG_LINUXBOOT) += linuxboot

linuxboot_version := git
linuxboot_repo := https://github.com/osresearch/linuxboot
linuxboot_version := f0ea0af8609e235631f1fcb65a4f7eb69f28ea26
linuxboot_hash := a814a12272718af74fead5dbf6f24491f30c259f56c074775e86f1572b2e2990
linuxboot_tar := linuxboot-$(linuxboot_version).tar.gz
linuxboot_url := https://github.com/osresearch/linuxboot/archive/$(linuxboot_version).tar.gz
linuxboot_base_dir := linuxboot-$(linuxboot_version)
linuxboot_depends := edk2

# linuxboot builds are specialized on a per-target basis.
# They can be specialized by defining $(CONFIG_LINUXBOOT_BOARD)
Expand All @@ -11,13 +14,8 @@ linuxboot_base_dir := linuxboot-$(linuxboot_version)
linuxboot_board := $(or $(CONFIG_LINUXBOOT_BOARD),$(BOARD))
linuxboot_dir := linuxboot-$(linuxboot_version)/build/$(linuxboot_board)

linuxboot_configure := \
echo >&2 "Pre-building edk2" ; \
$(MAKE) \
-C $(build)/$(linuxboot_base_dir) \
BOARD=$(linuxboot_board) \
edk2.force \
|| exit 1 ; \
linuxboot_configure = \
ln -sf ../edk2-$(edk2_version) $(build)/$(linuxboot_base_dir)/edk2 ; \
if [ "$(linuxboot_board)" = "qemu" ]; then \
echo >&2 "Pre-building edk2 OVMF" ; \
( cd $(build)/$(linuxboot_base_dir)/edk2/OvmfPkg ; \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
From 1d212a83df0eaf32a6f5d4159beb2d77832e0231 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <[email protected]>
Date: Fri, 2 Mar 2018 17:11:52 +0100
Subject: [PATCH 01/11] BaseTools/header.makefile: add
"-Wno-stringop-truncation"

gcc-8 (which is part of Fedora 28) enables the new warning
"-Wstringop-truncation" in "-Wall". This warning is documented in detail
at <https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html>; the
introduction says

> Warn for calls to bounded string manipulation functions such as strncat,
> strncpy, and stpncpy that may either truncate the copied string or leave
> the destination unchanged.

It breaks the BaseTools build with:

> EfiUtilityMsgs.c: In function 'PrintMessage':
> EfiUtilityMsgs.c:484:9: error: 'strncat' output may be truncated copying
> between 0 and 511 bytes from a string of length 511
> [-Werror=stringop-truncation]
> strncat (Line, Line2, MAX_LINE_LEN - strlen (Line) - 1);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> EfiUtilityMsgs.c:469:9: error: 'strncat' output may be truncated copying
> between 0 and 511 bytes from a string of length 511
> [-Werror=stringop-truncation]
> strncat (Line, Line2, MAX_LINE_LEN - strlen (Line) - 1);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> EfiUtilityMsgs.c:511:5: error: 'strncat' output may be truncated copying
> between 0 and 511 bytes from a string of length 511
> [-Werror=stringop-truncation]
> strncat (Line, Line2, MAX_LINE_LEN - strlen (Line) - 1);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The right way to fix the warning would be to implement string concat with
snprintf(). However, Microsoft does not appear to support snprintf()
before VS2015
<https://stackoverflow.com/questions/2915672/snprintf-and-visual-studio-2010>,
so we just have to shut up the warning. The strncat() calls flagged above
are valid BTW.

Cc: Ard Biesheuvel <[email protected]>
Cc: Cole Robinson <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Yonghong Zhu <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <[email protected]>
Reviewed-by: Liming Gao <[email protected]>
---
BaseTools/Source/C/Makefiles/header.makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index 0976973bdd..550f8b35bc 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -71,9 +71,9 @@ INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I $(MAKE
BUILD_CPPFLAGS = $(INCLUDE) -O2
ifeq ($(DARWIN),Darwin)
# assume clang or clang compatible flags on OS X
-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -c -g
+BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-self-assign -Wno-unused-result -nostdlib -c -g
else
-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-unused-result -nostdlib -c -g
+BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-unused-result -nostdlib -c -g
endif
BUILD_LFLAGS =
BUILD_CXXFLAGS = -Wno-unused-result
--
2.25.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
From 9de306701312f986c9638cb819d3f1f848d55cab Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <[email protected]>
Date: Fri, 2 Mar 2018 17:11:52 +0100
Subject: [PATCH] BaseTools/GenVtf: silence false "stringop-overflow" warning
with memcpy()

gcc-8 (which is part of Fedora 28) enables the new warning
"-Wstringop-overflow" in "-Wall". This warning is documented in detail at
<https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html>; the
introduction says

> Warn for calls to string manipulation functions such as memcpy and
> strcpy that are determined to overflow the destination buffer.

It breaks the BaseTools build with:

> GenVtf.c: In function 'ConvertVersionInfo':
> GenVtf.c:132:7: error: 'strncpy' specified bound depends on the length
> of the source argument [-Werror=stringop-overflow=]
> strncpy (TemStr + 4 - Length, Str, Length);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> GenVtf.c:130:14: note: length computed here
> Length = strlen(Str);
> ^~~~~~~~~~~

It is a false positive because, while the bound equals the length of the
source argument, the destination pointer is moved back towards the
beginning of the destination buffer by the same amount (and this amount is
range-checked first, so we can't precede the start of the dest buffer).

Replace both strncpy() calls with memcpy().

Cc: Ard Biesheuvel <[email protected]>
Cc: Cole Robinson <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Yonghong Zhu <[email protected]>
Reported-by: Cole Robinson <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <[email protected]>
Reviewed-by: Liming Gao <[email protected]>
---
BaseTools/Source/C/GenVtf/GenVtf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/GenVtf/GenVtf.c b/BaseTools/Source/C/GenVtf/GenVtf.c
index 65ae08eece..fc7ae02203 100644
--- a/BaseTools/Source/C/GenVtf/GenVtf.c
+++ b/BaseTools/Source/C/GenVtf/GenVtf.c
@@ -129,9 +129,9 @@ Returns:
} else {
Length = strlen(Str);
if (Length < 4) {
- strncpy (TemStr + 4 - Length, Str, Length);
+ memcpy (TemStr + 4 - Length, Str, Length);
} else {
- strncpy (TemStr, Str + Length - 4, 4);
+ memcpy (TemStr, Str + Length - 4, 4);
}

sscanf (
--
2.25.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
From 88e8498f8a72cff1f7af6852ec8166772913399e Mon Sep 17 00:00:00 2001
From: Liming Gao <[email protected]>
Date: Thu, 1 Nov 2018 22:35:29 +0800
Subject: [PATCH] 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.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <[email protected]>
Cc: Yonghong Zhu <[email protected]>
Reviewed-by: Yonghong Zhu <[email protected]>
---
BaseTools/Conf/tools_def.template | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index a22b96c0b8..e0e68fd7fb 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4253,7 +4253,7 @@ DEFINE GCC48_AARCH64_ASLDLINK_FLAGS = DEF(GCC47_AARCH64_ASLDLINK_FLAGS)
DEFINE GCC49_IA32_CC_FLAGS = DEF(GCC48_IA32_CC_FLAGS)
DEFINE GCC49_X64_CC_FLAGS = DEF(GCC48_X64_CC_FLAGS)
DEFINE GCC49_IA32_X64_DLINK_COMMON = -nostdlib -Wl,-n,-q,--gc-sections -z common-page-size=0x40
-DEFINE GCC49_IA32_X64_ASLDLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_COMMON) -Wl,--entry,ReferenceAcpiTable -u ReferenceAcpiTable
+DEFINE GCC49_IA32_X64_ASLDLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_COMMON) -Wl,--defsym=PECOFF_HEADER_SIZE=0 DEF(GCC_DLINK2_FLAGS_COMMON) -Wl,--entry,ReferenceAcpiTable -u ReferenceAcpiTable
DEFINE GCC49_IA32_X64_DLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_COMMON) -Wl,--entry,$(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map,--whole-archive
DEFINE GCC49_IA32_DLINK2_FLAGS = DEF(GCC48_IA32_DLINK2_FLAGS)
DEFINE GCC49_X64_DLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_FLAGS) -Wl,-melf_x86_64,--oformat=elf64-x86-64,-pie
--
2.25.1