Source-Changes-HG archive

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

[src/trunk]: src/sys PAX_SEGVGUARD doesn't seem to work properly in testing f...



details:   https://anonhg.NetBSD.org/src/rev/75491366df6a
branches:  trunk
changeset: 848281:75491366df6a
user:      ad <ad%NetBSD.org@localhost>
date:      Thu Jan 23 10:21:14 2020 +0000

description:
PAX_SEGVGUARD doesn't seem to work properly in testing for me, but at least
make it not cause problems:

- Cover it with exec_lock so the updates are not racy.
- Using fileassoc is silly.  Just hang a pointer off the vnode.

diffstat:

 sys/kern/kern_pax.c  |  44 +++++++++++++++++---------------------------
 sys/kern/kern_sig.c  |   7 +++++--
 sys/kern/vfs_vnode.c |  11 +++++++++--
 sys/sys/pax.h        |   4 +++-
 sys/sys/vnode.h      |   4 +++-
 5 files changed, 37 insertions(+), 33 deletions(-)

diffs (245 lines):

diff -r bbe6e7213a1c -r 75491366df6a sys/kern/kern_pax.c
--- a/sys/kern/kern_pax.c       Thu Jan 23 10:05:44 2020 +0000
+++ b/sys/kern/kern_pax.c       Thu Jan 23 10:21:14 2020 +0000
@@ -1,7 +1,7 @@
-/*     $NetBSD: kern_pax.c,v 1.60 2017/06/25 04:10:47 snj Exp $        */
+/*     $NetBSD: kern_pax.c,v 1.61 2020/01/23 10:21:14 ad Exp $ */
 
 /*
- * Copyright (c) 2015 The NetBSD Foundation, Inc.
+ * Copyright (c) 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(0, "$NetBSD: kern_pax.c,v 1.60 2017/06/25 04:10:47 snj Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_pax.c,v 1.61 2020/01/23 10:21:14 ad Exp $");
 
 #include "opt_pax.h"
 
@@ -69,7 +69,6 @@
 #include <sys/sysctl.h>
 #include <sys/kmem.h>
 #include <sys/mman.h>
-#include <sys/fileassoc.h>
 #include <sys/syslog.h>
 #include <sys/vnode.h>
 #include <sys/queue.h>
@@ -155,8 +154,6 @@
 static int pax_segvguard_suspension = PAX_SEGVGUARD_SUSPENSION;
 static int pax_segvguard_maxcrashes = PAX_SEGVGUARD_MAXCRASHES;
 
-static fileassoc_t segvguard_id;
-
 struct pax_segvguard_uid_entry {
        uid_t sue_uid;
        size_t sue_ncrashes;
@@ -170,7 +167,6 @@
 };
 
 static bool pax_segvguard_elf_flags_active(uint32_t);
-static void pax_segvguard_cleanup_cb(void *);
 #endif /* PAX_SEGVGUARD */
 
 SYSCTL_SETUP(sysctl_security_pax_setup, "sysctl security.pax setup")
@@ -338,15 +334,6 @@
 void
 pax_init(void)
 {
-#ifdef PAX_SEGVGUARD
-       int error;
-
-       error = fileassoc_register("segvguard", pax_segvguard_cleanup_cb,
-           &segvguard_id);
-       if (error) {
-               panic("pax_init: segvguard_id: error=%d\n", error);
-       }
-#endif /* PAX_SEGVGUARD */
 #ifdef PAX_ASLR
        /* Adjust maximum stack by the size we can consume for ASLR */
        extern rlim_t maxsmap;
@@ -704,13 +691,13 @@
        return true;
 }
 
-static void
-pax_segvguard_cleanup_cb(void *v)
+void
+pax_segvguard_cleanup(struct vnode *vp)
 {
-       struct pax_segvguard_entry *p = v;
+       struct pax_segvguard_entry *p = vp->v_segvguard;
        struct pax_segvguard_uid_entry *up;
 
-       if (p == NULL) {
+       if (__predict_true(p == NULL)) {
                return;
        }
        while ((up = LIST_FIRST(&p->segv_uids)) != NULL) {
@@ -718,14 +705,17 @@
                kmem_free(up, sizeof(*up));
        }
        kmem_free(p, sizeof(*p));
+       vp->v_segvguard = NULL;
 }
 
 /*
  * Called when a process of image vp generated a segfault.
+ *
+ * => exec_lock must be held by the caller
+ * => if "crashed" is true, exec_lock must be held for write
  */
 int
-pax_segvguard(struct lwp *l, struct vnode *vp, const char *name,
-    bool crashed)
+pax_segvguard(struct lwp *l, struct vnode *vp, const char *name, bool crashed)
 {
        struct pax_segvguard_entry *p;
        struct pax_segvguard_uid_entry *up;
@@ -734,6 +724,9 @@
        uint32_t flags;
        bool have_uid;
 
+       KASSERT(rw_lock_held(&exec_lock));
+       KASSERT(!crashed || rw_write_held(&exec_lock));
+
        flags = l->l_proc->p_pax;
        if (!pax_flags_active(flags, P_PAX_GUARD))
                return 0;
@@ -741,11 +734,8 @@
        if (vp == NULL)
                return EFAULT;  
 
-       /* Check if we already monitor the file. */
-       p = fileassoc_lookup(vp, segvguard_id);
-
        /* Fast-path if starting a program we don't know. */
-       if (p == NULL && !crashed)
+       if ((p = vp->v_segvguard) == NULL && !crashed)
                return 0;
 
        microtime(&tv);
@@ -756,7 +746,7 @@
         */
        if (p == NULL) {
                p = kmem_alloc(sizeof(*p), KM_SLEEP);
-               fileassoc_add(vp, segvguard_id, p);
+               vp->v_segvguard = p;
                LIST_INIT(&p->segv_uids);
 
                /*
diff -r bbe6e7213a1c -r 75491366df6a sys/kern/kern_sig.c
--- a/sys/kern/kern_sig.c       Thu Jan 23 10:05:44 2020 +0000
+++ b/sys/kern/kern_sig.c       Thu Jan 23 10:21:14 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_sig.c,v 1.381 2019/12/06 21:36:10 ad Exp $        */
+/*     $NetBSD: kern_sig.c,v 1.382 2020/01/23 10:21:14 ad Exp $        */
 
 /*-
  * Copyright (c) 2006, 2007, 2008, 2019 The NetBSD Foundation, Inc.
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_sig.c,v 1.381 2019/12/06 21:36:10 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_sig.c,v 1.382 2020/01/23 10:21:14 ad Exp $");
 
 #include "opt_ptrace.h"
 #include "opt_dtrace.h"
@@ -2304,8 +2304,11 @@
                }
 
 #ifdef PAX_SEGVGUARD
+               rw_enter(&exec_lock, RW_WRITER);
                pax_segvguard(l, p->p_textvp, p->p_comm, true);
+               rw_exit(&exec_lock);
 #endif /* PAX_SEGVGUARD */
+
                /* Acquire the sched state mutex.  exit1() will release it. */
                mutex_enter(p->p_lock);
                if (error == 0)
diff -r bbe6e7213a1c -r 75491366df6a sys/kern/vfs_vnode.c
--- a/sys/kern/vfs_vnode.c      Thu Jan 23 10:05:44 2020 +0000
+++ b/sys/kern/vfs_vnode.c      Thu Jan 23 10:21:14 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_vnode.c,v 1.107 2020/01/12 17:49:17 ad Exp $       */
+/*     $NetBSD: vfs_vnode.c,v 1.108 2020/01/23 10:21:14 ad Exp $       */
 
 /*-
  * Copyright (c) 1997-2011, 2019 The NetBSD Foundation, Inc.
@@ -146,7 +146,9 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.107 2020/01/12 17:49:17 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.108 2020/01/23 10:21:14 ad Exp $");
+
+#include "opt_pax.h"
 
 #include <sys/param.h>
 #include <sys/kernel.h>
@@ -162,6 +164,7 @@
 #include <sys/module.h>
 #include <sys/mount.h>
 #include <sys/namei.h>
+#include <sys/pax.h>
 #include <sys/syscallargs.h>
 #include <sys/sysctl.h>
 #include <sys/systm.h>
@@ -1675,6 +1678,10 @@
        mutex_enter(vp->v_interlock);
        fstrans_done(mp);
        KASSERT((vp->v_iflag & VI_ONWORKLST) == 0);
+
+#ifdef PAX_SEGVGUARD
+       pax_segvguard_cleanup(vp);
+#endif /* PAX_SEGVGUARD */
 }
 
 /*
diff -r bbe6e7213a1c -r 75491366df6a sys/sys/pax.h
--- a/sys/sys/pax.h     Thu Jan 23 10:05:44 2020 +0000
+++ b/sys/sys/pax.h     Thu Jan 23 10:21:14 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: pax.h,v 1.26 2017/05/06 21:34:52 joerg Exp $ */
+/* $NetBSD: pax.h,v 1.27 2020/01/23 10:21:14 ad Exp $ */
 
 /*-
  * Copyright (c) 2006 Elad Efrat <elad%NetBSD.org@localhost>
@@ -53,6 +53,8 @@
 extern int pax_aslr_debug;
 #endif
 
+void   pax_segvguard_cleanup(struct vnode *);
+
 #if defined(PAX_MPROTECT) || defined(PAX_SEGVGUARD) || defined(PAX_ASLR)
 void pax_init(void);
 void pax_set_flags(struct exec_package *, struct proc *);
diff -r bbe6e7213a1c -r 75491366df6a sys/sys/vnode.h
--- a/sys/sys/vnode.h   Thu Jan 23 10:05:44 2020 +0000
+++ b/sys/sys/vnode.h   Thu Jan 23 10:21:14 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vnode.h,v 1.286 2019/12/22 19:47:34 ad Exp $   */
+/*     $NetBSD: vnode.h,v 1.287 2020/01/23 10:21:14 ad Exp $   */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -124,6 +124,7 @@
  * lock.  Field markings and the corresponding locks:
  *
  *     :       stable, reference to the vnode is required
+ *     e       exec_lock
  *     f       vnode_free_list_lock, or vrele_lock for vrele_list
  *     i       v_interlock
  *     u       locked by underlying filesystem
@@ -159,6 +160,7 @@
        enum vtagtype   v_tag;                  /* :: type of underlying data */
        void            *v_data;                /* :: private data for fs */
        struct klist    v_klist;                /* i: notes attached to vnode */
+       void            *v_segvguard;           /* e: for PAX_SEGVGUARD */
 };
 #define        v_usecount      v_uobj.uo_refs
 #define        v_interlock     v_uobj.vmobjlock



Home | Main Index | Thread Index | Old Index