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); }