From mboxrd@z Thu Jan 1 00:00:00 1970 From: guijianfeng@cn.fujitsu.com (Gui Jianfeng) Date: Mon, 23 Feb 2009 17:27:39 +0800 Subject: [ltt-dev] [PATCH v2] lttng: Fix potential invalid pointer derefference In-Reply-To: <49A237E5.4010001@cn.fujitsu.com> References: <49A237E5.4010001@cn.fujitsu.com> Message-ID: <49A26C0B.3010005@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 | 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 #include #include +#include /* 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(<t_root_dentry->d_subdirs)) { - debugfs_remove(ltt_root_dentry); - ltt_root_dentry = NULL; + if (ltt_root_dentry) { + if (kref_put(<t_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(<t_root_kref); + goto out; } + kref_get(<t_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