Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys Tidy up the vnode locking around execve() on ELF images ...



details:   https://anonhg.NetBSD.org/src/rev/ff76e4e9b6e1
branches:  trunk
changeset: 467068:ff76e4e9b6e1
user:      ad <ad%NetBSD.org@localhost>
date:      Sun Jan 12 18:30:58 2020 +0000

description:
Tidy up the vnode locking around execve() on ELF images to acquire and
release the locks fewer times.  Proposed on tech-kern a very long time go.

diffstat:

 sys/compat/linux/arch/amd64/linux_exec_machdep.c |   6 +-
 sys/compat/linux/common/linux_exec_elf32.c       |  43 +++++++-----
 sys/kern/exec_elf.c                              |  78 +++++++++++++++--------
 sys/kern/exec_subr.c                             |  14 ++-
 sys/kern/kern_exec.c                             |  18 +++--
 sys/sys/exec.h                                   |   6 +-
 6 files changed, 99 insertions(+), 66 deletions(-)

diffs (truncated from 505 to 300 lines):

diff -r c5caba7578f8 -r ff76e4e9b6e1 sys/compat/linux/arch/amd64/linux_exec_machdep.c
--- a/sys/compat/linux/arch/amd64/linux_exec_machdep.c  Sun Jan 12 17:49:17 2020 +0000
+++ b/sys/compat/linux/arch/amd64/linux_exec_machdep.c  Sun Jan 12 18:30:58 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: linux_exec_machdep.c,v 1.22 2014/02/23 12:01:51 njoly Exp $ */
+/*     $NetBSD: linux_exec_machdep.c,v 1.23 2020/01/12 18:30:58 ad Exp $ */
 
 /*-
  * Copyright (c) 2005 Emmanuel Dreyfus, all rights reserved
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: linux_exec_machdep.c,v 1.22 2014/02/23 12:01:51 njoly Exp $");
+__KERNEL_RCSID(0, "$NetBSD: linux_exec_machdep.c,v 1.23 2020/01/12 18:30:58 ad Exp $");
 
 #define ELFSIZE 64
 
@@ -156,7 +156,7 @@
        if (ap == NULL) {
                phsize = eh->e_phnum * sizeof(Elf_Phdr);
                ph = (Elf_Phdr *)kmem_alloc(phsize, KM_SLEEP);
-               error = exec_read_from(l, pack->ep_vp, eh->e_phoff, ph, phsize);
+               error = exec_read(l, pack->ep_vp, eh->e_phoff, ph, phsize, 0);
                if (error != 0) {
                        for (i = 0; i < eh->e_phnum; i++) {
                                if (ph[i].p_type == PT_PHDR) {
diff -r c5caba7578f8 -r ff76e4e9b6e1 sys/compat/linux/common/linux_exec_elf32.c
--- a/sys/compat/linux/common/linux_exec_elf32.c        Sun Jan 12 17:49:17 2020 +0000
+++ b/sys/compat/linux/common/linux_exec_elf32.c        Sun Jan 12 18:30:58 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: linux_exec_elf32.c,v 1.99 2019/03/01 11:06:56 pgoyette Exp $   */
+/*     $NetBSD: linux_exec_elf32.c,v 1.100 2020/01/12 18:30:58 ad Exp $        */
 
 /*-
  * Copyright (c) 1995, 1998, 2000, 2001 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: linux_exec_elf32.c,v 1.99 2019/03/01 11:06:56 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: linux_exec_elf32.c,v 1.100 2020/01/12 18:30:58 ad Exp $");
 
 #ifndef ELFSIZE
 /* XXX should die */
@@ -107,7 +107,8 @@
        /* Load the section header table. */
        shsize = eh->e_shnum * sizeof(Elf_Shdr);
        sh = (Elf_Shdr *) malloc(shsize, M_TEMP, M_WAITOK);
-       error = exec_read_from(l, epp->ep_vp, eh->e_shoff, sh, shsize);
+       error = exec_read(l, epp->ep_vp, eh->e_shoff, sh, shsize,
+           IO_NODELOCKED);
        if (error)
                goto out;
 
@@ -126,8 +127,8 @@
                if (s->sh_name + sigsz > sh[shstrndx].sh_size)
                        continue;
 
-               error = exec_read_from(l, epp->ep_vp, stroff + s->sh_name, tbuf,
-                   sigsz);
+               error = exec_read(l, epp->ep_vp, stroff + s->sh_name, tbuf,
+                   sigsz, IO_NODELOCKED);
                if (error)
                        goto out;
                if (!memcmp(tbuf, signature, sigsz)) {
@@ -171,7 +172,8 @@
 
        shsize = eh->e_shnum * sizeof(Elf_Shdr);
        sh = (Elf_Shdr *) malloc(shsize, M_TEMP, M_WAITOK);
-       error = exec_read_from(l, epp->ep_vp, eh->e_shoff, sh, shsize);
+       error = exec_read(l, epp->ep_vp, eh->e_shoff, sh, shsize,
+           IO_NODELOCKED);
        if (error)
                goto out;
 
@@ -189,8 +191,8 @@
                    s->sh_size < sizeof(signature) - 1)
                        continue;
 
-               error = exec_read_from(l, epp->ep_vp, s->sh_offset, tbuf,
-                   sizeof(signature) - 1);
+               error = exec_read(l, epp->ep_vp, s->sh_offset, tbuf,
+                   sizeof(signature) - 1, IO_NODELOCKED);
                if (error)
                        continue;
 
@@ -230,7 +232,8 @@
        /* Load the section header table. */
        shsize = eh->e_shnum * sizeof(Elf_Shdr);
        sh = (Elf_Shdr *) malloc(shsize, M_TEMP, M_WAITOK);
-       error = exec_read_from(l, epp->ep_vp, eh->e_shoff, sh, shsize);
+       error = exec_read(l, epp->ep_vp, eh->e_shoff, sh, shsize,
+           IO_NODELOCKED);
        if (error)
                goto out;
 
@@ -249,8 +252,8 @@
                if (s->sh_name + sigsz > sh[shstrndx].sh_size)
                        continue;
 
-               error = exec_read_from(l, epp->ep_vp, stroff + s->sh_name, tbuf,
-                   sigsz);
+               error = exec_read(l, epp->ep_vp, stroff + s->sh_name, tbuf,
+                   sigsz, IO_NODELOCKED);
                if (error)
                        goto out;
                if (!memcmp(tbuf, signature, sigsz)) {
@@ -290,7 +293,8 @@
        /* Load the section header table. */
        shsize = eh->e_shnum * sizeof(Elf_Shdr);
        sh = malloc(shsize, M_TEMP, M_WAITOK);
-       error = exec_read_from(l, epp->ep_vp, eh->e_shoff, sh, shsize);
+       error = exec_read(l, epp->ep_vp, eh->e_shoff, sh, shsize,
+           IO_NODELOCKED);
        if (error)
                goto out;
 
@@ -309,8 +313,8 @@
                if (s->sh_name + sigsz > sh[shstrndx].sh_size)
                        continue;
 
-               error = exec_read_from(l, epp->ep_vp, stroff + s->sh_name, tbuf,
-                   sigsz);
+               error = exec_read(l, epp->ep_vp, stroff + s->sh_name, tbuf,
+                   sigsz, IO_NODELOCKED);
                if (error)
                        goto out;
                if (!memcmp(tbuf, signature, sigsz)) {
@@ -329,8 +333,8 @@
                sh[i].sh_size = 1024 * 1024;
 
        tmp = malloc(sh[i].sh_size, M_TEMP, M_WAITOK);
-       error = exec_read_from(l, epp->ep_vp, sh[i].sh_offset, tmp,
-           sh[i].sh_size);
+       error = exec_read(l, epp->ep_vp, sh[i].sh_offset, tmp,
+           sh[i].sh_size, IO_NODELOCKED);
        if (error)
                goto out;
 
@@ -368,7 +372,8 @@
 
        phsize = eh->e_phnum * sizeof(Elf_Phdr);
        ph = (Elf_Phdr *)malloc(phsize, M_TEMP, M_WAITOK);
-       error = exec_read_from(l, epp->ep_vp, eh->e_phoff, ph, phsize);
+       error = exec_read(l, epp->ep_vp, eh->e_phoff, ph, phsize,
+           IO_NODELOCKED);
        if (error)
                goto out;
 
@@ -383,8 +388,8 @@
                        continue;
 
                np = (Elf_Nhdr *)malloc(ephp->p_filesz, M_TEMP, M_WAITOK);
-               error = exec_read_from(l, epp->ep_vp, ephp->p_offset, np,
-                   ephp->p_filesz);
+               error = exec_read(l, epp->ep_vp, ephp->p_offset, np,
+                   ephp->p_filesz, IO_NODELOCKED);
                if (error)
                        goto next;
 
diff -r c5caba7578f8 -r ff76e4e9b6e1 sys/kern/exec_elf.c
--- a/sys/kern/exec_elf.c       Sun Jan 12 17:49:17 2020 +0000
+++ b/sys/kern/exec_elf.c       Sun Jan 12 18:30:58 2020 +0000
@@ -1,7 +1,7 @@
-/*     $NetBSD: exec_elf.c,v 1.100 2019/09/16 11:11:34 christos Exp $  */
+/*     $NetBSD: exec_elf.c,v 1.101 2020/01/12 18:30:58 ad Exp $        */
 
 /*-
- * Copyright (c) 1994, 2000, 2005, 2015 The NetBSD Foundation, Inc.
+ * Copyright (c) 1994, 2000, 2005, 2015, 2020 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -57,7 +57,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(1, "$NetBSD: exec_elf.c,v 1.100 2019/09/16 11:11:34 christos Exp $");
+__KERNEL_RCSID(1, "$NetBSD: exec_elf.c,v 1.101 2020/01/12 18:30:58 ad Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_pax.h"
@@ -330,6 +330,8 @@
        long diff, offset;
        int vmprot = 0;
 
+       KASSERT(VOP_ISLOCKED(vp) != LK_NONE);
+
        /*
         * If the user specified an address, then we load there.
         */
@@ -462,14 +464,15 @@
         */
        if (vp->v_type != VREG) {
                error = EACCES;
-               goto badunlock;
+               goto bad;
        }
        if ((error = VOP_ACCESS(vp, VEXEC, l->l_cred)) != 0)
-               goto badunlock;
+               goto bad;
 
        /* get attributes */
+       /* XXX VOP_GETATTR() is the only thing that needs LK_EXCLUSIVE ^ */
        if ((error = VOP_GETATTR(vp, &attr, l->l_cred)) != 0)
-               goto badunlock;
+               goto bad;
 
        /*
         * Check mount point.  Though we're not trying to exec this binary,
@@ -478,18 +481,17 @@
         */
        if (vp->v_mount->mnt_flag & MNT_NOEXEC) {
                error = EACCES;
-               goto badunlock;
+               goto bad;
        }
        if (vp->v_mount->mnt_flag & MNT_NOSUID)
                epp->ep_vap->va_mode &= ~(S_ISUID | S_ISGID);
 
        error = vn_marktext(vp);
        if (error)
-               goto badunlock;
+               goto bad;
 
-       VOP_UNLOCK(vp);
-
-       if ((error = exec_read_from(l, vp, 0, &eh, sizeof(eh))) != 0)
+       error = exec_read(l, vp, 0, &eh, sizeof(eh), IO_NODELOCKED);
+       if (error != 0)
                goto bad;
 
        if ((error = elf_check_header(&eh)) != 0)
@@ -503,7 +505,8 @@
        phsize = eh.e_phnum * sizeof(Elf_Phdr);
        ph = kmem_alloc(phsize, KM_SLEEP);
 
-       if ((error = exec_read_from(l, vp, eh.e_phoff, ph, phsize)) != 0)
+       error = exec_read(l, vp, eh.e_phoff, ph, phsize, IO_NODELOCKED);
+       if (error != 0)
                goto bad;
 
 #ifdef ELF_INTERP_NON_RELOCATABLE
@@ -629,16 +632,13 @@
         * This value is ignored if TOPDOWN.
         */
        *last = addr;
-       vrele(vp);
+       vput(vp);
        return 0;
 
-badunlock:
-       VOP_UNLOCK(vp);
-
 bad:
        if (ph != NULL)
                kmem_free(ph, phsize);
-       vrele(vp);
+       vput(vp);
        return error;
 }
 
@@ -683,9 +683,13 @@
                return ENOEXEC;
        }
 
+       /* XXX only LK_EXCLUSIVE to match all others - allow spinning */
+       vn_lock(epp->ep_vp, LK_EXCLUSIVE | LK_RETRY);
        error = vn_marktext(epp->ep_vp);
-       if (error)
+       if (error) {
+               VOP_UNLOCK(epp->ep_vp);
                return error;
+       }
 
        /*
         * Allocate space to hold all the program headers, and read them
@@ -694,9 +698,12 @@
        phsize = eh->e_phnum * sizeof(Elf_Phdr);
        ph = kmem_alloc(phsize, KM_SLEEP);
 
-       if ((error = exec_read_from(l, epp->ep_vp, eh->e_phoff, ph, phsize)) !=
-           0)
+       error = exec_read(l, epp->ep_vp, eh->e_phoff, ph, phsize,
+           IO_NODELOCKED);
+       if (error != 0) {
+               VOP_UNLOCK(epp->ep_vp);
                goto bad;
+       }
 
        epp->ep_taddr = epp->ep_tsize = ELFDEFNNAME(NO_ADDR);
        epp->ep_daddr = epp->ep_dsize = ELFDEFNNAME(NO_ADDR);
@@ -708,16 +715,21 @@
                                DPRINTF("bad interpreter namelen %#jx",
                                    (uintmax_t)pp->p_filesz);
                                error = ENOEXEC;
+                               VOP_UNLOCK(epp->ep_vp);
                                goto bad;
                        }



Home | Main Index | Thread Index | Old Index