[ltt-dev] [PATCH] lttng: Fix potential invalid pointer derefference
Gui Jianfeng
guijianfeng at cn.fujitsu.com
Mon Feb 23 00:45:09 EST 2009
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 | 8 +++++++-
ltt/ltt-core.c | 38 +++++++++++++++++++++++++++-----------
ltt/ltt-relay.c | 7 ++++++-
ltt/ltt-trace-control.c | 13 +++++++------
ltt/ltt-userspace-event.c | 12 ++++++++----
5 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/include/linux/ltt-core.h b/include/linux/ltt-core.h
index 28394cb..4e20f78 100644
--- a/include/linux/ltt-core.h
+++ b/include/linux/ltt-core.h
@@ -27,11 +27,17 @@ struct ltt_traces {
extern struct ltt_traces ltt_traces;
+/* ltt's root dir */
+struct ltt_root {
+ atomic_t ref;
+ struct dentry *root;
+};
+
/*
* get dentry of ltt's root dir
*/
+struct ltt_root *get_ltt_root(void);
void put_ltt_root(void);
-struct dentry *get_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..ed393bd 100644
--- a/ltt/ltt-core.c
+++ b/ltt/ltt-core.c
@@ -6,10 +6,10 @@
* Distributed under the GPL license
*/
-#include <linux/ltt-core.h>
#include <linux/percpu.h>
#include <linux/module.h>
#include <linux/debugfs.h>
+#include <linux/ltt-core.h>
/* Traces structures */
struct ltt_traces ltt_traces = {
@@ -21,26 +21,42 @@ EXPORT_SYMBOL(ltt_traces);
/* Traces list writer locking */
static DEFINE_MUTEX(ltt_traces_mutex);
-/* dentry of ltt's root dir */
-static struct dentry *ltt_root_dentry;
+static struct ltt_root *ltt_root;
void put_ltt_root(void)
{
- if (ltt_root_dentry && list_empty(<t_root_dentry->d_subdirs)) {
- debugfs_remove(ltt_root_dentry);
- ltt_root_dentry = NULL;
+ if (ltt_root) {
+ if (atomic_dec_and_test(<t_root->ref)) {
+ debugfs_remove(ltt_root->root);
+ kfree(ltt_root);
+ ltt_root = NULL;
+ }
}
}
EXPORT_SYMBOL_GPL(put_ltt_root);
-struct dentry *get_ltt_root(void)
+struct ltt_root *get_ltt_root(void)
{
- if (!ltt_root_dentry) {
- ltt_root_dentry = debugfs_create_dir(LTT_ROOT, NULL);
- if (!ltt_root_dentry)
+ if (!ltt_root) {
+ ltt_root = kmalloc(sizeof(*ltt_root), GFP_KERNEL);
+ if (!ltt_root)
+ return NULL;
+ atomic_set(<t_root->ref, 1);
+ ltt_root->root = debugfs_create_dir(LTT_ROOT, NULL);
+ if (!ltt_root->root) {
printk(KERN_ERR "LTT : create ltt root dir failed\n");
+ goto create_dir_fail;
+ }
+ return ltt_root;
+ } else {
+ atomic_inc(<t_root->ref);
+ return ltt_root;
}
- return ltt_root_dentry;
+
+create_dir_fail:
+ kfree(ltt_root);
+ ltt_root = NULL;
+ return NULL;
}
EXPORT_SYMBOL_GPL(get_ltt_root);
diff --git a/ltt/ltt-relay.c b/ltt/ltt-relay.c
index 7c544f4..6e4e4e5 100644
--- a/ltt/ltt-relay.c
+++ b/ltt/ltt-relay.c
@@ -833,8 +833,13 @@ end:
static int ltt_relay_create_dirs(struct ltt_trace_struct *new_trace)
{
+ struct ltt_root *ltt_root;
+
+ ltt_root = get_ltt_root();
new_trace->dentry.trace_root = debugfs_create_dir(new_trace->trace_name,
- get_ltt_root());
+ ltt_root->root);
+ put_ltt_root();
+ ltt_root = 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-trace-control.c b/ltt/ltt-trace-control.c
index d7c9119..aa40e71 100644
--- a/ltt/ltt-trace-control.c
+++ b/ltt/ltt-trace-control.c
@@ -16,6 +16,7 @@
#include <linux/uaccess.h>
#include <linux/debugfs.h>
#include <linux/ltt-tracer.h>
+#include <linux/ltt-core.h>
#define LTT_CONTROL_DIR "control"
#define LTT_SETUP_TRACE_FILE "setup_trace"
@@ -696,15 +697,15 @@ static struct file_operations ltt_destroy_trace_operations = {
static int __init ltt_trace_control_init(void)
{
int err = 0;
- struct dentry *ltt_root_dentry;
+ struct ltt_root *ltt_root;
- ltt_root_dentry = get_ltt_root();
- if (!ltt_root_dentry) {
+ ltt_root = get_ltt_root();
+ if (!ltt_root) {
err = -ENOENT;
goto err_no_root;
}
- ltt_control_dir = debugfs_create_dir(LTT_CONTROL_DIR, ltt_root_dentry);
+ ltt_control_dir = debugfs_create_dir(LTT_CONTROL_DIR, ltt_root->root);
if (IS_ERR(ltt_control_dir) || !ltt_control_dir) {
printk(KERN_ERR
"ltt_channel_control_init: create dir of %s failed\n",
@@ -714,7 +715,7 @@ static int __init ltt_trace_control_init(void)
}
ltt_setup_trace_file = debugfs_create_file(LTT_SETUP_TRACE_FILE,
- S_IWUSR, ltt_root_dentry, NULL, <t_setup_trace_operations);
+ S_IWUSR, ltt_root->root, NULL, <t_setup_trace_operations);
if (IS_ERR(ltt_setup_trace_file) || !ltt_setup_trace_file) {
printk(KERN_ERR
"ltt_channel_control_init: create file of %s failed\n",
@@ -724,7 +725,7 @@ static int __init ltt_trace_control_init(void)
}
ltt_destroy_trace_file = debugfs_create_file(LTT_DESTROY_TRACE_FILE,
- S_IWUSR, ltt_root_dentry, NULL, <t_destroy_trace_operations);
+ S_IWUSR, ltt_root->root, NULL, <t_destroy_trace_operations);
if (IS_ERR(ltt_destroy_trace_file) || !ltt_destroy_trace_file) {
printk(KERN_ERR
"ltt_channel_control_init: create file of %s failed\n",
diff --git a/ltt/ltt-userspace-event.c b/ltt/ltt-userspace-event.c
index a48f336..5686022 100644
--- a/ltt/ltt-userspace-event.c
+++ b/ltt/ltt-userspace-event.c
@@ -22,6 +22,7 @@
#include <linux/gfp.h>
#include <linux/fs.h>
#include <linux/debugfs.h>
+#include <linux/ltt-core.h>
#include "probes/ltt-type-serializer.h"
@@ -89,18 +90,18 @@ static struct file_operations ltt_userspace_operations = {
static int __init ltt_userspace_init(void)
{
- struct dentry *ltt_root_dentry;
+ struct ltt_root *ltt_root;
int err = 0;
- ltt_root_dentry = get_ltt_root();
- if (!ltt_root_dentry) {
+ ltt_root = get_ltt_root();
+ if (!ltt_root) {
err = -ENOENT;
goto err_no_root;
}
ltt_event_file = debugfs_create_file(LTT_WRITE_EVENT_FILE,
S_IWUGO,
- ltt_root_dentry,
+ ltt_root->root,
NULL,
<t_userspace_operations);
if (IS_ERR(ltt_event_file) || !ltt_event_file) {
@@ -110,7 +111,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
More information about the lttng-dev
mailing list