From: anton <anton@openbsd.org>
Date: Tue, 11 Jan 2022 06:00:41 +0000 (+0000)
Subject: In revision 1.43 of kcov.c, the redundant conditional of checking for
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=c1a72b652b73c91009631bf5d600078106f9d359;p=openbsd

In revision 1.43 of kcov.c, the redundant conditional of checking for
an exising kcov descriptor with the given device minor was removed since
kcov is a cloning device; i.e. the device minor should always be unique.

However, there's one edge case to still consider in which one thread
have tracing enabled while another thread closes the same kcov
descriptor. The kcov descriptor is kept alive until thread with tracing
enabled exits to prevent usage after free. This does however cause the
spec file layer above to flag the device minor as unused. Any subsequent
open of /dev/kcov would trip on the assertion in kcovopen() until the
thread with tracing enabled exits.

Therefore unconditionally remove the kcov descriptor from the global
list of active descriptors which is fine since the same kcov descriptor
will later be freed in kcov_exit().

I have never seen this in the wild but realized while hunting another
bug.
---

diff --git a/sys/dev/kcov.c b/sys/dev/kcov.c
index 31231b20dc7..e1e7fbbf091 100644
--- a/sys/dev/kcov.c
+++ b/sys/dev/kcov.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kcov.c,v 1.44 2021/12/29 07:15:13 anton Exp $	*/
+/*	$OpenBSD: kcov.c,v 1.45 2022/01/11 06:00:41 anton Exp $	*/
 
 /*
  * Copyright (c) 2018 Anton Lindqvist <anton@openbsd.org>
@@ -319,6 +319,7 @@ kcovclose(dev_t dev, int flag, int mode, struct proc *p)
 		return (ENXIO);
 	}
 
+	TAILQ_REMOVE(&kd_list, kd, kd_entry);
 	if (kd->kd_state == KCOV_STATE_TRACE && kd->kd_kr == NULL) {
 		/*
 		 * Another thread is currently using the kcov descriptor,
@@ -548,8 +549,6 @@ kd_free(struct kcov_dev *kd)
 
 	MUTEX_ASSERT_LOCKED(&kcov_mtx);
 
-	TAILQ_REMOVE(&kd_list, kd, kd_entry);
-
 	kr = kd->kd_kr;
 	if (kr != NULL)
 		kcov_remote_detach(kd, kr);