[Devel] [PATCH vz10 20/24] dm-ploop: fix NULL-derefs on the ctr error / fault-injection paths

[Konstantin Khorenko]

Two NULL-dereferences reachable when a ploop_ctr() allocation fails (or is
made to fail via CONFIG_FAULT_INJECTION):

1) ploop_destroy() stops both worker kthreads under a single
   "if (ploop->kt_worker)" guard and dereferences ploop->kt_allocator->task
   unconditionally. ploop_ctr() creates kt_worker first and kt_allocator
   second (a second ploop_worker_create()); if that second create fails,
   the error unwind calls ploop_destroy() with kt_worker set but
   kt_allocator still NULL, so kthread_stop(ploop->kt_allocator->task)
   NULL-derefs. Guard the kt_allocator teardown with its own NULL check
   (the following kfree() is already NULL-safe).

2) ploop_get_delta_file() is annotated ALLOW_ERROR_INJECTION(..., ERRNO_NULL)
   but only ever returns a valid file or ERR_PTR(); its sole caller,
   ploop_add_deltas_stack(), checks IS_ERR() only and then passes the file
   to vfs_fsync(). fail_function's default retval for ERRNO_NULL is 0 (NULL),
   so default-configured injection returns NULL, slips past IS_ERR(), and
   NULL-derefs in vfs_fsync(). The function never legitimately returns NULL,
   so the correct injection class is ERRNO (error pointers only).

Fixes: 69aa8c83a32e ("dm-ploop: make filespace preallocations async")
Fixes: a73b362376db ("dm-ploop: Allow fault injection for all ploop functions")
Feature: dm-ploop: ploop target driver
https://virtuozzo.atlassian.net/browse/VSTOR-137234
Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 drivers/md/dm-ploop-target.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-ploop-target.c b/drivers/md/dm-ploop-target.c
index 0de0e84d440d..3b589579741c 100644
--- a/drivers/md/dm-ploop-target.c
+++ b/drivers/md/dm-ploop-target.c
@@ -188,7 +188,9 @@ static void ploop_destroy(struct ploop *ploop)
 
 		/* waits for the thread to stop */
 		kthread_stop(ploop->kt_worker->task);
-		kthread_stop(ploop->kt_allocator->task);
+		/* kt_allocator may be NULL if ploop_ctr() failed early */
+		if (ploop->kt_allocator)
+			kthread_stop(ploop->kt_allocator->task);
 
 		WARN_ON(!llist_empty(&ploop->pios[PLOOP_LIST_PREPARE]));
 		WARN_ON(!llist_empty(&ploop->llresubmit_pios));
@@ -255,7 +257,7 @@ static struct file *ploop_get_delta_file(struct ploop *ploop, int fd)
 	return ERR_PTR(ret);
 
 }
-ALLOW_ERROR_INJECTION(ploop_get_delta_file, ERRNO_NULL);
+ALLOW_ERROR_INJECTION(ploop_get_delta_file, ERRNO);
 
 static int ploop_check_top_delta(struct ploop *ploop, struct file *file)
 {
-- 
2.47.1