[ltt-dev] [PATCH v2] lttng: Fix potential invalid pointer derefference

[Gui Jianfeng]

Hi Mathieu,

Calling get_ltt_root() and put_ltt_root() pair may induce invalid pointer
derefference. Consider the following scenario:

         CPU 0                   CPU 1
  ----------------------  ----------------------
1  root = get_ltt_root()   
2                          root = get_ltt_root()
3                          put_ltt_root() //global root is freed
4  using root(crash here)

Here is the fix for this issue. Thanks a lot to Zhaolei to point this out
in offline.

This patch assumes that the *ltt root remove* patch has been applied.

Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujistu.com>
---
 include/linux/ltt-core.h  |    3 ++-
 ltt/ltt-core.c            |   25 +++++++++++++++++++++----
 ltt/ltt-relay.c           |   10 +++++++++-
 ltt/ltt-userspace-event.c |    3 +++
 4 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/include/linux/ltt-core.h b/include/linux/ltt-core.h
index 28394cb..786c2b1 100644
--- a/include/linux/ltt-core.h
+++ b/include/linux/ltt-core.h
@@ -30,9 +30,10 @@ extern struct ltt_traces ltt_traces;
 /*
  * get dentry of ltt's root dir
  */
-void put_ltt_root(void);
 struct dentry *get_ltt_root(void);
 
+void put_ltt_root(void);
+
 /* Keep track of trap nesting inside LTT */
 DECLARE_PER_CPU(unsigned int, ltt_nesting);
 
diff --git a/ltt/ltt-core.c b/ltt/ltt-core.c
index 314750b..03b81ca 100644
--- a/ltt/ltt-core.c
+++ b/ltt/ltt-core.c
@@ -10,6 +10,7 @@
 #include <linux/percpu.h>
 #include <linux/module.h>
 #include <linux/debugfs.h>
+#include <linux/kref.h>
 
 /* Traces structures */
 struct ltt_traces ltt_traces = {
@@ -23,12 +24,22 @@ static DEFINE_MUTEX(ltt_traces_mutex);
 
 /* dentry of ltt's root dir */
 static struct dentry *ltt_root_dentry;
+static struct kref ltt_root_kref = {
+	.refcount = ATOMIC_INIT(0),
+};
+
+static void ltt_root_release(struct kref *ref)
+{
+	return;
+}
 
 void put_ltt_root(void)
 {
-	if (ltt_root_dentry && list_empty(&ltt_root_dentry->d_subdirs)) {
-		debugfs_remove(ltt_root_dentry);
-		ltt_root_dentry = NULL;
+	if (ltt_root_dentry) {
+		if (kref_put(&ltt_root_kref, ltt_root_release)) {
+			debugfs_remove(ltt_root_dentry);
+			ltt_root_dentry = NULL;
+		}
 	}
 }
 EXPORT_SYMBOL_GPL(put_ltt_root);
@@ -37,9 +48,15 @@ struct dentry *get_ltt_root(void)
 {
 	if (!ltt_root_dentry) {
 		ltt_root_dentry = debugfs_create_dir(LTT_ROOT, NULL);
-		if (!ltt_root_dentry)
+		if (!ltt_root_dentry) {
 			printk(KERN_ERR "LTT : create ltt root dir failed\n");
+			goto out;
+		}
+		kref_init(&ltt_root_kref);
+		goto out;
 	}
+	kref_get(&ltt_root_kref);
+out:
 	return ltt_root_dentry;
 }
 EXPORT_SYMBOL_GPL(get_ltt_root);
diff --git a/ltt/ltt-relay.c b/ltt/ltt-relay.c
index 7c544f4..8378802 100644
--- a/ltt/ltt-relay.c
+++ b/ltt/ltt-relay.c
@@ -833,8 +833,16 @@ end:
 
 static int ltt_relay_create_dirs(struct ltt_trace_struct *new_trace)
 {
+	struct dentry *ltt_root_dentry;
+
+	ltt_root_dentry = get_ltt_root();
+	if (!ltt_root_dentry)
+		return ENOENT;
+
 	new_trace->dentry.trace_root = debugfs_create_dir(new_trace->trace_name,
-			get_ltt_root());
+			ltt_root_dentry);
+	put_ltt_root();
+	ltt_root_dentry = NULL;
 	if (new_trace->dentry.trace_root == NULL) {
 		printk(KERN_ERR "LTT : Trace directory name %s already taken\n",
 				new_trace->trace_name);
diff --git a/ltt/ltt-userspace-event.c b/ltt/ltt-userspace-event.c
index a48f336..487516b 100644
--- a/ltt/ltt-userspace-event.c
+++ b/ltt/ltt-userspace-event.c
@@ -110,7 +110,10 @@ static int __init ltt_userspace_init(void)
 		err = -EPERM;
 		goto err_no_file;
 	}
+
+	return err;
 err_no_file:
+	put_ltt_root();
 err_no_root:
 	return err;
 }
-- 
1.5.4.rc3