Merge ~paelzer/ubuntu/+source/qemu:lp-1749393-emu-sbrk-space-FOCAL into ubuntu/+source/qemu:ubuntu/focal-devel

Proposed by Christian Ehrhardt 
Status: Merged
Merge reported by: Christian Ehrhardt 
Merged at revision: 7bd0792adab4786f8b26ca6319fa83de71ccf282
Proposed branch: ~paelzer/ubuntu/+source/qemu:lp-1749393-emu-sbrk-space-FOCAL
Merge into: ubuntu/+source/qemu:ubuntu/focal-devel
Diff against target: 293 lines (+265/-0)
4 files modified
debian/changelog (+9/-0)
debian/patches/series (+2/-0)
debian/patches/ubuntu/lp-1749393-linux-user-Reserve-space-for-brk.patch (+158/-0)
debian/patches/ubuntu/lp-1929926-target-s390x-Fix-translation-exception-on-illegal-in.patch (+96/-0)
Reviewer Review Type Date Requested Status
Paride Legovini (community) Approve
Canonical Server packageset reviewers Pending
git-ubuntu developers Pending
Review via email: mp+401771@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 :

FYI: This is based on the current latest focal-proposed wich isn't imported yet.
The change/review is only for the last changelog stanza.

In a few hours this will resolve and the diff should also be correct here.

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

Rebased and refreshed, working the same still.

New build of qemu_4.2-3ubuntu6.19~focalppa3_source.changes started in the PPA.
The tests all completed before and in a VM that I used to debug bug 1928075.

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

PPA https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4535
has a new build that also includes bug 1929926 which we wanted to piggy-back on another bug.

Revision history for this message
Paride Legovini (paride) wrote :

LGTM, both the upstream cherry-pick and the upstream backport look fully correct, with nice and complete DEP3 headers. The other debian/* changes are all good.

The PPA builds look fine. There are no autopkgtests but you reported the fixed package passes the KVM regression tests, which are even more meaningful.

Approve, but see my comment here:

https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1929926

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

Uploaded to -unapproved

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 20c9285..34da269 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,12 @@
6+qemu (1:4.2-3ubuntu6.19) focal; urgency=medium
7+
8+ * d/p/u/lp-1749393-linux-user-Reserve-space-for-brk.patch: fix static
9+ use cases needing a lot of brk space (LP: #1749393)
10+ * d/p/u/lp-1929926-target-s390x-Fix-translation-exception-on-illegal-in.patch:
11+ fix uretprobe in s390x TCG (LP: #1929926)
12+
13+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Mon, 26 Apr 2021 11:11:19 +0200
14+
15 qemu (1:4.2-3ubuntu6.18) focal; urgency=medium
16
17 * enhance loading of old modules post upgrade (LP: #1913421)
18diff --git a/debian/patches/series b/debian/patches/series
19index 745af86..43a47e7 100644
20--- a/debian/patches/series
21+++ b/debian/patches/series
22@@ -307,3 +307,5 @@ CVE-2021-3607.patch
23 CVE-2021-3608.patch
24 CVE-2021-20221.patch
25 CVE-2021-20257.patch
26+ubuntu/lp-1749393-linux-user-Reserve-space-for-brk.patch
27+ubuntu/lp-1929926-target-s390x-Fix-translation-exception-on-illegal-in.patch
28diff --git a/debian/patches/ubuntu/lp-1749393-linux-user-Reserve-space-for-brk.patch b/debian/patches/ubuntu/lp-1749393-linux-user-Reserve-space-for-brk.patch
29new file mode 100644
30index 0000000..71a89aa
31--- /dev/null
32+++ b/debian/patches/ubuntu/lp-1749393-linux-user-Reserve-space-for-brk.patch
33@@ -0,0 +1,158 @@
34+From 6fd5944980f4ccee728ce34bdaffc117db50b34d Mon Sep 17 00:00:00 2001
35+From: Richard Henderson <richard.henderson@linaro.org>
36+Date: Fri, 17 Jan 2020 13:02:45 -1000
37+Subject: [PATCH] linux-user: Reserve space for brk
38+MIME-Version: 1.0
39+Content-Type: text/plain; charset=UTF-8
40+Content-Transfer-Encoding: 8bit
41+
42+With bad luck, we can wind up with no space at all for brk,
43+which will generally cause the guest malloc to fail.
44+
45+This bad luck is easier to come by with ET_DYN (PIE) binaries,
46+where either the stack or the interpreter (ld.so) gets placed
47+immediately after the main executable.
48+
49+But there's nothing preventing this same thing from happening
50+with ET_EXEC (normal) binaries, during probe_guest_base().
51+
52+In both cases, reserve some extra space via mmap and release
53+it back to the system after loading the interpreter and
54+allocating the stack.
55+
56+The choice of 16MB is somewhat arbitrary. It's enough for libc
57+to get going, but without being so large that 32-bit guests or
58+32-bit hosts are in danger of running out of virtual address space.
59+It is expected that libc will be able to fall back to mmap arenas
60+after the limited brk space is exhausted.
61+
62+Launchpad: https://bugs.launchpad.net/qemu/+bug/1749393
63+Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
64+Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
65+Tested-by: Alex Bennée <alex.bennee@linaro.org>
66+Message-Id: <20200117230245.5040-1-richard.henderson@linaro.org>
67+Signed-off-by: Laurent Vivier <laurent@vivier.eu>
68+
69+Origin: upstream, https://git.qemu.org/?p=qemu.git;a=commit;h=6fd5944980f4ccee728ce34bdaffc117db50b34d
70+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1749393
71+Last-Update: 2021-04-26
72+
73+---
74+ linux-user/elfload.c | 73 +++++++++++++++++++++++++++++++++-----------
75+ linux-user/qemu.h | 1 +
76+ 2 files changed, 57 insertions(+), 17 deletions(-)
77+
78+diff --git a/linux-user/elfload.c b/linux-user/elfload.c
79+index 511e450078..f3080a1635 100644
80+--- a/linux-user/elfload.c
81++++ b/linux-user/elfload.c
82+@@ -10,6 +10,7 @@
83+ #include "qemu/path.h"
84+ #include "qemu/queue.h"
85+ #include "qemu/guest-random.h"
86++#include "qemu/units.h"
87+
88+ #ifdef _ARCH_PPC64
89+ #undef ARCH_DLINFO
90+@@ -2364,24 +2365,51 @@ static void load_elf_image(const char *image_name, int image_fd,
91+ }
92+ }
93+
94+- load_addr = loaddr;
95+- if (ehdr->e_type == ET_DYN) {
96+- /* The image indicates that it can be loaded anywhere. Find a
97+- location that can hold the memory space required. If the
98+- image is pre-linked, LOADDR will be non-zero. Since we do
99+- not supply MAP_FIXED here we'll use that address if and
100+- only if it remains available. */
101+- load_addr = target_mmap(loaddr, hiaddr - loaddr, PROT_NONE,
102+- MAP_PRIVATE | MAP_ANON | MAP_NORESERVE,
103+- -1, 0);
104+- if (load_addr == -1) {
105+- goto exit_perror;
106++ if (pinterp_name != NULL) {
107++ /*
108++ * This is the main executable.
109++ *
110++ * Reserve extra space for brk.
111++ * We hold on to this space while placing the interpreter
112++ * and the stack, lest they be placed immediately after
113++ * the data segment and block allocation from the brk.
114++ *
115++ * 16MB is chosen as "large enough" without being so large
116++ * as to allow the result to not fit with a 32-bit guest on
117++ * a 32-bit host.
118++ */
119++ info->reserve_brk = 16 * MiB;
120++ hiaddr += info->reserve_brk;
121++
122++ if (ehdr->e_type == ET_EXEC) {
123++ /*
124++ * Make sure that the low address does not conflict with
125++ * MMAP_MIN_ADDR or the QEMU application itself.
126++ */
127++ probe_guest_base(image_name, loaddr, hiaddr);
128+ }
129+- } else if (pinterp_name != NULL) {
130+- /* This is the main executable. Make sure that the low
131+- address does not conflict with MMAP_MIN_ADDR or the
132+- QEMU application itself. */
133+- probe_guest_base(image_name, loaddr, hiaddr);
134++ }
135++
136++ /*
137++ * Reserve address space for all of this.
138++ *
139++ * In the case of ET_EXEC, we supply MAP_FIXED so that we get
140++ * exactly the address range that is required.
141++ *
142++ * Otherwise this is ET_DYN, and we are searching for a location
143++ * that can hold the memory space required. If the image is
144++ * pre-linked, LOADDR will be non-zero, and the kernel should
145++ * honor that address if it happens to be free.
146++ *
147++ * In both cases, we will overwrite pages in this range with mappings
148++ * from the executable.
149++ */
150++ load_addr = target_mmap(loaddr, hiaddr - loaddr, PROT_NONE,
151++ MAP_PRIVATE | MAP_ANON | MAP_NORESERVE |
152++ (ehdr->e_type == ET_EXEC ? MAP_FIXED : 0),
153++ -1, 0);
154++ if (load_addr == -1) {
155++ goto exit_perror;
156+ }
157+ load_bias = load_addr - loaddr;
158+
159+@@ -2860,6 +2888,17 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
160+ bprm->core_dump = &elf_core_dump;
161+ #endif
162+
163++ /*
164++ * If we reserved extra space for brk, release it now.
165++ * The implementation of do_brk in syscalls.c expects to be able
166++ * to mmap pages in this space.
167++ */
168++ if (info->reserve_brk) {
169++ abi_ulong start_brk = HOST_PAGE_ALIGN(info->brk);
170++ abi_ulong end_brk = HOST_PAGE_ALIGN(info->brk + info->reserve_brk);
171++ target_munmap(start_brk, end_brk - start_brk);
172++ }
173++
174+ return 0;
175+ }
176+
177+diff --git a/linux-user/qemu.h b/linux-user/qemu.h
178+index f6f5fe5fbb..560a68090e 100644
179+--- a/linux-user/qemu.h
180++++ b/linux-user/qemu.h
181+@@ -35,6 +35,7 @@ struct image_info {
182+ abi_ulong end_data;
183+ abi_ulong start_brk;
184+ abi_ulong brk;
185++ abi_ulong reserve_brk;
186+ abi_ulong start_mmap;
187+ abi_ulong start_stack;
188+ abi_ulong stack_limit;
189+--
190+2.31.1
191+
192diff --git a/debian/patches/ubuntu/lp-1929926-target-s390x-Fix-translation-exception-on-illegal-in.patch b/debian/patches/ubuntu/lp-1929926-target-s390x-Fix-translation-exception-on-illegal-in.patch
193new file mode 100644
194index 0000000..a151900
195--- /dev/null
196+++ b/debian/patches/ubuntu/lp-1929926-target-s390x-Fix-translation-exception-on-illegal-in.patch
197@@ -0,0 +1,96 @@
198+From 86131c71b13257e095d8c4f4453d52cbc6553c07 Mon Sep 17 00:00:00 2001
199+From: Ilya Leoshkevich <iii@linux.ibm.com>
200+Date: Fri, 16 Apr 2021 17:49:36 +0200
201+Subject: [PATCH] target/s390x: Fix translation exception on illegal
202+ instruction
203+
204+Hitting an uretprobe in a s390x TCG guest causes a SIGSEGV. What
205+happens is:
206+
207+* uretprobe maps a userspace page containing an invalid instruction.
208+* uretprobe replaces the target function's return address with the
209+ address of that page.
210+* When tb_gen_code() is called on that page, tb->size ends up being 0
211+ (because the page starts with the invalid instruction), which causes
212+ virt_page2 to point to the previous page.
213+* The previous page is not mapped, so this causes a spurious
214+ translation exception.
215+
216+tb->size must never be 0: even if there is an illegal instruction, the
217+instruction bytes that have been looked at must count towards tb->size.
218+So adjust s390x's translate_one() to act this way for both illegal
219+instructions and instructions that are known to generate exceptions.
220+
221+Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
222+Reviewed-by: David Hildenbrand <david@redhat.com>
223+Message-Id: <20210416154939.32404-2-iii@linux.ibm.com>
224+Signed-off-by: Cornelia Huck <cohuck@redhat.com>
225+
226+Origin: backport, https://git.qemu.org/?p=qemu.git;a=commit;h=86131c71b13257e095d8c4f4453d52cbc6553c07
227+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1929926
228+Last-Update: 2021-10-12
229+
230+---
231+ target/s390x/translate.c | 16 +++++++++++-----
232+ 1 file changed, 11 insertions(+), 5 deletions(-)
233+
234+--- a/target/s390x/translate.c
235++++ b/target/s390x/translate.c
236+@@ -6317,7 +6317,8 @@ static DisasJumpType translate_one(CPUS3
237+ qemu_log_mask(LOG_UNIMP, "unimplemented opcode 0x%02x%02x\n",
238+ f.op, f.op2);
239+ gen_illegal_opcode(s);
240+- return DISAS_NORETURN;
241++ ret = DISAS_NORETURN;
242++ goto out;
243+ }
244+
245+ #ifndef CONFIG_USER_ONLY
246+@@ -6333,7 +6334,8 @@ static DisasJumpType translate_one(CPUS3
247+ /* privileged instruction */
248+ if ((s->base.tb->flags & FLAG_MASK_PSTATE) && (insn->flags & IF_PRIV)) {
249+ gen_program_exception(s, PGM_PRIVILEGED);
250+- return DISAS_NORETURN;
251++ ret = DISAS_NORETURN;
252++ goto out;
253+ }
254+
255+ /* if AFP is not enabled, instructions and registers are forbidden */
256+@@ -6360,7 +6362,8 @@ static DisasJumpType translate_one(CPUS3
257+ }
258+ if (dxc) {
259+ gen_data_exception(dxc);
260+- return DISAS_NORETURN;
261++ ret = DISAS_NORETURN;
262++ goto out;
263+ }
264+ }
265+
266+@@ -6368,7 +6371,8 @@ static DisasJumpType translate_one(CPUS3
267+ if (insn->flags & IF_VEC) {
268+ if (!((s->base.tb->flags & FLAG_MASK_VECTOR))) {
269+ gen_data_exception(0xfe);
270+- return DISAS_NORETURN;
271++ ret = DISAS_NORETURN;
272++ goto out;
273+ }
274+ }
275+ }
276+@@ -6381,7 +6385,8 @@ static DisasJumpType translate_one(CPUS3
277+ (insn->spec & SPEC_r1_f128 && !is_fp_pair(get_field(&f, r1))) ||
278+ (insn->spec & SPEC_r2_f128 && !is_fp_pair(get_field(&f, r2)))) {
279+ gen_program_exception(s, PGM_SPECIFICATION);
280+- return DISAS_NORETURN;
281++ ret = DISAS_NORETURN;
282++ goto out;
283+ }
284+ }
285+
286+@@ -6440,6 +6445,7 @@ static DisasJumpType translate_one(CPUS3
287+ }
288+ #endif
289+
290++out:
291+ /* Advance to the next instruction. */
292+ s->base.pc_next = s->pc_tmp;
293+ return ret;

Subscribers

People subscribed via source and target branches