[src/trunk]: src/sys/kern Avoid panic on DIAGNOSTIC kernels with ktrace -p <n...

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/e4becab13767
branches:  trunk
changeset: 772359:e4becab13767
user:      christos <christos%NetBSD.org@localhost>
date:      Fri Dec 30 20:33:04 2011 +0000

description:
Avoid panic on DIAGNOSTIC kernels with ktrace -p <not-existing-process>
The old logic was:

        error = ktrace_common(, fp);
        if (fp)
                if (error)
                        fd_abort(, fp, );
                else
                        fd_abort(, NULL, );

The 'if (fp)' portion really means if the op is not KTROP_CLEAR,
since the logic above always sets up fp otherwise, so change the
code to test this directly.

ktrace_common() can return an error both on the kernel thread
creation failure, which means that we should be calling fd_abort()
with fp, since nobody used the file yet and we should clear it now.
But it can also return an error because later, after the thread
creation if the process or process group was not found. In this
second case, we should be calling fd_abort with NULL, since the fp
is now used by the thread and it is going to clean it later. So
instead of checking the error from ktrace_common() to decide if we
are going to call fd_abort() with a NULL fp or not, let krace_common()
decide for us.  So the new logic becomes:

        error = ktrace_common(, &fp);
        if (op != KTROP_CLEAR)
                fd_abort(, fp, );

Since I am here, fix a freed memory access, by setting ktd to FALSE.

diffstat:

 sys/kern/kern_ktrace.c |  27 ++++++++++++---------------
 1 files changed, 12 insertions(+), 15 deletions(-)

diffs (95 lines):

diff -r da579507cb41 -r e4becab13767 sys/kern/kern_ktrace.c
--- a/sys/kern/kern_ktrace.c    Fri Dec 30 20:11:23 2011 +0000
+++ b/sys/kern/kern_ktrace.c    Fri Dec 30 20:33:04 2011 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_ktrace.c,v 1.159 2011/11/30 10:27:46 mbalmer Exp $        */
+/*     $NetBSD: kern_ktrace.c,v 1.160 2011/12/30 20:33:04 christos Exp $       */
 
 /*-
  * Copyright (c) 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_ktrace.c,v 1.159 2011/11/30 10:27:46 mbalmer Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_ktrace.c,v 1.160 2011/12/30 20:33:04 christos Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -132,7 +132,7 @@
 static int     ktealloc(struct ktrace_entry **, void **, lwp_t *, int,
                         size_t);
 static void    ktrwrite(struct ktr_desc *, struct ktrace_entry *);
-static int     ktrace_common(lwp_t *, int, int, int, file_t *);
+static int     ktrace_common(lwp_t *, int, int, int, file_t **);
 static int     ktrops(lwp_t *, struct proc *, int, int,
                    struct ktr_desc *);
 static int     ktrsetchildren(lwp_t *, struct proc *, int, int,
@@ -1045,12 +1045,13 @@
 /* Interface and common routines */
 
 int
-ktrace_common(lwp_t *curl, int ops, int facs, int pid, file_t *fp)
+ktrace_common(lwp_t *curl, int ops, int facs, int pid, file_t **fpp)
 {
        struct proc *curp;
        struct proc *p;
        struct pgrp *pg;
        struct ktr_desc *ktd = NULL;
+       file_t *fp = *fpp;
        int ret = 0;
        int error = 0;
        int descend;
@@ -1112,6 +1113,7 @@
                            ktrace_thread, ktd, &ktd->ktd_lwp, "ktrace");
                        if (error != 0) {
                                kmem_free(ktd, sizeof(*ktd));
+                               ktd = NULL;
                                mutex_enter(&fp->f_lock);
                                fp->f_count--;
                                mutex_exit(&fp->f_lock);
@@ -1141,6 +1143,7 @@
         */
        if (!facs) {
                error = EINVAL;
+               *fpp = NULL;
                goto done;
        }
 
@@ -1181,6 +1184,7 @@
        mutex_exit(proc_lock);
        if (error == 0 && !ret)
                error = EPERM;
+       *fpp = NULL;
 done:
        if (ktd != NULL) {
                mutex_enter(&ktrace_lock);
@@ -1222,7 +1226,7 @@
                error = EBADF;
        else
                error = ktrace_common(l, SCARG(uap, ops),
-                   SCARG(uap, facs), SCARG(uap, pid), fp);
+                   SCARG(uap, facs), SCARG(uap, pid), &fp);
        fd_putfile(fd);
        return error;
 }
@@ -1290,16 +1294,9 @@
                vp = NULL;
        }
        error = ktrace_common(l, SCARG(uap, ops), SCARG(uap, facs),
-           SCARG(uap, pid), fp);
-       if (fp != NULL) {
-               if (error != 0) {
-                       /* File unused. */
-                       fd_abort(curproc, fp, fd);
-               } else {
-                       /* File was used. */
-                       fd_abort(curproc, NULL, fd);
-               }
-       }
+           SCARG(uap, pid), &fp);
+       if (KTROP(SCARG(uap, ops)) != KTROP_CLEAR)
+               fd_abort(curproc, fp, fd);
        return (error);
 }