Module Name:    src
Committed By:   christos
Date:           Fri Dec 30 20:33:04 UTC 2011

Modified Files:
        src/sys/kern: kern_ktrace.c

Log Message:
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.


To generate a diff of this commit:
cvs rdiff -u -r1.159 -r1.160 src/sys/kern/kern_ktrace.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/kern/kern_ktrace.c
diff -u src/sys/kern/kern_ktrace.c:1.159 src/sys/kern/kern_ktrace.c:1.160
--- src/sys/kern/kern_ktrace.c:1.159	Wed Nov 30 05:27:46 2011
+++ src/sys/kern/kern_ktrace.c	Fri Dec 30 15:33:04 2011
@@ -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 @@ struct ktr_desc {
 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 @@ ktr_mib(const int *name, u_int namelen)
 /* 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_common(lwp_t *curl, int ops, int 
 			    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 @@ ktrace_common(lwp_t *curl, int ops, int 
 	 */
 	if (!facs) {
 		error = EINVAL;
+		*fpp = NULL;
 		goto done;
 	}
 
@@ -1181,6 +1184,7 @@ ktrace_common(lwp_t *curl, int ops, int 
 	mutex_exit(proc_lock);
 	if (error == 0 && !ret)
 		error = EPERM;
+	*fpp = NULL;
 done:
 	if (ktd != NULL) {
 		mutex_enter(&ktrace_lock);
@@ -1222,7 +1226,7 @@ sys_fktrace(struct lwp *l, const struct 
 		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 @@ sys_ktrace(struct lwp *l, const struct s
 		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);
 }
 

Reply via email to