Re: uprobe: checking probe event include directory

2012-11-19 Thread Steven Rostedt 
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
>  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 
> 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 
> Reviewed-by: Srikar Dronamraju 
> ---
>  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;
>  }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-11-19 Thread Steven Rostedt 
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
 sri...@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 bookj...@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 bookj...@gmail.com
 Reviewed-by: Srikar Dronamraju sri...@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;
  }


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-10-11 Thread Jovi Zhang 
On Wed, Jul 18, 2012 at 7:45 PM, Srikar Dronamraju
 wrote:
> * Jovi Zhang  [2012-07-18 19:38:27]:
>
>> On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju
>>  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 
>> 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 
>> Reviewed-by: Srikar Dronamraju 
>> ---
>>  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 
>

Hi Andrew,
Is this patch ok to go through your tree?

.jovi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-10-11 Thread Jovi Zhang 
On Wed, Jul 18, 2012 at 7:45 PM, Srikar Dronamraju
sri...@linux.vnet.ibm.com wrote:
 * Jovi Zhang bookj...@gmail.com [2012-07-18 19:38:27]:

 On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju
 sri...@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 bookj...@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 bookj...@gmail.com
 Reviewed-by: Srikar Dronamraju sri...@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 sri...@linux.vnet.ibm.com


Hi Andrew,
Is this patch ok to go through your tree?

.jovi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-07-18 Thread Srikar Dronamraju 
* Jovi Zhang  [2012-07-18 19:38:27]:

> On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju
>  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 
> 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 
> Reviewed-by: Srikar Dronamraju 
> ---
>  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 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [offlist] uprobe: checking probe event include directory

2012-07-18 Thread Jovi Zhang 
On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju
 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 
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 
Reviewed-by: Srikar Dronamraju 
---
 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [offlist] uprobe: checking probe event include directory

2012-07-18 Thread Jovi Zhang 
On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju
sri...@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 bookj...@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 bookj...@gmail.com
Reviewed-by: Srikar Dronamraju sri...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-07-18 Thread Srikar Dronamraju 
* Jovi Zhang bookj...@gmail.com [2012-07-18 19:38:27]:

 On Wed, Jul 18, 2012 at 7:07 PM, Srikar Dronamraju
 sri...@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 bookj...@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 bookj...@gmail.com
 Reviewed-by: Srikar Dronamraju sri...@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 sri...@linux.vnet.ibm.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-07-17 Thread Jovi Zhang 
On Wed, Jul 18, 2012 at 1:27 AM, Srikar Dronamraju
 wrote:
> * Frederic Weisbecker  [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 
>> > 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 
>>
>> 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 
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 
Reviewed-by: Srikar Dronamraju 
---
 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-07-17 Thread Srikar Dronamraju 
* Frederic Weisbecker  [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 
> > 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 
> 
> 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
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-07-17 Thread Frederic Weisbecker 
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 
> 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 

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-07-17 Thread Frederic Weisbecker 
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 bookj...@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 bookj...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-07-17 Thread Srikar Dronamraju 
* Frederic Weisbecker fweis...@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 bookj...@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 bookj...@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
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: uprobe: checking probe event include directory

2012-07-17 Thread Jovi Zhang 
On Wed, Jul 18, 2012 at 1:27 AM, Srikar Dronamraju
sri...@linux.vnet.ibm.com wrote:
 * Frederic Weisbecker fweis...@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 bookj...@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 bookj...@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 bookj...@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 bookj...@gmail.com
Reviewed-by: Srikar Dronamraju sri...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/