* uprobe: checking probe event include directory @ 2012-07-17 10:12 Jovi Zhang 2012-07-17 10:59 ` Frederic Weisbecker 0 siblings, 1 reply; 8+ messages in thread From: Jovi Zhang @ 2012-07-17 10:12 UTC (permalink / raw) To: Steven Rostedt, Frédéric Weisbecker, Ingo Molnar, LKML >From 16ed13ee9098ae01705e8456005d1ad6d9909128 Mon Sep 17 00:00:00 2001 From: Jovi Zhang <bookjovi@gmail.com> Date: Wed, 18 Jul 2012 01:16:23 +0800 Subject: [PATCH] uprobe: checking probe event include directory Currently below command run successful: $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events this don't make sense, because /bin is a directory, so make this checking earily, not report error untill probe enable. Signed-off-by: Jovi Zhang <bookjovi@gmail.com> --- kernel/trace/trace_uprobe.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index 85158fa..cf382de 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -259,6 +259,11 @@ static int create_trace_uprobe(int argc, char **argv) goto fail_address_parse; inode = igrab(path.dentry->d_inode); + if (S_ISDIR(inode->i_mode)) { + ret = -EINVAL; + pr_info("probe file cannot be directory.\n"); + goto fail_address_parse; + } argc -= 2; argv += 2; -- 1.7.9.7 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: uprobe: checking probe event include directory 2012-07-17 10:12 uprobe: checking probe event include directory Jovi Zhang @ 2012-07-17 10:59 ` Frederic Weisbecker 2012-07-17 17:27 ` Srikar Dronamraju 0 siblings, 1 reply; 8+ messages in thread From: Frederic Weisbecker @ 2012-07-17 10:59 UTC (permalink / raw) To: Jovi Zhang, Srikar Dronamraju; +Cc: Steven Rostedt, Ingo Molnar, LKML On Tue, Jul 17, 2012 at 06:12:28PM +0800, Jovi Zhang wrote: > From 16ed13ee9098ae01705e8456005d1ad6d9909128 Mon Sep 17 00:00:00 2001 > From: Jovi Zhang <bookjovi@gmail.com> > Date: Wed, 18 Jul 2012 01:16:23 +0800 > Subject: [PATCH] uprobe: checking probe event include directory > > Currently below command run successful: > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > this don't make sense, because /bin is a directory, > so make this checking earily, not report error untill probe enable. > > Signed-off-by: Jovi Zhang <bookjovi@gmail.com> Adding Srikar in Cc. > --- > kernel/trace/trace_uprobe.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > index 85158fa..cf382de 100644 > --- a/kernel/trace/trace_uprobe.c > +++ b/kernel/trace/trace_uprobe.c > @@ -259,6 +259,11 @@ static int create_trace_uprobe(int argc, char **argv) > goto fail_address_parse; > > inode = igrab(path.dentry->d_inode); > + if (S_ISDIR(inode->i_mode)) { > + ret = -EINVAL; > + pr_info("probe file cannot be directory.\n"); > + goto fail_address_parse; > + } > > argc -= 2; > argv += 2; > -- > 1.7.9.7 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: uprobe: checking probe event include directory 2012-07-17 10:59 ` Frederic Weisbecker @ 2012-07-17 17:27 ` Srikar Dronamraju 2012-07-18 2:36 ` Jovi Zhang 0 siblings, 1 reply; 8+ messages in thread From: Srikar Dronamraju @ 2012-07-17 17:27 UTC (permalink / raw) To: Frederic Weisbecker, Jovi Zhang; +Cc: Steven Rostedt, Ingo Molnar, LKML * Frederic Weisbecker <fweisbec@gmail.com> [2012-07-17 12:59:39]: > On Tue, Jul 17, 2012 at 06:12:28PM +0800, Jovi Zhang wrote: > > From 16ed13ee9098ae01705e8456005d1ad6d9909128 Mon Sep 17 00:00:00 2001 > > From: Jovi Zhang <bookjovi@gmail.com> > > Date: Wed, 18 Jul 2012 01:16:23 +0800 > > Subject: [PATCH] uprobe: checking probe event include directory > > > > Currently below command run successful: > > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events good catch. > > > > this don't make sense, because /bin is a directory, > > so make this checking earily, not report error untill probe enable. > > > > Signed-off-by: Jovi Zhang <bookjovi@gmail.com> > > Adding Srikar in Cc. > > > --- > > kernel/trace/trace_uprobe.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > > index 85158fa..cf382de 100644 > > --- a/kernel/trace/trace_uprobe.c > > +++ b/kernel/trace/trace_uprobe.c > > @@ -259,6 +259,11 @@ static int create_trace_uprobe(int argc, char **argv) > > goto fail_address_parse; > > > > inode = igrab(path.dentry->d_inode); > > + if (S_ISDIR(inode->i_mode)) { How about checking for regular files but not directory. i.e it should avoid tracing special files Something like: if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { > > + ret = -EINVAL; > > + pr_info("probe file cannot be directory.\n"); we could drop the pr_info line here, since we would any print we failed to parse the address. Probably you could change the last pr_info from pr_info("Failed to parse address.\n"); to pr_info("Failed to parse address or file.\n"); -- Thanks and Regards Srikar > > + goto fail_address_parse; > > + } > > > > argc -= 2; > > argv += 2; > > -- > > 1.7.9.7 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: uprobe: checking probe event include directory 2012-07-17 17:27 ` Srikar Dronamraju @ 2012-07-18 2:36 ` Jovi Zhang [not found] ` <20120718110731.GE5006@linux.vnet.ibm.com> 0 siblings, 1 reply; 8+ messages in thread From: Jovi Zhang @ 2012-07-18 2:36 UTC (permalink / raw) To: Srikar Dronamraju; +Cc: Frederic Weisbecker, Steven Rostedt, Ingo Molnar, LKML On Wed, Jul 18, 2012 at 1:27 AM, Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote: > * Frederic Weisbecker <fweisbec@gmail.com> [2012-07-17 12:59:39]: > >> On Tue, Jul 17, 2012 at 06:12:28PM +0800, Jovi Zhang wrote: >> > From 16ed13ee9098ae01705e8456005d1ad6d9909128 Mon Sep 17 00:00:00 2001 >> > From: Jovi Zhang <bookjovi@gmail.com> >> > Date: Wed, 18 Jul 2012 01:16:23 +0800 >> > Subject: [PATCH] uprobe: checking probe event include directory >> > >> > Currently below command run successful: >> > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > good catch. > >> > >> > this don't make sense, because /bin is a directory, >> > so make this checking earily, not report error untill probe enable. >> > >> > Signed-off-by: Jovi Zhang <bookjovi@gmail.com> >> >> Adding Srikar in Cc. >> >> > --- >> > kernel/trace/trace_uprobe.c | 5 +++++ >> > 1 file changed, 5 insertions(+) >> > >> > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c >> > index 85158fa..cf382de 100644 >> > --- a/kernel/trace/trace_uprobe.c >> > +++ b/kernel/trace/trace_uprobe.c >> > @@ -259,6 +259,11 @@ static int create_trace_uprobe(int argc, char **argv) >> > goto fail_address_parse; >> > >> > inode = igrab(path.dentry->d_inode); >> > + if (S_ISDIR(inode->i_mode)) { > > How about checking for regular files but not directory. > i.e it should avoid tracing special files > > Something like: > > if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { > >> > + ret = -EINVAL; >> > + pr_info("probe file cannot be directory.\n"); > > we could drop the pr_info line here, since we would any print we > failed to parse the address. > > Probably you could change the last pr_info from > > pr_info("Failed to parse address.\n"); > > to > > pr_info("Failed to parse address or file.\n"); > > Thanks srikar, try below patch: >From dd37d3cc563e2619ec325d6c11d769a32def411b Mon Sep 17 00:00:00 2001 From: Jovi Zhang <bookjovi@gmail.com> Date: Wed, 18 Jul 2012 18:16:44 +0800 Subject: [PATCH] uprobe: checking uprobe event inode valid Currently below command run successful: $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events this don't make sense, because /bin is a directory, we need to check uprobe event inode valid more earily. Signed-off-by: Jovi Zhang <bookjovi@gmail.com> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> --- kernel/trace/trace_uprobe.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index 85158fa..3b5f646 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -259,6 +259,10 @@ static int create_trace_uprobe(int argc, char **argv) goto fail_address_parse; inode = igrab(path.dentry->d_inode); + if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { + ret = -EINVAL; + goto fail_address_parse; + } argc -= 2; argv += 2; @@ -358,7 +362,7 @@ fail_address_parse: if (inode) iput(inode); - pr_info("Failed to parse address.\n"); + pr_info("Failed to parse address or file.\n"); return ret; } -- 1.7.9.7 ^ permalink raw reply related [flat|nested] 8+ messages in thread
[parent not found: <20120718110731.GE5006@linux.vnet.ibm.com>]
* Re: [offlist] uprobe: checking probe event include directory [not found] ` <20120718110731.GE5006@linux.vnet.ibm.com> @ 2012-07-18 11:38 ` Jovi Zhang 2012-07-18 11:45 ` Srikar Dronamraju 2012-11-19 9:40 ` Steven Rostedt 0 siblings, 2 replies; 8+ messages in thread From: Jovi Zhang @ 2012-07-18 11:38 UTC (permalink / raw) To: Srikar Dronamraju, Frédéric Weisbecker, Steven Rostedt, LKML On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote: > The patch looks good, > > Can you modify the description a bit. However you are free to ignore > these comments. After knowing your response, I will ack the patch. > > I would probably put this as: > > The subject could be > tracing: Verify target file before registering a uprobe event > > Description: > Without this patch, we can register a uprobe event for a directory. > Enabling such a uprobe event would anyway fail. > > Example: > > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > However directories cannot be valid targets for uprobe. > Hence verify if the target is a regular file during the probe > registration. Thanks srikar, your description is more clear than mine. >From fd5077196038cc271e2116e1fca359a0011e1669 Mon Sep 17 00:00:00 2001 From: Jovi Zhang <bookjovi@gmail.com> Date: Wed, 18 Jul 2012 18:16:44 +0800 Subject: [PATCH] tracing: Verify target file before registering a uprobe event Without this patch, we can register a uprobe event for a directory. Enabling such a uprobe event would anyway fail. Example: $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events However dirctories cannot be valid targets for uprobe. Hence verify if the target is a regular file during the probe registration. Signed-off-by: Jovi Zhang <bookjovi@gmail.com> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> --- kernel/trace/trace_uprobe.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index 85158fa..3b5f646 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -259,6 +259,10 @@ static int create_trace_uprobe(int argc, char **argv) goto fail_address_parse; inode = igrab(path.dentry->d_inode); + if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { + ret = -EINVAL; + goto fail_address_parse; + } argc -= 2; argv += 2; @@ -358,7 +362,7 @@ fail_address_parse: if (inode) iput(inode); - pr_info("Failed to parse address.\n"); + pr_info("Failed to parse address or file.\n"); return ret; } -- 1.7.9.7 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: uprobe: checking probe event include directory 2012-07-18 11:38 ` [offlist] " Jovi Zhang @ 2012-07-18 11:45 ` Srikar Dronamraju 2012-10-12 0:20 ` Jovi Zhang 2012-11-19 9:40 ` Steven Rostedt 1 sibling, 1 reply; 8+ messages in thread From: Srikar Dronamraju @ 2012-07-18 11:45 UTC (permalink / raw) To: Jovi Zhang; +Cc: Fr?d?ric Weisbecker, Steven Rostedt, LKML * Jovi Zhang <bookjovi@gmail.com> [2012-07-18 19:38:27]: > On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju > <srikar@linux.vnet.ibm.com> wrote: > > The patch looks good, > > > > Can you modify the description a bit. However you are free to ignore > > these comments. After knowing your response, I will ack the patch. > > > > I would probably put this as: > > > > The subject could be > > tracing: Verify target file before registering a uprobe event > > > > Description: > > Without this patch, we can register a uprobe event for a directory. > > Enabling such a uprobe event would anyway fail. > > > > Example: > > > > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > > > However directories cannot be valid targets for uprobe. > > Hence verify if the target is a regular file during the probe > > registration. > > Thanks srikar, your description is more clear than mine. > > > From fd5077196038cc271e2116e1fca359a0011e1669 Mon Sep 17 00:00:00 2001 > From: Jovi Zhang <bookjovi@gmail.com> > Date: Wed, 18 Jul 2012 18:16:44 +0800 > Subject: [PATCH] tracing: Verify target file before registering a uprobe > event > > Without this patch, we can register a uprobe event for a directory. > Enabling such a uprobe event would anyway fail. > > Example: > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > However dirctories cannot be valid targets for uprobe. > Hence verify if the target is a regular file during the probe > registration. > > Signed-off-by: Jovi Zhang <bookjovi@gmail.com> > Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > --- > kernel/trace/trace_uprobe.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > index 85158fa..3b5f646 100644 > --- a/kernel/trace/trace_uprobe.c > +++ b/kernel/trace/trace_uprobe.c > @@ -259,6 +259,10 @@ static int create_trace_uprobe(int argc, char **argv) > goto fail_address_parse; > > inode = igrab(path.dentry->d_inode); > + if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { > + ret = -EINVAL; > + goto fail_address_parse; > + } > > argc -= 2; > argv += 2; > @@ -358,7 +362,7 @@ fail_address_parse: > if (inode) > iput(inode); > > - pr_info("Failed to parse address.\n"); > + pr_info("Failed to parse address or file.\n"); > > return ret; > } Looks good. Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: uprobe: checking probe event include directory 2012-07-18 11:45 ` Srikar Dronamraju @ 2012-10-12 0:20 ` Jovi Zhang 0 siblings, 0 replies; 8+ messages in thread From: Jovi Zhang @ 2012-10-12 0:20 UTC (permalink / raw) To: Andrew Morton; +Cc: Srikar Dronamraju, Steven Rostedt, LKML On Wed, Jul 18, 2012 at 7:45 PM, Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote: > * Jovi Zhang <bookjovi@gmail.com> [2012-07-18 19:38:27]: > >> On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju >> <srikar@linux.vnet.ibm.com> wrote: >> > The patch looks good, >> > >> > Can you modify the description a bit. However you are free to ignore >> > these comments. After knowing your response, I will ack the patch. >> > >> > I would probably put this as: >> > >> > The subject could be >> > tracing: Verify target file before registering a uprobe event >> > >> > Description: >> > Without this patch, we can register a uprobe event for a directory. >> > Enabling such a uprobe event would anyway fail. >> > >> > Example: >> > >> > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events >> > >> > However directories cannot be valid targets for uprobe. >> > Hence verify if the target is a regular file during the probe >> > registration. >> >> Thanks srikar, your description is more clear than mine. >> >> >> From fd5077196038cc271e2116e1fca359a0011e1669 Mon Sep 17 00:00:00 2001 >> From: Jovi Zhang <bookjovi@gmail.com> >> Date: Wed, 18 Jul 2012 18:16:44 +0800 >> Subject: [PATCH] tracing: Verify target file before registering a uprobe >> event >> >> Without this patch, we can register a uprobe event for a directory. >> Enabling such a uprobe event would anyway fail. >> >> Example: >> $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events >> >> However dirctories cannot be valid targets for uprobe. >> Hence verify if the target is a regular file during the probe >> registration. >> >> Signed-off-by: Jovi Zhang <bookjovi@gmail.com> >> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> >> --- >> kernel/trace/trace_uprobe.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c >> index 85158fa..3b5f646 100644 >> --- a/kernel/trace/trace_uprobe.c >> +++ b/kernel/trace/trace_uprobe.c >> @@ -259,6 +259,10 @@ static int create_trace_uprobe(int argc, char **argv) >> goto fail_address_parse; >> >> inode = igrab(path.dentry->d_inode); >> + if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { >> + ret = -EINVAL; >> + goto fail_address_parse; >> + } >> >> argc -= 2; >> argv += 2; >> @@ -358,7 +362,7 @@ fail_address_parse: >> if (inode) >> iput(inode); >> >> - pr_info("Failed to parse address.\n"); >> + pr_info("Failed to parse address or file.\n"); >> >> return ret; >> } > > Looks good. > > Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > Hi Andrew, Is this patch ok to go through your tree? .jovi ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: uprobe: checking probe event include directory 2012-07-18 11:38 ` [offlist] " Jovi Zhang 2012-07-18 11:45 ` Srikar Dronamraju @ 2012-11-19 9:40 ` Steven Rostedt 1 sibling, 0 replies; 8+ messages in thread From: Steven Rostedt @ 2012-11-19 9:40 UTC (permalink / raw) To: Jovi Zhang; +Cc: Srikar Dronamraju, Frédéric Weisbecker, LKML I've been cleaning out my inbox and found this patch hasn't made it into mainline yet. There were a couple of versions of the patch in this thread. Please re-submit the final version with the associated acks. And not within this thread. Patches that are added to replies in threads seldom get applied. -- Steve On Wed, 2012-07-18 at 19:38 +0800, Jovi Zhang wrote: > On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju > <srikar@linux.vnet.ibm.com> wrote: > > The patch looks good, > > > > Can you modify the description a bit. However you are free to ignore > > these comments. After knowing your response, I will ack the patch. > > > > I would probably put this as: > > > > The subject could be > > tracing: Verify target file before registering a uprobe event > > > > Description: > > Without this patch, we can register a uprobe event for a directory. > > Enabling such a uprobe event would anyway fail. > > > > Example: > > > > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > > > However directories cannot be valid targets for uprobe. > > Hence verify if the target is a regular file during the probe > > registration. > > Thanks srikar, your description is more clear than mine. > > > >From fd5077196038cc271e2116e1fca359a0011e1669 Mon Sep 17 00:00:00 2001 > From: Jovi Zhang <bookjovi@gmail.com> > Date: Wed, 18 Jul 2012 18:16:44 +0800 > Subject: [PATCH] tracing: Verify target file before registering a uprobe > event > > Without this patch, we can register a uprobe event for a directory. > Enabling such a uprobe event would anyway fail. > > Example: > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > However dirctories cannot be valid targets for uprobe. > Hence verify if the target is a regular file during the probe > registration. > > Signed-off-by: Jovi Zhang <bookjovi@gmail.com> > Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > --- > kernel/trace/trace_uprobe.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > index 85158fa..3b5f646 100644 > --- a/kernel/trace/trace_uprobe.c > +++ b/kernel/trace/trace_uprobe.c > @@ -259,6 +259,10 @@ static int create_trace_uprobe(int argc, char **argv) > goto fail_address_parse; > > inode = igrab(path.dentry->d_inode); > + if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { > + ret = -EINVAL; > + goto fail_address_parse; > + } > > argc -= 2; > argv += 2; > @@ -358,7 +362,7 @@ fail_address_parse: > if (inode) > iput(inode); > > - pr_info("Failed to parse address.\n"); > + pr_info("Failed to parse address or file.\n"); > > return ret; > } ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-11-19 9:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-17 10:12 uprobe: checking probe event include directory Jovi Zhang
2012-07-17 10:59 ` Frederic Weisbecker
2012-07-17 17:27 ` Srikar Dronamraju
2012-07-18 2:36 ` Jovi Zhang
[not found] ` <20120718110731.GE5006@linux.vnet.ibm.com>
2012-07-18 11:38 ` [offlist] " Jovi Zhang
2012-07-18 11:45 ` Srikar Dronamraju
2012-10-12 0:20 ` Jovi Zhang
2012-11-19 9:40 ` Steven Rostedt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox