From mboxrd@z Thu Jan 1 00:00:00 1970 From: guijianfeng@cn.fujitsu.com (Gui Jianfeng) Date: Mon, 23 Feb 2009 13:45:09 +0800 Subject: [ltt-dev] [PATCH] lttng: Fix potential invalid pointer derefference Message-ID: <49A237E5.4010001@cn.fujitsu.com> 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 --- 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 #include #include #include +#include /* 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 #include #include +#include #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 #include #include +#include #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