* [PATCH v2] linux-user: Fixes for zero_bss
@ 2023-09-09 18:45 Richard Henderson
2023-09-21 14:52 ` Michael Tokarev
2023-09-21 16:51 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 3+ messages in thread
From: Richard Henderson @ 2023-09-09 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: thuth, philmd, qemu-stable
The previous change, 2d385be6152, assumed !PAGE_VALID meant that
the page would be unmapped by the elf image. However, since we
reserved the entire image space via mmap, PAGE_VALID will always
be set. Instead, assume PROT_NONE for the same condition.
Furthermore, assume bss is only ever present for writable segments,
and that there is no page overlap between PT_LOAD segments.
Instead of an assert, return false to indicate failure.
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1854
Fixes: 2d385be6152 ("linux-user: Do not adjust zero_bss for host page size")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v2: Pass errp to zero_bss, so we can give a reasonable error message.
---
linux-user/elfload.c | 53 +++++++++++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 13 deletions(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index a5b28fa3e7..5271b749b0 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2304,31 +2304,58 @@ static abi_ulong setup_arg_pages(struct linux_binprm *bprm,
* Map and zero the bss. We need to explicitly zero any fractional pages
* after the data section (i.e. bss). Return false on mapping failure.
*/
-static bool zero_bss(abi_ulong start_bss, abi_ulong end_bss, int prot)
+static bool zero_bss(abi_ulong start_bss, abi_ulong end_bss,
+ int prot, Error **errp)
{
abi_ulong align_bss;
+ /* We only expect writable bss; the code segment shouldn't need this. */
+ if (!(prot & PROT_WRITE)) {
+ error_setg(errp, "PT_LOAD with non-writable bss");
+ return false;
+ }
+
align_bss = TARGET_PAGE_ALIGN(start_bss);
end_bss = TARGET_PAGE_ALIGN(end_bss);
if (start_bss < align_bss) {
int flags = page_get_flags(start_bss);
- if (!(flags & PAGE_VALID)) {
- /* Map the start of the bss. */
+ if (!(flags & PAGE_BITS)) {
+ /*
+ * The whole address space of the executable was reserved
+ * at the start, therefore all pages will be VALID.
+ * But assuming there are no PROT_NONE PT_LOAD segments,
+ * a PROT_NONE page means no data all bss, and we can
+ * simply extend the new anon mapping back to the start
+ * of the page of bss.
+ */
align_bss -= TARGET_PAGE_SIZE;
- } else if (flags & PAGE_WRITE) {
- /* The page is already mapped writable. */
- memset(g2h_untagged(start_bss), 0, align_bss - start_bss);
} else {
- /* Read-only zeros? */
- g_assert_not_reached();
+ /*
+ * The start of the bss shares a page with something.
+ * The only thing that we expect is the data section,
+ * which would already be marked writable.
+ * Overlapping the RX code segment seems malformed.
+ */
+ if (!(flags & PAGE_WRITE)) {
+ error_setg(errp, "PT_LOAD with bss overlapping "
+ "non-writable page");
+ return false;
+ }
+
+ /* The page is already mapped and writable. */
+ memset(g2h_untagged(start_bss), 0, align_bss - start_bss);
}
}
- return align_bss >= end_bss ||
- target_mmap(align_bss, end_bss - align_bss, prot,
- MAP_FIXED | MAP_PRIVATE | MAP_ANON, -1, 0) != -1;
+ if (align_bss < end_bss &&
+ target_mmap(align_bss, end_bss - align_bss, prot,
+ MAP_FIXED | MAP_PRIVATE | MAP_ANON, -1, 0) == -1) {
+ error_setg_errno(errp, errno, "Error mapping bss");
+ return false;
+ }
+ return true;
}
#if defined(TARGET_ARM)
@@ -3352,8 +3379,8 @@ static void load_elf_image(const char *image_name, int image_fd,
/* If the load segment requests extra zeros (e.g. bss), map it. */
if (vaddr_ef < vaddr_em &&
- !zero_bss(vaddr_ef, vaddr_em, elf_prot)) {
- goto exit_mmap;
+ !zero_bss(vaddr_ef, vaddr_em, elf_prot, &err)) {
+ goto exit_errmsg;
}
/* Find the full program boundaries. */
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] linux-user: Fixes for zero_bss
2023-09-09 18:45 [PATCH v2] linux-user: Fixes for zero_bss Richard Henderson
@ 2023-09-21 14:52 ` Michael Tokarev
2023-09-21 16:51 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 3+ messages in thread
From: Michael Tokarev @ 2023-09-21 14:52 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: thuth, philmd, qemu-stable
09.09.2023 21:45, Richard Henderson wrote:
> The previous change, 2d385be6152, assumed !PAGE_VALID meant that
> the page would be unmapped by the elf image. However, since we
> reserved the entire image space via mmap, PAGE_VALID will always
> be set. Instead, assume PROT_NONE for the same condition.
>
> Furthermore, assume bss is only ever present for writable segments,
> and that there is no page overlap between PT_LOAD segments.
> Instead of an assert, return false to indicate failure.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1854
> Fixes: 2d385be6152 ("linux-user: Do not adjust zero_bss for host page size")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Pass errp to zero_bss, so we can give a reasonable error message.
> ---
> linux-user/elfload.c | 53 +++++++++++++++++++++++++++++++++-----------
> 1 file changed, 40 insertions(+), 13 deletions(-)
Ping? Has this been forgotten?
I picked this one up for debian 8.1 package, at least I don't see
regressions with it applied (together with stuff staging for 8.1.1).
Thanks,
/mjt
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] linux-user: Fixes for zero_bss
2023-09-09 18:45 [PATCH v2] linux-user: Fixes for zero_bss Richard Henderson
2023-09-21 14:52 ` Michael Tokarev
@ 2023-09-21 16:51 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-21 16:51 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: thuth, qemu-stable, Laurent Vivier
On 9/9/23 20:45, Richard Henderson wrote:
> The previous change, 2d385be6152, assumed !PAGE_VALID meant that
> the page would be unmapped by the elf image. However, since we
> reserved the entire image space via mmap, PAGE_VALID will always
> be set. Instead, assume PROT_NONE for the same condition.
>
> Furthermore, assume bss is only ever present for writable segments,
> and that there is no page overlap between PT_LOAD segments.
> Instead of an assert, return false to indicate failure.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1854
> Fixes: 2d385be6152 ("linux-user: Do not adjust zero_bss for host page size")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Pass errp to zero_bss, so we can give a reasonable error message.
> ---
> linux-user/elfload.c | 53 +++++++++++++++++++++++++++++++++-----------
> 1 file changed, 40 insertions(+), 13 deletions(-)
To the best of my knowledge,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-21 16:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-09 18:45 [PATCH v2] linux-user: Fixes for zero_bss Richard Henderson
2023-09-21 14:52 ` Michael Tokarev
2023-09-21 16:51 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).