git: fc369a353b5b - main - ktrace: fix a race between writes and close

[Konstantin Belousov]

The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=fc369a353b5b5e0f8046687fcbd78a7cd9ad1810

commit fc369a353b5b5e0f8046687fcbd78a7cd9ad1810
Author:     Konstantin Belousov <kib at FreeBSD.org>
AuthorDate: 2021-05-22 12:40:00 +0000
Commit:     Konstantin Belousov <kib at FreeBSD.org>
CommitDate: 2021-05-22 20:14:13 +0000

    ktrace: fix a race between writes and close
    
    It was possible that termination of ktrace session occured during some
    record write, in which case write occured after the close of the vnode.
    Use ktr_io_params refcounting to avoid this situation, by taking the
    reference on the structure instead of vnode.
    
    Reviewed by:    markj
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D30400
---
 sys/kern/kern_ktrace.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sys/kern/kern_ktrace.c b/sys/kern/kern_ktrace.c
index 875c079df3b9..3a223215a60d 100644
--- a/sys/kern/kern_ktrace.c
+++ b/sys/kern/kern_ktrace.c
@@ -1257,7 +1257,7 @@ ktrsetchildren(struct thread *td, struct proc *top, int ops, int facs,
 static void
 ktr_writerequest(struct thread *td, struct ktr_request *req)
 {
-	struct ktr_io_params *kiop;
+	struct ktr_io_params *kiop, *kiop1;
 	struct ktr_header *kth;
 	struct vnode *vp;
 	struct proc *p;
@@ -1272,14 +1272,10 @@ ktr_writerequest(struct thread *td, struct ktr_request *req)
 	p = td->td_proc;
 
 	/*
-	 * We hold the vnode and credential for use in I/O in case ktrace is
+	 * We reference the kiop for use in I/O in case ktrace is
 	 * disabled on the process as we write out the request.
-	 *
-	 * XXXRW: This is not ideal: we could end up performing a write after
-	 * the vnode has been closed.
 	 */
 	mtx_lock(&ktrace_mtx);
-
 	kiop = p->p_ktrioparms;
 
 	/*
@@ -1291,13 +1287,12 @@ ktr_writerequest(struct thread *td, struct ktr_request *req)
 		return;
 	}
 
+	ktr_io_params_ref(kiop);
 	vp = kiop->vp;
 	cred = kiop->cr;
 	lim = kiop->lim;
 
-	vrefact(vp);
 	KASSERT(cred != NULL, ("ktr_writerequest: cred == NULL"));
-	crhold(cred);
 	mtx_unlock(&ktrace_mtx);
 
 	kth = &req->ktr_header;
@@ -1339,9 +1334,11 @@ ktr_writerequest(struct thread *td, struct ktr_request *req)
 		error = VOP_WRITE(vp, &auio, IO_UNIT | IO_APPEND, cred);
 	VOP_UNLOCK(vp);
 	vn_finished_write(mp);
-	crfree(cred);
 	if (error == 0) {
-		vrele(vp);
+		mtx_lock(&ktrace_mtx);
+		kiop = ktr_io_params_rele(kiop);
+		mtx_unlock(&ktrace_mtx);
+		ktr_io_params_free(kiop);
 		return;
 	}
 
@@ -1354,12 +1351,15 @@ ktr_writerequest(struct thread *td, struct ktr_request *req)
 	    "ktrace write failed, errno %d, tracing stopped for pid %d\n",
 	    error, p->p_pid);
 
+	kiop1 = NULL;
 	PROC_LOCK(p);
 	mtx_lock(&ktrace_mtx);
 	if (p->p_ktrioparms != NULL && p->p_ktrioparms->vp == vp)
-		kiop = ktr_freeproc(p);
+		kiop1 = ktr_freeproc(p);
+	kiop = ktr_io_params_rele(kiop);
 	mtx_unlock(&ktrace_mtx);
 	PROC_UNLOCK(p);
+	ktr_io_params_free(kiop1);
 	ktr_io_params_free(kiop);
 	vrele(vp);
 }