Merge ~paelzer/ubuntu/+source/ipxe:lp-1882671-fix-tpl-on-efi-entry-FOCAL into ubuntu/+source/ipxe:ubuntu/focal-devel

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4156/+packages

[Christian Ehrhardt  (paelzer)]

Tested the new PPA for Focal, working fine fixing the issue.

I have fully unblocked the Groovy ipxe upload, that should migrate soon.

If ok (or only minor changes needed), I'd appreciate if this could be sponsored into focal-unapproved by the reviewer to not loose the week I'm off.

[Christian Ehrhardt  (paelzer)]

Groovy upload reached -release now.

[Rafael David Tinoco (rafaeldtinoco)]

+1. All good. Change is the same as the one we've discussed in groovy. Much better for SRU'ing indeed.

review: Approve

[Christian Ehrhardt  (paelzer)]

Thanks tagged and uploaded

diff --git a/debian/changelog b/debian/changelog
index e021aec..20fb879 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,7 +1,17 @@
+ipxe (1.0.0+git-20190109.133f4c4-0ubuntu3.2) focal; urgency=medium
+
+  * Revert the changes of the non released version
+    1.0.0+git-20190109.133f4c4-0ubuntu3.1 as there is a less impactful
+    fix more suited for an SRU.
+  * d/p/lp-1882671-efi-Raise-TPL-during-driver-entry-point.patch: fix the
+    formerly avoided efi TPL issues (LP: #1882671)
+
+ -- Christian Ehrhardt <[e-mail]>  Thu, 16 Jul 2020 16:51:30 +0200
+
 ipxe (1.0.0+git-20190109.133f4c4-0ubuntu3.1) focal; urgency=medium
 
   * only enable https on non EFI roms. This lets EFI/OVMF handle https
-    itself and avoids breakage in TPL manipulations (LP: #1882671)
+    itself and avoids breakage in TPL manipulations (LP: 1882671)
     - d/p/enable-https.patch: drop old global way to Enable HTTPS support
     - d/rules: enable https on non EFI roms.
     - d/util/check-rom-sizes: fix if size does exactly match
diff --git a/debian/patches/enable-https.patch b/debian/patches/enable-https.patch
new file mode 100644
index 0000000..5309417
--- /dev/null
+++ b/debian/patches/enable-https.patch
@@ -0,0 +1,19 @@
+Description: enable https
+Forwarded: no (downstream config option)
+Author: James Page <[e-mail]>
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1025239
+Last-Update: 2012-11-14
+
+Index: ipxe-1.0.0+git-20150424.a25a16d/src/config/general.h
+===================================================================
+--- ipxe-1.0.0+git-20150424.a25a16d.orig/src/config/general.h
++++ ipxe-1.0.0+git-20150424.a25a16d/src/config/general.h
+@@ -52,7 +52,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
+ 
+ #define	DOWNLOAD_PROTO_TFTP	/* Trivial File Transfer Protocol */
+ #define	DOWNLOAD_PROTO_HTTP	/* Hypertext Transfer Protocol */
+-#undef	DOWNLOAD_PROTO_HTTPS	/* Secure Hypertext Transfer Protocol */
++#define	DOWNLOAD_PROTO_HTTPS	/* Secure Hypertext Transfer Protocol */
+ #undef	DOWNLOAD_PROTO_FTP	/* File Transfer Protocol */
+ #undef	DOWNLOAD_PROTO_SLAM	/* Scalable Local Area Multicast */
+ #undef	DOWNLOAD_PROTO_NFS	/* Network File System Protocol */
diff --git a/debian/patches/lp-1882671-efi-Raise-TPL-during-driver-entry-point.patch b/debian/patches/lp-1882671-efi-Raise-TPL-during-driver-entry-point.patch
new file mode 100644
index 0000000..de507a9
--- /dev/null
+++ b/debian/patches/lp-1882671-efi-Raise-TPL-during-driver-entry-point.patch
@@ -0,0 +1,65 @@
+From 2ae5d4338661b65c63eb5cb1a96e5b803fe7d620 Mon Sep 17 00:00:00 2001
+From: Michael Brown <[e-mail]>
+Date: Tue, 30 Jun 2020 16:32:59 +0100
+Subject: [PATCH] [efi] Raise TPL during driver entry point
+
+As per commit c89a446 ("[efi] Run at TPL_CALLBACK to protect against
+UEFI timers") we expect to run at TPL_CALLBACK almost all of the time.
+Various code paths rely on this assumption.  Code paths that need to
+temporarily lower the TPL (e.g. for entropy gathering) will restore it
+to TPL_CALLBACK.
+
+The entropy gathering code will be run during DRBG initialisation,
+which happens during the call to startup().  In the case of iPXE
+compiled as an EFI application this code will run within the scope of
+efi_snp_claim() and so will execute at TPL_CALLBACK as expected.
+
+In the case of iPXE compiled as an EFI driver the code will
+incorrectly run at TPL_APPLICATION since there is nothing within the
+EFI driver entry point that raises (and restores) the TPL.  The net
+effect is that a build that includes the entropy-gathering code
+(e.g. a build with HTTPS enabled) will return from the driver entry
+point at TPL_CALLBACK, which causes a system lockup.
+
+Fix by raising and restoring the TPL within the EFI driver entry
+point.
+
+Debugged-by: Ignat Korchagin <[e-mail]>
+Signed-off-by: Michael Brown <[e-mail]>
+
+Origin: backport, https://github.com/ipxe/ipxe/commit/2ae5d4338661b65c63eb5cb1a96e5b803fe7d620
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1882671
+Last-Update: 2020-07-16
+
+---
+ src/interface/efi/efidrvprefix.c | 9 +++++++++
+ 1 file changed, 9 insertions(+)
+
+--- a/src/interface/efi/efidrvprefix.c
++++ b/src/interface/efi/efidrvprefix.c
+@@ -34,16 +34,25 @@ FILE_LICENCE ( GPL2_OR_LATER );
+  */
+ EFI_STATUS EFIAPI _efidrv_start ( EFI_HANDLE image_handle,
+ 				  EFI_SYSTEM_TABLE *systab ) {
++	EFI_BOOT_SERVICES *bs;
++	EFI_TPL saved_tpl;
+ 	EFI_STATUS efirc;
+ 
+ 	/* Initialise EFI environment */
+ 	if ( ( efirc = efi_init ( image_handle, systab ) ) != 0 )
+ 		return efirc;
+ 
++	/* Raise TPL */
++	bs = efi_systab->BootServices;
++	saved_tpl = bs->RaiseTPL ( TPL_CALLBACK );
++
+ 	/* Initialise iPXE environment */
+ 	initialise();
+ 	startup();
+ 
++	/* Restore TPL */
++	bs->RestoreTPL ( saved_tpl );
++
+ 	return 0;
+ }
+ 
diff --git a/debian/patches/series b/debian/patches/series
index fd451e8..5b4384d 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,4 +1,6 @@
 0001-rom-change-banner-timeout.diff.patch
 util-elf2efi-GNU_SOURCE.patch
+enable-https.patch
 handle-dhcp-nack.patch
 0005-strip-802.1Q-VLAN-0-priority-tags.patch
+lp-1882671-efi-Raise-TPL-during-driver-entry-point.patch
diff --git a/debian/rules b/debian/rules
index 9de0eee..22f9a36 100755
--- a/debian/rules
+++ b/debian/rules
@@ -17,29 +17,15 @@ override_dh_auto_configure:
 # This is _a bit_ hacky, it assumes any rom we build are for qemu. Which
 # is true at the moment, but maybe it changes at some point.
 src/bin/%.rom src/bin-x86_64-efi/%.efirom: export CONFIG=qemu
-# qemu EFI roms are built without HTTPS, EFI does that on its own (and better)
-src/bin-x86_64-efi/%.efirom: export NOHTTPS=true
 
 src/bin/% src/bin-x86_64-efi/%:
-	# clean and reconfigure if either NOHTTPS or CONFIG=qemu switches
-	if [ "config:$(CONFIG)nohttps:$(NOHTTPS)" != "$(shell cat debian/last-used-config 2>/dev/null)" ]; then \
-	    if [ "$(NOHTTPS)" = "true" ]; then \
-	        cp -v debian/config/* src/config/local; \
-	    else \
-	        echo "#define DOWNLOAD_PROTO_HTTPS" >> src/config/local/general.h; \
-	    fi; \
-	   $(MAKE) -C src clean; \
-	   echo -n "config:$(CONFIG)" > debian/last-used-config; \
-	   echo -n "nohttps:$(NOHTTPS)" >> debian/last-used-config; \
-	fi;
+	[ "$(CONFIG)" = "$(shell cat debian/current-config 2>/dev/null)" ] || $(MAKE) -C src clean
+	@echo $(CONFIG) > debian/current-config
 	dh_auto_build --sourcedirectory=src -- V=1 NO_WERROR=1 VERSION="$(VERSION)" $(subst src/,,$@)
 
 src/bin-efi/%.efirom: src/bin/%.rom src/bin-x86_64-efi/%.efirom
 	@[ -d $(dir $@) ] || mkdir $(dir $@)
 	src/util/catrom.pl $^ > $@
-	# efi roms are >256k since Bionic due to https + natural growth
-	# stay at 256k-512k after disabling https in efi via padding
-	src/util/padimg.pl --blksize=524288 --byte=0xff --verbose $@
 
 src/bin-efi/%.iso: src/bin/%.lkrn src/bin-x86_64-efi/%.efi
 	@[ -d $(dir $@) ] || mkdir $(dir $@)
@@ -53,7 +39,7 @@ override_dh_install:
 
 override_dh_auto_clean:
 	$(MAKE) -C src veryclean
-	rm -f debian/last-used-config
+	rm -f debian/current-config
 	rm -fr src/bin*
 	rm -fr src/config/local/*
 
diff --git a/debian/util/check-rom-sizes b/debian/util/check-rom-sizes
index d1333fd..31664d1 100755
--- a/debian/util/check-rom-sizes
+++ b/debian/util/check-rom-sizes
@@ -52,15 +52,11 @@ for rom in "${!roms[@]}"; do
     done
     bucket=$((bucket<<1))
     # being smaller could be ok, but check -eq to warn/break on unexpected size loss as well
-    if [ "${size}" -eq "${roms[$rom]}" ]; then
-        echo "OK: ${rom} is exactly ${size} bytes as expected"
+    if [ "${bucket}" -eq "${roms[$rom]}" ]; then
+        echo "OK: ${rom} is ${size} bytes and thereby in bucket ${bucket} which matches ${roms[$rom]}"
     else
-        if [ "${bucket}" -eq "${roms[$rom]}" ]; then
-            echo "OK: ${rom} is ${size} bytes and thereby in bucket ${bucket} which matches ${roms[$rom]}"
-        else
-            echo "ERROR: ${rom} is ${size} bytes and thereby in bucket ${bucket} which does not match ${roms[$rom]}"
-            rc=1
-        fi
+        echo "ERROR: ${rom} is ${size} bytes and thereby in bucket ${bucket} which does not match ${roms[$rom]}"
+        rc=1
     fi
 done