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

Proposed by Christian Ehrhardt 
Status: Merged
Approved by: Christian Ehrhardt 
Approved revision: 4fe18a4b9257c32c080e89e876280d41fc599a40
Merged at revision: 4fe18a4b9257c32c080e89e876280d41fc599a40
Proposed branch: ~paelzer/ubuntu/+source/ipxe:lp-1882671-fix-tpl-on-efi-entry-FOCAL
Merge into: ubuntu/+source/ipxe:ubuntu/focal-devel
Diff against target: 198 lines (+104/-26)
6 files modified
debian/changelog (+11/-1)
debian/patches/enable-https.patch (+19/-0)
debian/patches/lp-1882671-efi-Raise-TPL-during-driver-entry-point.patch (+65/-0)
debian/patches/series (+2/-0)
debian/rules (+3/-17)
debian/util/check-rom-sizes (+4/-8)
Reviewer Review Type Date Requested Status
Rafael David Tinoco (community) Approve
Canonical Server packageset reviewers Pending
Canonical Server Pending
Review via email: mp+387531@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

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.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Groovy upload reached -release now.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

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

review: Approve
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks tagged and uploaded

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index e021aec..20fb879 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,7 +1,17 @@
1ipxe (1.0.0+git-20190109.133f4c4-0ubuntu3.2) focal; urgency=medium
2
3 * Revert the changes of the non released version
4 1.0.0+git-20190109.133f4c4-0ubuntu3.1 as there is a less impactful
5 fix more suited for an SRU.
6 * d/p/lp-1882671-efi-Raise-TPL-during-driver-entry-point.patch: fix the
7 formerly avoided efi TPL issues (LP: #1882671)
8
9 -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Thu, 16 Jul 2020 16:51:30 +0200
10
1ipxe (1.0.0+git-20190109.133f4c4-0ubuntu3.1) focal; urgency=medium11ipxe (1.0.0+git-20190109.133f4c4-0ubuntu3.1) focal; urgency=medium
212
3 * only enable https on non EFI roms. This lets EFI/OVMF handle https13 * only enable https on non EFI roms. This lets EFI/OVMF handle https
4 itself and avoids breakage in TPL manipulations (LP: #1882671)14 itself and avoids breakage in TPL manipulations (LP: 1882671)
5 - d/p/enable-https.patch: drop old global way to Enable HTTPS support15 - d/p/enable-https.patch: drop old global way to Enable HTTPS support
6 - d/rules: enable https on non EFI roms.16 - d/rules: enable https on non EFI roms.
7 - d/util/check-rom-sizes: fix if size does exactly match17 - 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
8new file mode 10064418new file mode 100644
index 0000000..5309417
--- /dev/null
+++ b/debian/patches/enable-https.patch
@@ -0,0 +1,19 @@
1Description: enable https
2Forwarded: no (downstream config option)
3Author: James Page <james.page@ubuntu.com>
4Bug-Ubuntu: https://bugs.launchpad.net/bugs/1025239
5Last-Update: 2012-11-14
6
7Index: ipxe-1.0.0+git-20150424.a25a16d/src/config/general.h
8===================================================================
9--- ipxe-1.0.0+git-20150424.a25a16d.orig/src/config/general.h
10+++ ipxe-1.0.0+git-20150424.a25a16d/src/config/general.h
11@@ -52,7 +52,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
12
13 #define DOWNLOAD_PROTO_TFTP /* Trivial File Transfer Protocol */
14 #define DOWNLOAD_PROTO_HTTP /* Hypertext Transfer Protocol */
15-#undef DOWNLOAD_PROTO_HTTPS /* Secure Hypertext Transfer Protocol */
16+#define DOWNLOAD_PROTO_HTTPS /* Secure Hypertext Transfer Protocol */
17 #undef DOWNLOAD_PROTO_FTP /* File Transfer Protocol */
18 #undef DOWNLOAD_PROTO_SLAM /* Scalable Local Area Multicast */
19 #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
0new file mode 10064420new 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 @@
1From 2ae5d4338661b65c63eb5cb1a96e5b803fe7d620 Mon Sep 17 00:00:00 2001
2From: Michael Brown <mcb30@ipxe.org>
3Date: Tue, 30 Jun 2020 16:32:59 +0100
4Subject: [PATCH] [efi] Raise TPL during driver entry point
5
6As per commit c89a446 ("[efi] Run at TPL_CALLBACK to protect against
7UEFI timers") we expect to run at TPL_CALLBACK almost all of the time.
8Various code paths rely on this assumption. Code paths that need to
9temporarily lower the TPL (e.g. for entropy gathering) will restore it
10to TPL_CALLBACK.
11
12The entropy gathering code will be run during DRBG initialisation,
13which happens during the call to startup(). In the case of iPXE
14compiled as an EFI application this code will run within the scope of
15efi_snp_claim() and so will execute at TPL_CALLBACK as expected.
16
17In the case of iPXE compiled as an EFI driver the code will
18incorrectly run at TPL_APPLICATION since there is nothing within the
19EFI driver entry point that raises (and restores) the TPL. The net
20effect is that a build that includes the entropy-gathering code
21(e.g. a build with HTTPS enabled) will return from the driver entry
22point at TPL_CALLBACK, which causes a system lockup.
23
24Fix by raising and restoring the TPL within the EFI driver entry
25point.
26
27Debugged-by: Ignat Korchagin <ignat@cloudflare.com>
28Signed-off-by: Michael Brown <mcb30@ipxe.org>
29
30Origin: backport, https://github.com/ipxe/ipxe/commit/2ae5d4338661b65c63eb5cb1a96e5b803fe7d620
31Bug-Ubuntu: https://bugs.launchpad.net/bugs/1882671
32Last-Update: 2020-07-16
33
34---
35 src/interface/efi/efidrvprefix.c | 9 +++++++++
36 1 file changed, 9 insertions(+)
37
38--- a/src/interface/efi/efidrvprefix.c
39+++ b/src/interface/efi/efidrvprefix.c
40@@ -34,16 +34,25 @@ FILE_LICENCE ( GPL2_OR_LATER );
41 */
42 EFI_STATUS EFIAPI _efidrv_start ( EFI_HANDLE image_handle,
43 EFI_SYSTEM_TABLE *systab ) {
44+ EFI_BOOT_SERVICES *bs;
45+ EFI_TPL saved_tpl;
46 EFI_STATUS efirc;
47
48 /* Initialise EFI environment */
49 if ( ( efirc = efi_init ( image_handle, systab ) ) != 0 )
50 return efirc;
51
52+ /* Raise TPL */
53+ bs = efi_systab->BootServices;
54+ saved_tpl = bs->RaiseTPL ( TPL_CALLBACK );
55+
56 /* Initialise iPXE environment */
57 initialise();
58 startup();
59
60+ /* Restore TPL */
61+ bs->RestoreTPL ( saved_tpl );
62+
63 return 0;
64 }
65
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 @@
10001-rom-change-banner-timeout.diff.patch10001-rom-change-banner-timeout.diff.patch
2util-elf2efi-GNU_SOURCE.patch2util-elf2efi-GNU_SOURCE.patch
3enable-https.patch
3handle-dhcp-nack.patch4handle-dhcp-nack.patch
40005-strip-802.1Q-VLAN-0-priority-tags.patch50005-strip-802.1Q-VLAN-0-priority-tags.patch
6lp-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:
17# This is _a bit_ hacky, it assumes any rom we build are for qemu. Which17# This is _a bit_ hacky, it assumes any rom we build are for qemu. Which
18# is true at the moment, but maybe it changes at some point.18# is true at the moment, but maybe it changes at some point.
19src/bin/%.rom src/bin-x86_64-efi/%.efirom: export CONFIG=qemu19src/bin/%.rom src/bin-x86_64-efi/%.efirom: export CONFIG=qemu
20# qemu EFI roms are built without HTTPS, EFI does that on its own (and better)
21src/bin-x86_64-efi/%.efirom: export NOHTTPS=true
2220
23src/bin/% src/bin-x86_64-efi/%:21src/bin/% src/bin-x86_64-efi/%:
24 # clean and reconfigure if either NOHTTPS or CONFIG=qemu switches22 [ "$(CONFIG)" = "$(shell cat debian/current-config 2>/dev/null)" ] || $(MAKE) -C src clean
25 if [ "config:$(CONFIG)nohttps:$(NOHTTPS)" != "$(shell cat debian/last-used-config 2>/dev/null)" ]; then \23 @echo $(CONFIG) > debian/current-config
26 if [ "$(NOHTTPS)" = "true" ]; then \
27 cp -v debian/config/* src/config/local; \
28 else \
29 echo "#define DOWNLOAD_PROTO_HTTPS" >> src/config/local/general.h; \
30 fi; \
31 $(MAKE) -C src clean; \
32 echo -n "config:$(CONFIG)" > debian/last-used-config; \
33 echo -n "nohttps:$(NOHTTPS)" >> debian/last-used-config; \
34 fi;
35 dh_auto_build --sourcedirectory=src -- V=1 NO_WERROR=1 VERSION="$(VERSION)" $(subst src/,,$@)24 dh_auto_build --sourcedirectory=src -- V=1 NO_WERROR=1 VERSION="$(VERSION)" $(subst src/,,$@)
3625
37src/bin-efi/%.efirom: src/bin/%.rom src/bin-x86_64-efi/%.efirom26src/bin-efi/%.efirom: src/bin/%.rom src/bin-x86_64-efi/%.efirom
38 @[ -d $(dir $@) ] || mkdir $(dir $@)27 @[ -d $(dir $@) ] || mkdir $(dir $@)
39 src/util/catrom.pl $^ > $@28 src/util/catrom.pl $^ > $@
40 # efi roms are >256k since Bionic due to https + natural growth
41 # stay at 256k-512k after disabling https in efi via padding
42 src/util/padimg.pl --blksize=524288 --byte=0xff --verbose $@
4329
44src/bin-efi/%.iso: src/bin/%.lkrn src/bin-x86_64-efi/%.efi30src/bin-efi/%.iso: src/bin/%.lkrn src/bin-x86_64-efi/%.efi
45 @[ -d $(dir $@) ] || mkdir $(dir $@)31 @[ -d $(dir $@) ] || mkdir $(dir $@)
@@ -53,7 +39,7 @@ override_dh_install:
5339
54override_dh_auto_clean:40override_dh_auto_clean:
55 $(MAKE) -C src veryclean41 $(MAKE) -C src veryclean
56 rm -f debian/last-used-config42 rm -f debian/current-config
57 rm -fr src/bin*43 rm -fr src/bin*
58 rm -fr src/config/local/*44 rm -fr src/config/local/*
5945
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
52 done52 done
53 bucket=$((bucket<<1))53 bucket=$((bucket<<1))
54 # being smaller could be ok, but check -eq to warn/break on unexpected size loss as well54 # being smaller could be ok, but check -eq to warn/break on unexpected size loss as well
55 if [ "${size}" -eq "${roms[$rom]}" ]; then55 if [ "${bucket}" -eq "${roms[$rom]}" ]; then
56 echo "OK: ${rom} is exactly ${size} bytes as expected"56 echo "OK: ${rom} is ${size} bytes and thereby in bucket ${bucket} which matches ${roms[$rom]}"
57 else57 else
58 if [ "${bucket}" -eq "${roms[$rom]}" ]; then58 echo "ERROR: ${rom} is ${size} bytes and thereby in bucket ${bucket} which does not match ${roms[$rom]}"
59 echo "OK: ${rom} is ${size} bytes and thereby in bucket ${bucket} which matches ${roms[$rom]}"59 rc=1
60 else
61 echo "ERROR: ${rom} is ${size} bytes and thereby in bucket ${bucket} which does not match ${roms[$rom]}"
62 rc=1
63 fi
64 fi60 fi
65done61done
6662

Subscribers

People subscribed via source and target branches