DragonFlyBSD Kernel Audit
DF-2940 / fix.diff
← back to finding ↓ download raw
--- fix.diff ---
# DF-2940: mpipe teardown while dm-crypt I/O in flight
#
# Part 1 (sys/kern/kern_mpipe.c): mpipe_done() must not free the pipe
#   while buffers are still checked out, and must let queued allocation
#   retries complete instead of orphaning them.
# Part 2 (sys/dev/disk/dm/crypt/dm_target_crypt.c): the only in-tree
#   teardown caller must wait for its in-flight bios to return their
#   mpipe buffers before starting teardown (this is the part that was
#   build+run validated on the guest: every dmtc_destroy entry saw
#   rd free=20/total=20 wr free=20/total=20 after the wait, where the
#   baseline repeatedly saw wr free=8..19/total=20).

diff --git a/sys/kern/kern_mpipe.c b/sys/kern/kern_mpipe.c
index 1111111..2222222 100644
--- a/sys/kern/kern_mpipe.c
+++ b/sys/kern/kern_mpipe.c
@@ -122,17 +122,37 @@ mpipe_thread(void *arg)
 /*
  * Destroy a previously initialized mpipe.  This routine can also safely be
  * called on an uninitialized mpipe structure if it was zero'd or mpipe_done()
  * was previously called on it.
  */
 void
 mpipe_done(malloc_pipe_t mpipe)
 {
     void *buf;
+    struct mpipe_callback *mcb;
     int n;
 
+    /*
+     * DF-2940: wait for buffers that are still checked out to be
+     * returned by in-flight users before freeing anything.  Callers
+     * must guarantee no new allocations are started before calling
+     * mpipe_done(); in-flight frees are what we are waiting for.
+     */
+    for (n = 0; n < 600 && mpipe->free_count != mpipe->total_count; ++n)
+	tsleep(mpipe, 0, "mpipedn", 1);
+
     KKASSERT(mpipe->free_count == mpipe->total_count);	/* no outstanding mem */
 
     /*
      * Clean up the kthread
      */
     lwkt_gettoken(&mpipe->token);
+    /*
+     * Let the support thread run queued allocation retries to
+     * completion (free_count == total_count guarantees the retries
+     * can obtain buffers), instead of orphaning the parked bios.
+     */
+    while (STAILQ_FIRST(&mpipe->queue) != NULL) {
+	wakeup(&mpipe->queue);
+	lwkt_reltoken(&mpipe->token);
+	tsleep(&mpipe->queue, 0, "mpipedq", 1);
+	lwkt_gettoken(&mpipe->token);
+    }
     mpipe->mpflags |= MPF_EXITING;
     while (mpipe->thread) {
 	wakeup(&mpipe->queue);
 	tsleep(mpipe, 0, "mpipex", 1);
     }
diff --git a/sys/dev/disk/dm/crypt/dm_target_crypt.c b/sys/dev/disk/dm/crypt/dm_target_crypt.c
index 3333333..4444444 100644
--- a/sys/dev/disk/dm/crypt/dm_target_crypt.c
+++ b/sys/dev/disk/dm/crypt/dm_target_crypt.c
@@ -200,10 +200,32 @@ dmtc_zero_mpipe_buffer(void *buffer, void *priv __unused)
 	explicit_bzero(buffer, DMTC_BUF_SIZE);
 }
 
+/*
+ * DF-2940: wait until every in-flight crypt bio has returned its mpipe
+ * buffer and no allocation retry is still parked in the mpipe queue.
+ *
+ * The dm framework releases the table reference as soon as the target's
+ * strategy routine returns, so bios issued through this table can still
+ * be executing the encrypt/decrypt path when the table is destroyed
+ * (DM_DEV_REMOVE, or suspend/load/resume table switch).  Without this
+ * wait, mpipe_done() is entered with buffers checked out (observed
+ * 1..12 outstanding on the reproducer), tripping its KKASSERT on
+ * INVARIANTS kernels and freeing live buffers otherwise.
+ */
+static void
+dmtc_wait_mpipe_drain(struct malloc_pipe *mpipe)
+{
+	int n;
+
+	for (n = 0; n < 600; ++n) {	/* up to ~60 seconds */
+		lwkt_gettoken(&mpipe->token);
+		if (mpipe->free_count == mpipe->total_count &&
+		    STAILQ_FIRST(&mpipe->queue) == NULL) {
+			lwkt_reltoken(&mpipe->token);
+			return;
+		}
+		lwkt_reltoken(&mpipe->token);
+		tsleep(mpipe, 0, "dmtcdrn", hz / 10 + 1);
+	}
+	kprintf("dm_target_crypt: mpipe drain timeout, tearing down anyway\n");
+}
+
 static void
 dmtc_init_mpipe(struct target_crypt_config *priv)
 {
@@ -605,6 +627,10 @@ dm_target_crypt_destroy(dm_table_entry_t *table_en)
 	priv = table_en->target_config;
 	if (priv == NULL)
 		return 0;
+
+	dmtc_wait_mpipe_drain(&priv->read_mpipe);
+	dmtc_wait_mpipe_drain(&priv->write_mpipe);
+
 	dm_pdev_decr(priv->pdev);
 
 	dmtc_destroy_mpipe(priv);