tracing: Fix bad use of igrab in trace_uprobe.c
As Miklos reported and suggested: This pattern repeats two times in trace_uprobe.c and in kernel/events/core.c as well: ret = kern_path(filename, LOOKUP_FOLLOW, &path); if (ret) goto fail_address_parse; inode = igrab(d_inode(path.dentry)); path_put(&path); And it's wrong. You can only hold a reference to the inode if you have an active ref to the superblock as well (which is normally through path.mnt) or holding s_umount. This way unmounting the containing filesystem while the tracepoint is active will give you the "VFS: Busy inodes after unmount..." message and a crash when the inode is finally put. Solution: store path instead of inode. This patch fixes two instances in trace_uprobe.c. struct path is added to struct trace_uprobe to keep the inode and containing mount point referenced. Link: http://lkml.kernel.org/r/20180423172135.4050588-1-songliubraving@fb.com Fixes:f3f096cfed
("tracing: Provide trace events interface for uprobes") Fixes:33ea4b2427
("perf/core: Implement the 'perf_uprobe' PMU") Cc: stable@vger.kernel.org Cc: Ingo Molnar <mingo@redhat.com> Cc: Howard McLauchlan <hmclauchlan@fb.com> Cc: Josef Bacik <jbacik@fb.com> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com> Acked-by: Miklos Szeredi <mszeredi@redhat.com> Reported-by: Miklos Szeredi <miklos@szeredi.hu> Signed-off-by: Song Liu <songliubraving@fb.com> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
This commit is contained in:
parent
9a0fd67530
commit
0c92c7a3c5
|
@ -55,6 +55,7 @@ struct trace_uprobe {
|
||||||
struct list_head list;
|
struct list_head list;
|
||||||
struct trace_uprobe_filter filter;
|
struct trace_uprobe_filter filter;
|
||||||
struct uprobe_consumer consumer;
|
struct uprobe_consumer consumer;
|
||||||
|
struct path path;
|
||||||
struct inode *inode;
|
struct inode *inode;
|
||||||
char *filename;
|
char *filename;
|
||||||
unsigned long offset;
|
unsigned long offset;
|
||||||
|
@ -289,7 +290,7 @@ static void free_trace_uprobe(struct trace_uprobe *tu)
|
||||||
for (i = 0; i < tu->tp.nr_args; i++)
|
for (i = 0; i < tu->tp.nr_args; i++)
|
||||||
traceprobe_free_probe_arg(&tu->tp.args[i]);
|
traceprobe_free_probe_arg(&tu->tp.args[i]);
|
||||||
|
|
||||||
iput(tu->inode);
|
path_put(&tu->path);
|
||||||
kfree(tu->tp.call.class->system);
|
kfree(tu->tp.call.class->system);
|
||||||
kfree(tu->tp.call.name);
|
kfree(tu->tp.call.name);
|
||||||
kfree(tu->filename);
|
kfree(tu->filename);
|
||||||
|
@ -363,7 +364,6 @@ end:
|
||||||
static int create_trace_uprobe(int argc, char **argv)
|
static int create_trace_uprobe(int argc, char **argv)
|
||||||
{
|
{
|
||||||
struct trace_uprobe *tu;
|
struct trace_uprobe *tu;
|
||||||
struct inode *inode;
|
|
||||||
char *arg, *event, *group, *filename;
|
char *arg, *event, *group, *filename;
|
||||||
char buf[MAX_EVENT_NAME_LEN];
|
char buf[MAX_EVENT_NAME_LEN];
|
||||||
struct path path;
|
struct path path;
|
||||||
|
@ -371,7 +371,6 @@ static int create_trace_uprobe(int argc, char **argv)
|
||||||
bool is_delete, is_return;
|
bool is_delete, is_return;
|
||||||
int i, ret;
|
int i, ret;
|
||||||
|
|
||||||
inode = NULL;
|
|
||||||
ret = 0;
|
ret = 0;
|
||||||
is_delete = false;
|
is_delete = false;
|
||||||
is_return = false;
|
is_return = false;
|
||||||
|
@ -437,21 +436,16 @@ static int create_trace_uprobe(int argc, char **argv)
|
||||||
}
|
}
|
||||||
/* Find the last occurrence, in case the path contains ':' too. */
|
/* Find the last occurrence, in case the path contains ':' too. */
|
||||||
arg = strrchr(argv[1], ':');
|
arg = strrchr(argv[1], ':');
|
||||||
if (!arg) {
|
if (!arg)
|
||||||
ret = -EINVAL;
|
return -EINVAL;
|
||||||
goto fail_address_parse;
|
|
||||||
}
|
|
||||||
|
|
||||||
*arg++ = '\0';
|
*arg++ = '\0';
|
||||||
filename = argv[1];
|
filename = argv[1];
|
||||||
ret = kern_path(filename, LOOKUP_FOLLOW, &path);
|
ret = kern_path(filename, LOOKUP_FOLLOW, &path);
|
||||||
if (ret)
|
if (ret)
|
||||||
goto fail_address_parse;
|
return ret;
|
||||||
|
|
||||||
inode = igrab(d_real_inode(path.dentry));
|
if (!d_is_reg(path.dentry)) {
|
||||||
path_put(&path);
|
|
||||||
|
|
||||||
if (!inode || !S_ISREG(inode->i_mode)) {
|
|
||||||
ret = -EINVAL;
|
ret = -EINVAL;
|
||||||
goto fail_address_parse;
|
goto fail_address_parse;
|
||||||
}
|
}
|
||||||
|
@ -490,7 +484,7 @@ static int create_trace_uprobe(int argc, char **argv)
|
||||||
goto fail_address_parse;
|
goto fail_address_parse;
|
||||||
}
|
}
|
||||||
tu->offset = offset;
|
tu->offset = offset;
|
||||||
tu->inode = inode;
|
tu->path = path;
|
||||||
tu->filename = kstrdup(filename, GFP_KERNEL);
|
tu->filename = kstrdup(filename, GFP_KERNEL);
|
||||||
|
|
||||||
if (!tu->filename) {
|
if (!tu->filename) {
|
||||||
|
@ -558,7 +552,7 @@ error:
|
||||||
return ret;
|
return ret;
|
||||||
|
|
||||||
fail_address_parse:
|
fail_address_parse:
|
||||||
iput(inode);
|
path_put(&path);
|
||||||
|
|
||||||
pr_info("Failed to parse address or file.\n");
|
pr_info("Failed to parse address or file.\n");
|
||||||
|
|
||||||
|
@ -922,6 +916,7 @@ probe_event_enable(struct trace_uprobe *tu, struct trace_event_file *file,
|
||||||
goto err_flags;
|
goto err_flags;
|
||||||
|
|
||||||
tu->consumer.filter = filter;
|
tu->consumer.filter = filter;
|
||||||
|
tu->inode = d_real_inode(tu->path.dentry);
|
||||||
ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
|
ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
|
||||||
if (ret)
|
if (ret)
|
||||||
goto err_buffer;
|
goto err_buffer;
|
||||||
|
@ -967,6 +962,7 @@ probe_event_disable(struct trace_uprobe *tu, struct trace_event_file *file)
|
||||||
WARN_ON(!uprobe_filter_is_empty(&tu->filter));
|
WARN_ON(!uprobe_filter_is_empty(&tu->filter));
|
||||||
|
|
||||||
uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
|
uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
|
||||||
|
tu->inode = NULL;
|
||||||
tu->tp.flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;
|
tu->tp.flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;
|
||||||
|
|
||||||
uprobe_buffer_disable();
|
uprobe_buffer_disable();
|
||||||
|
@ -1337,7 +1333,6 @@ struct trace_event_call *
|
||||||
create_local_trace_uprobe(char *name, unsigned long offs, bool is_return)
|
create_local_trace_uprobe(char *name, unsigned long offs, bool is_return)
|
||||||
{
|
{
|
||||||
struct trace_uprobe *tu;
|
struct trace_uprobe *tu;
|
||||||
struct inode *inode;
|
|
||||||
struct path path;
|
struct path path;
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
|
@ -1345,11 +1340,8 @@ create_local_trace_uprobe(char *name, unsigned long offs, bool is_return)
|
||||||
if (ret)
|
if (ret)
|
||||||
return ERR_PTR(ret);
|
return ERR_PTR(ret);
|
||||||
|
|
||||||
inode = igrab(d_inode(path.dentry));
|
if (!d_is_reg(path.dentry)) {
|
||||||
path_put(&path);
|
path_put(&path);
|
||||||
|
|
||||||
if (!inode || !S_ISREG(inode->i_mode)) {
|
|
||||||
iput(inode);
|
|
||||||
return ERR_PTR(-EINVAL);
|
return ERR_PTR(-EINVAL);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1364,11 +1356,12 @@ create_local_trace_uprobe(char *name, unsigned long offs, bool is_return)
|
||||||
if (IS_ERR(tu)) {
|
if (IS_ERR(tu)) {
|
||||||
pr_info("Failed to allocate trace_uprobe.(%d)\n",
|
pr_info("Failed to allocate trace_uprobe.(%d)\n",
|
||||||
(int)PTR_ERR(tu));
|
(int)PTR_ERR(tu));
|
||||||
|
path_put(&path);
|
||||||
return ERR_CAST(tu);
|
return ERR_CAST(tu);
|
||||||
}
|
}
|
||||||
|
|
||||||
tu->offset = offs;
|
tu->offset = offs;
|
||||||
tu->inode = inode;
|
tu->path = path;
|
||||||
tu->filename = kstrdup(name, GFP_KERNEL);
|
tu->filename = kstrdup(name, GFP_KERNEL);
|
||||||
init_trace_event_call(tu, &tu->tp.call);
|
init_trace_event_call(tu, &tu->tp.call);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue