From e8217a1fe0cf341b7219a426f23e02cb44281301 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 12 Oct 2013 07:00:58 +0000
Subject: avoid use-after-free with multi-process setups

readdir on the same DIR pointer is undefined if DIR was inherited by
multiple children.  Using the reentrant readdir_r would not have
helped, since the underlying file descriptor and kernel file handle
were still shared (and we need rewinddir, too).

This readdir usage bug existed in cmogstored since the earliest
releases, but was harmless until the cmogstored 1.3 series.

This misuse of readdir lead to hitting a leftover call to free().
So this bug only manifested since
commit 1fab1e7a7f03f3bc0abb1b5181117f2d4605ce3b
(svc: implement top-level by_mog_devid hash)

Fortunately, these bugs only affect users of the undocumented
multi-process feature (not just multi-threaded).
---
 cmogstored.c |  1 +
 cmogstored.h |  1 +
 svc.c        | 29 +++++++++++++++++++++++++++++
 svc_dev.c    |  5 ++++-
 4 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/cmogstored.c b/cmogstored.c
index 64b932d..31bdd85 100644
--- a/cmogstored.c
+++ b/cmogstored.c
@@ -419,6 +419,7 @@ static void fork_worker(unsigned worker_id)
 		mog_process_register(pid, worker_id);
 	} else if (pid == 0) {
 		mog_process_reset();
+		mog_svc_each(mog_svc_atfork_child, &parent);
 
 		/* worker will call mog_intr_enable() later in notify loop */
 		run_worker(parent);
diff --git a/cmogstored.h b/cmogstored.h
index 3c1801c..1006fc3 100644
--- a/cmogstored.h
+++ b/cmogstored.h
@@ -371,6 +371,7 @@ bool mog_svc_start_each(void *svc_ptr, void *have_mgmt_ptr);
 void mog_svc_thrpool_rescale(struct mog_svc *, unsigned ndev_new);
 void mog_svc_aio_threads_enqueue(struct mog_svc *, unsigned nr);
 void mog_svc_aio_threads_handler(void);
+bool mog_svc_atfork_child(void *svc_ptr, void *parent);
 
 /* dev.c */
 struct mog_dev *mog_dev_for(struct mog_svc *, uint32_t mog_devid, bool update);
diff --git a/svc.c b/svc.c
index 4a44493..86f69db 100644
--- a/svc.c
+++ b/svc.c
@@ -69,6 +69,35 @@ static void svc_once(void)
 	atexit(svc_atexit);
 }
 
+bool mog_svc_atfork_child(void *svc_ptr, void *parent)
+{
+	struct mog_svc *svc = svc_ptr;
+	pid_t ppid = *((pid_t *)parent);
+	const char *failfn;
+
+	if (closedir(svc->dir) < 0) {
+		failfn = "closedir";
+		goto err;
+	}
+
+	svc->dir = opendir(svc->docroot);
+	if (svc->dir == NULL) {
+		failfn = "opendir";
+		goto err;
+	}
+
+	svc->docroot_fd = dirfd(svc->dir);
+	if (svc->docroot_fd < 0) {
+		failfn = "dirfd";
+		goto err;
+	}
+	return true;
+err:
+	syslog(LOG_ERR, "%s(%s) failed with: %m", failfn, svc->docroot);
+	kill(ppid, SIGTERM);
+	return false;
+}
+
 struct mog_svc * mog_svc_new(const char *docroot)
 {
 	struct mog_svc *svc;
diff --git a/svc_dev.c b/svc_dev.c
index b2beec3..b6a1e71 100644
--- a/svc_dev.c
+++ b/svc_dev.c
@@ -129,7 +129,10 @@ static int svc_scandev(struct mog_svc *svc, unsigned *nr, mog_scandev_cb cb)
 
 		switch (hash_insert_if_absent(devhash, dev, NULL)) {
 		case 0:
-			free(dev);
+			/* do not free dev, it is in svc->by_mog_devid */
+			syslog(LOG_ERR,
+			       "%s/%s seen twice in readdir (BUG?)",
+			       svc->docroot, ent->d_name);
 			break;
 		case 1:
 			(*nr)++;
-- 
cgit v1.2.3-24-ge0c7