From: Warner Losh <imp@bsdimp.com>
To: qemu-devel@nongnu.org
Cc: Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>,
Jessica Clarke <jrtc27@jrtc27.com>
Subject: [PATCH 11/17] bsd-user: Replace set_brk and padzero with zerobss from linux-user
Date: Fri, 2 Aug 2024 17:56:11 -0600 [thread overview]
Message-ID: <20240802235617.7971-12-imp@bsdimp.com> (raw)
In-Reply-To: <20240802235617.7971-1-imp@bsdimp.com>
The zero_bss interface from linux-user is much better at doing this. Use
it in preference to set_brk (badly named) and padzero. These both have
issues with the new variable page size code, so it's best to just retire
them and reuse the code from linux-user. Also start to use the error
reporting code that linux-user uses to give better error messages on
failure.
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
bsd-user/elfload.c | 110 +++++++++++++++++++++++----------------------
1 file changed, 57 insertions(+), 53 deletions(-)
diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index dba03f17465..0a2f2379c93 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -22,6 +22,7 @@
#include "qemu.h"
#include "disas/disas.h"
#include "qemu/path.h"
+#include "qapi/error.h"
static abi_ulong target_auxents; /* Where the AUX entries are in target */
static size_t target_auxents_sz; /* Size of AUX entries including AT_NULL */
@@ -210,62 +211,63 @@ static void setup_arg_pages(struct bsd_binprm *bprm, struct image_info *info,
}
}
-static void set_brk(abi_ulong start, abi_ulong end)
+/**
+ * zero_bss:
+ *
+ * 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, Error **errp)
{
- /* page-align the start and end addresses... */
- start = HOST_PAGE_ALIGN(start);
- end = HOST_PAGE_ALIGN(end);
- if (end <= start) {
- return;
- }
- if (target_mmap(start, end - start, PROT_READ | PROT_WRITE | PROT_EXEC,
- MAP_FIXED | MAP_PRIVATE | MAP_ANON, -1, 0) == -1) {
- perror("cannot mmap brk");
- exit(-1);
+ 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);
-/*
- * We need to explicitly zero any fractional pages after the data
- * section (i.e. bss). This would contain the junk from the file that
- * should not be in memory.
- */
-static void padzero(abi_ulong elf_bss, abi_ulong last_bss)
-{
- abi_ulong nbyte;
+ if (start_bss < align_bss) {
+ int flags = page_get_flags(start_bss);
- if (elf_bss >= last_bss) {
- return;
- }
+ if (!(flags & PAGE_RWX)) {
+ /*
+ * 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 {
+ /*
+ * 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;
+ }
- /*
- * XXX: this is really a hack : if the real host page size is
- * smaller than the target page size, some pages after the end
- * of the file may not be mapped. A better fix would be to
- * patch target_mmap(), but it is more complicated as the file
- * size must be known.
- */
- if (qemu_real_host_page_size() < qemu_host_page_size) {
- abi_ulong end_addr, end_addr1;
- end_addr1 = REAL_HOST_PAGE_ALIGN(elf_bss);
- end_addr = HOST_PAGE_ALIGN(elf_bss);
- if (end_addr1 < end_addr) {
- mmap((void *)g2h_untagged(end_addr1), end_addr - end_addr1,
- PROT_READ | PROT_WRITE | PROT_EXEC,
- MAP_FIXED | MAP_PRIVATE | MAP_ANON, -1, 0);
+ /* The page is already mapped and writable. */
+ memset(g2h_untagged(start_bss), 0, align_bss - start_bss);
}
}
-
- nbyte = elf_bss & (qemu_host_page_size - 1);
- if (nbyte) {
- nbyte = qemu_host_page_size - nbyte;
- do {
- /* FIXME - what to do if put_user() fails? */
- put_user_u8(0, elf_bss);
- elf_bss++;
- } while (--nbyte);
+ 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;
}
static abi_ulong load_elf_interp(const char *elf_interpreter,
@@ -535,6 +537,7 @@ load_elf_sections(const char *image_name, const struct elfhdr *hdr,
abi_ulong baddr;
int i;
bool first;
+ Error *err = NULL;
/*
* Now we do a little grungy work by mmaping the ELF image into
@@ -579,12 +582,10 @@ load_elf_sections(const char *image_name, const struct elfhdr *hdr,
start_bss = rbase + elf_ppnt->p_vaddr + elf_ppnt->p_filesz;
end_bss = rbase + elf_ppnt->p_vaddr + elf_ppnt->p_memsz;
- /*
- * Calling set_brk effectively mmaps the pages that we need for the
- * bss and break sections.
- */
- set_brk(start_bss, end_bss);
- padzero(start_bss, end_bss);
+ if (start_bss < end_bss &&
+ !zero_bss(start_bss, end_bss, elf_prot, &err)) {
+ goto exit_errmsg;
+ }
}
if (first) {
@@ -597,6 +598,9 @@ load_elf_sections(const char *image_name, const struct elfhdr *hdr,
*baddrp = baddr;
}
return 0;
+exit_errmsg:
+ error_reportf_err(err, "%s: ", image_name);
+ exit(-1);
}
int load_elf_binary(struct bsd_binprm *bprm, struct image_info *info)
--
2.45.1
next prev parent reply other threads:[~2024-08-02 23:59 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-02 23:56 [PATCH 00/17] For 9.2: A bunch of cleanups and work towards variable pagesize support Warner Losh
2024-08-02 23:56 ` [PATCH 01/17] bsd-user: Delete TaskState next member Warner Losh
2024-08-04 7:07 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 02/17] bsd-user: Make init_task_state global Warner Losh
2024-08-04 7:08 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 03/17] bsd-user: Make cpu_model and cpu_type file scope Warner Losh
2024-08-04 7:22 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 04/17] bsd-user: Implement cpu_copy() Warner Losh
2024-08-04 7:24 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 05/17] bsd-user: Eliminate unused regs arg in load_elf_binary Warner Losh
2024-08-04 7:26 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 06/17] bsd-user: Remove load_flt_binary prototype Warner Losh
2024-08-04 7:26 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 07/17] bsd-user: Remove deprecated -p argument Warner Losh
2024-08-04 7:26 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 08/17] bsd-user: Eliminate unused qemu_uname_release Warner Losh
2024-08-04 7:27 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 09/17] bsd-user: target_msync unused, remove it Warner Losh
2024-08-04 7:28 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 10/17] bsd-user: Pass image name down the stack Warner Losh
2024-08-04 7:29 ` Richard Henderson
2024-08-02 23:56 ` Warner Losh [this message]
2024-08-04 11:38 ` [PATCH 11/17] bsd-user: Replace set_brk and padzero with zerobss from linux-user Richard Henderson
2024-08-02 23:56 ` [PATCH 12/17] bsd-user: Use guest_range_valid_untagged to validate range Warner Losh
2024-08-04 21:30 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 13/17] bsd-user: target_mprotect: rename prot to target_prot Warner Losh
2024-08-04 21:31 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 14/17] bsd-user: target_mmap*: change " Warner Losh
2024-08-04 21:32 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 15/17] bsd-user: target_mprotect: use helper host_page_size local Warner Losh
2024-08-04 21:33 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 16/17] bsd-user: Define validate_prot_to_pageflags and use in mprotect Warner Losh
2024-08-04 21:44 ` Richard Henderson
2024-08-02 23:56 ` [PATCH 17/17] bsd-user: copy linux-user target_mprotect impl Warner Losh
2024-08-04 21:47 ` Richard Henderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240802235617.7971-12-imp@bsdimp.com \
--to=imp@bsdimp.com \
--cc=jrtc27@jrtc27.com \
--cc=kevans@freebsd.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.