diff mbox series

[22/32] bsd-user: Implement freebsd_exec_common, used in implementing execve/fexecve.

Message ID 20230827155746.84781-23-kariem.taha2.7@gmail.com
State New
Headers show
Series bsd-user: Implement freebsd process related system calls. | expand

Commit Message

Karim Taha Aug. 27, 2023, 3:57 p.m. UTC 
From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/freebsd/os-proc.c | 177 +++++++++++++++++++++++++++++++++++++
 1 file changed, 177 insertions(+)

Comments

Richard Henderson Aug. 29, 2023, 8:28 p.m. UTC | #1 
On 8/27/23 08:57, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/freebsd/os-proc.c | 177 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 177 insertions(+)

Acked-by: Richard Henderson <richard.henderson@linaro.org>

> +    if (do_fexec) {
> +        if (((int)path_or_fd > 0 &&
> +            is_target_elf_binary((int)path_or_fd)) == 1) {
> +            char execpath[PATH_MAX];
> +
> +            /*
> +             * The executable is an elf binary for the target
> +             * arch.  execve() it using the emulator if we can
> +             * determine the filename path from the fd.
> +             */
> +            if (get_filename_from_fd(getpid(), (int)path_or_fd, execpath,
> +                        sizeof(execpath)) != NULL) {
> +                memmove(qarg1 + 2, qarg1, (qargend-qarg1) * sizeof(*qarg1));
> +		qarg1[1] = qarg1[0];
> +		qarg1[0] = (char *)"-0";
> +		qarg1 += 2;
> +		qargend += 2;
> +                *qarg1 = execpath;
> +#ifndef DONT_INHERIT_INTERP_PREFIX
> +                memmove(qarg1 + 2, qarg1, (qargend-qarg1) * sizeof(*qarg1));
> +                *qarg1++ = (char *)"-L";
> +                *qarg1++ = (char *)interp_prefix;
> +#endif

I'm not especailly keen on the ifdef, but I'll let that go.

As for get_filename_from_fd, perhaps it would be cleaner to add a command-line parameter 
which would allow qemu to run from an open file descriptor?  Although perhaps that has 
CLOEXEC implications too...


r~
Warner Losh Aug. 29, 2023, 9:34 p.m. UTC | #2 
On Tue, Aug 29, 2023 at 2:28 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 8/27/23 08:57, Karim Taha wrote:
> > From: Stacey Son<sson@FreeBSD.org>
> >
> > Signed-off-by: Stacey Son<sson@FreeBSD.org>
> > Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> > ---
> >   bsd-user/freebsd/os-proc.c | 177 +++++++++++++++++++++++++++++++++++++
> >   1 file changed, 177 insertions(+)
>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
>
> > +    if (do_fexec) {
> > +        if (((int)path_or_fd > 0 &&
> > +            is_target_elf_binary((int)path_or_fd)) == 1) {
> > +            char execpath[PATH_MAX];
> > +
> > +            /*
> > +             * The executable is an elf binary for the target
> > +             * arch.  execve() it using the emulator if we can
> > +             * determine the filename path from the fd.
> > +             */
> > +            if (get_filename_from_fd(getpid(), (int)path_or_fd,
> execpath,
> > +                        sizeof(execpath)) != NULL) {
> > +                memmove(qarg1 + 2, qarg1, (qargend-qarg1) *
> sizeof(*qarg1));
> > +             qarg1[1] = qarg1[0];
> > +             qarg1[0] = (char *)"-0";
> > +             qarg1 += 2;
> > +             qargend += 2;
> > +                *qarg1 = execpath;
> > +#ifndef DONT_INHERIT_INTERP_PREFIX
> > +                memmove(qarg1 + 2, qarg1, (qargend-qarg1) *
> sizeof(*qarg1));
> > +                *qarg1++ = (char *)"-L";
> > +                *qarg1++ = (char *)interp_prefix;
> > +#endif
>
> I'm not especailly keen on the ifdef, but I'll let that go.
>
> As for get_filename_from_fd, perhaps it would be cleaner to add a
> command-line parameter
> which would allow qemu to run from an open file descriptor?  Although
> perhaps that has
> CLOEXEC implications too...
>

This is one area that's in transition in the bsd-user stuff, but we've not
yet finished
that transition. Doug Rabson has created something that caches a reference
to
the interpreter, and if we exec the same kind of binary, it will reuse that
reference.
In a jail that Doug's code runs, this allows the interpreter to be running
a binary
from outside the jail, while restricting the emulated binary's reach to the
jail.
This eliminates, in some cases, the need to inherit this prefix. However,
in other
cases, it still seems to be needed (like when I'm not in a chroot
environment and
wanting to pull the shared libraries from a different location). How to
resolve
these two cases is an on-going area of discussions. And all the work may not
yet be merged with the upstream tree.

Warner
diff mbox series

Patch

diff --git a/bsd-user/freebsd/os-proc.c b/bsd-user/freebsd/os-proc.c
index 5cd800e607..396f258a64 100644
--- a/bsd-user/freebsd/os-proc.c
+++ b/bsd-user/freebsd/os-proc.c
@@ -72,3 +72,180 @@  out:
     return ret;
 }
 
+/*
+ * execve/fexecve
+ */
+abi_long freebsd_exec_common(abi_ulong path_or_fd, abi_ulong guest_argp,
+        abi_ulong guest_envp, int do_fexec)
+{
+    char **argp, **envp, **qargp, **qarg1, **qarg0, **qargend;
+    int argc, envc;
+    abi_ulong gp;
+    abi_ulong addr;
+    char **q;
+    int total_size = 0;
+    void *p;
+    abi_long ret;
+
+    argc = 0;
+    for (gp = guest_argp; gp; gp += sizeof(abi_ulong)) {
+        if (get_user_ual(addr, gp)) {
+            return -TARGET_EFAULT;
+        }
+        if (!addr) {
+            break;
+        }
+        argc++;
+    }
+    envc = 0;
+    for (gp = guest_envp; gp; gp += sizeof(abi_ulong)) {
+        if (get_user_ual(addr, gp)) {
+            return -TARGET_EFAULT;
+        }
+        if (!addr) {
+            break;
+        }
+        envc++;
+    }
+
+    qarg0 = argp = g_new0(char *, argc + 9);
+    /* save the first agrument for the emulator */
+    *argp++ = (char *)getprogname();
+    qargp = argp;
+    *argp++ = (char *)getprogname();
+    qarg1 = argp;
+    envp = g_new0(char *, envc + 1);
+    for (gp = guest_argp, q = argp; gp; gp += sizeof(abi_ulong), q++) {
+        if (get_user_ual(addr, gp)) {
+            ret = -TARGET_EFAULT;
+            goto execve_end;
+        }
+        if (!addr) {
+            break;
+        }
+        *q = lock_user_string(addr);
+        if (*q == NULL) {
+            ret = -TARGET_EFAULT;
+            goto execve_end;
+        }
+        total_size += strlen(*q) + 1;
+    }
+    *q++ = NULL;
+    qargend = q;
+
+    for (gp = guest_envp, q = envp; gp; gp += sizeof(abi_ulong), q++) {
+        if (get_user_ual(addr, gp)) {
+            ret = -TARGET_EFAULT;
+            goto execve_end;
+        }
+        if (!addr) {
+            break;
+        }
+        *q = lock_user_string(addr);
+        if (*q == NULL) {
+            ret = -TARGET_EFAULT;
+            goto execve_end;
+        }
+        total_size += strlen(*q) + 1;
+    }
+    *q = NULL;
+
+    /*
+     * This case will not be caught by the host's execve() if its
+     * page size is bigger than the target's.
+     */
+    if (total_size > MAX_ARG_PAGES * TARGET_PAGE_SIZE) {
+        ret = -TARGET_E2BIG;
+        goto execve_end;
+    }
+
+    if (do_fexec) {
+        if (((int)path_or_fd > 0 &&
+            is_target_elf_binary((int)path_or_fd)) == 1) {
+            char execpath[PATH_MAX];
+
+            /*
+             * The executable is an elf binary for the target
+             * arch.  execve() it using the emulator if we can
+             * determine the filename path from the fd.
+             */
+            if (get_filename_from_fd(getpid(), (int)path_or_fd, execpath,
+                        sizeof(execpath)) != NULL) {
+                memmove(qarg1 + 2, qarg1, (qargend-qarg1) * sizeof(*qarg1));
+		qarg1[1] = qarg1[0];
+		qarg1[0] = (char *)"-0";
+		qarg1 += 2;
+		qargend += 2;
+                *qarg1 = execpath;
+#ifndef DONT_INHERIT_INTERP_PREFIX
+                memmove(qarg1 + 2, qarg1, (qargend-qarg1) * sizeof(*qarg1));
+                *qarg1++ = (char *)"-L";
+                *qarg1++ = (char *)interp_prefix;
+#endif
+                ret = get_errno(execve(qemu_proc_pathname, qargp, envp));
+            } else {
+                /* Getting the filename path failed. */
+                ret = -TARGET_EBADF;
+                goto execve_end;
+            }
+        } else {
+            ret = get_errno(fexecve((int)path_or_fd, argp, envp));
+        }
+    } else {
+        int fd;
+
+        p = lock_user_string(path_or_fd);
+        if (p == NULL) {
+            ret = -TARGET_EFAULT;
+            goto execve_end;
+        }
+
+        /*
+         * Check the header and see if it a target elf binary.  If so
+         * then execute using qemu user mode emulator.
+         */
+        fd = open(p, O_RDONLY | O_CLOEXEC);
+        if (fd > 0 && is_target_elf_binary(fd) == 1) {
+            close(fd);
+            /* execve() as a target binary using emulator. */
+            memmove(qarg1 + 2, qarg1, (qargend-qarg1) * sizeof(*qarg1));
+            qarg1[1] = qarg1[0];
+            qarg1[0] = (char *)"-0";
+            qarg1 += 2;
+	    qargend += 2;
+            *qarg1 = (char *)p;
+#ifndef DONT_INHERIT_INTERP_PREFIX
+            memmove(qarg1 + 2, qarg1, (qargend-qarg1) * sizeof(*qarg1));
+            *qarg1++ = (char *)"-L";
+            *qarg1++ = (char *)interp_prefix;
+#endif
+            ret = get_errno(execve(qemu_proc_pathname, qargp, envp));
+        } else {
+            close(fd);
+            /* Execve() as a host native binary. */
+            ret = get_errno(execve(p, argp, envp));
+        }
+        unlock_user(p, path_or_fd, 0);
+    }
+
+execve_end:
+    for (gp = guest_argp, q = argp; *q; gp += sizeof(abi_ulong), q++) {
+        if (get_user_ual(addr, gp) || !addr) {
+            break;
+        }
+        unlock_user(*q, addr, 0);
+    }
+
+    for (gp = guest_envp, q = envp; *q; gp += sizeof(abi_ulong), q++) {
+        if (get_user_ual(addr, gp) || !addr) {
+            break;
+        }
+        unlock_user(*q, addr, 0);
+    }
+
+    g_free(qarg0);
+    g_free(envp);
+
+    return ret;
+}
+