2018-03-15 08:28:12

by Ravi Bangoria

Subject: [PATCH v2 1/2] trace_uprobe: Use %lx to display offset

tu->offset is unsigned long, not a pointer, thus %lx should
be used to print it, not the %px.

Fixes: 0e4d819d0893 ("trace_uprobe: Display correct offset in uprobe_events")
Suggested-by: Kees Cook <[email protected]>
Signed-off-by: Ravi Bangoria <[email protected]>
---
v2 changelog:
- Keep prefixed 0s as is.

kernel/trace/trace_uprobe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 2014f4351ae0..0298bd15be83 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -608,7 +608,7 @@ static int probes_seq_show(struct seq_file *m, void *v)

/* Don't print "0x (null)" when offset is 0 */
if (tu->offset) {
- seq_printf(m, "0x%px", (void *)tu->offset);
+ seq_printf(m, "0x%0*lx", (int)(sizeof(void *) * 2), tu->offset);
} else {
switch (sizeof(void *)) {
case 4:
--
2.13.6

2018-03-15 08:26:59

by Ravi Bangoria

Subject: [PATCH v2 2/2] trace_uprobe: Simplify probes_seq_show()

Simplify probes_seq_show() function. No change in output
before and after patch.

Signed-off-by: Ravi Bangoria <[email protected]>
---
kernel/trace/trace_uprobe.c | 21 +++------------------
1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 0298bd15be83..544fa10a471b 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -602,24 +602,9 @@ static int probes_seq_show(struct seq_file *m, void *v)
char c = is_ret_probe(tu) ? 'r' : 'p';
int i;

- seq_printf(m, "%c:%s/%s", c, tu->tp.call.class->system,
- trace_event_name(&tu->tp.call));
- seq_printf(m, " %s:", tu->filename);
-
- /* Don't print "0x (null)" when offset is 0 */
- if (tu->offset) {
- seq_printf(m, "0x%0*lx", (int)(sizeof(void *) * 2), tu->offset);
- } else {
- switch (sizeof(void *)) {
- case 4:
- seq_printf(m, "0x00000000");
- break;
- case 8:
- default:
- seq_printf(m, "0x0000000000000000");
- break;
- }
- }
+ seq_printf(m, "%c:%s/%s %s:0x%0*lx", c, tu->tp.call.class->system,
+ trace_event_name(&tu->tp.call), tu->filename,
+ (int)(sizeof(void *) * 2), tu->offset);

for (i = 0; i < tu->tp.nr_args; i++)
seq_printf(m, " %s=%s", tu->tp.args[i].name, tu->tp.args[i].comm);
--
2.13.6

2018-03-15 09:30:27

by Masami Hiramatsu

Subject: Re: [PATCH v2 1/2] trace_uprobe: Use %lx to display offset

On Thu, 15 Mar 2018 13:57:55 +0530
Ravi Bangoria <[email protected]> wrote:

> tu->offset is unsigned long, not a pointer, thus %lx should
> be used to print it, not the %px.
>

This looks good to me.

Acked-by: Masami Hiramatsu <[email protected]>

Thanks,

> Fixes: 0e4d819d0893 ("trace_uprobe: Display correct offset in uprobe_events")
> Suggested-by: Kees Cook <[email protected]>
> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> v2 changelog:
> - Keep prefixed 0s as is.
>
> kernel/trace/trace_uprobe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 2014f4351ae0..0298bd15be83 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -608,7 +608,7 @@ static int probes_seq_show(struct seq_file *m, void *v)
>
> /* Don't print "0x (null)" when offset is 0 */
> if (tu->offset) {
> - seq_printf(m, "0x%px", (void *)tu->offset);
> + seq_printf(m, "0x%0*lx", (int)(sizeof(void *) * 2), tu->offset);
> } else {
> switch (sizeof(void *)) {
> case 4:
> --
> 2.13.6
>


--
Masami Hiramatsu <[email protected]>

2018-03-15 09:33:17

by Masami Hiramatsu

Subject: Re: [PATCH v2 2/2] trace_uprobe: Simplify probes_seq_show()

On Thu, 15 Mar 2018 13:57:56 +0530
Ravi Bangoria <[email protected]> wrote:

> Simplify probes_seq_show() function. No change in output
> before and after patch.

Yeah, we don't need this trick anymore.

Acked-by: Masami Hiramatsu <[email protected]>

Thank you!

>
> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> kernel/trace/trace_uprobe.c | 21 +++------------------
> 1 file changed, 3 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 0298bd15be83..544fa10a471b 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -602,24 +602,9 @@ static int probes_seq_show(struct seq_file *m, void *v)
> char c = is_ret_probe(tu) ? 'r' : 'p';
> int i;
>
> - seq_printf(m, "%c:%s/%s", c, tu->tp.call.class->system,
> - trace_event_name(&tu->tp.call));
> - seq_printf(m, " %s:", tu->filename);
> -
> - /* Don't print "0x (null)" when offset is 0 */
> - if (tu->offset) {
> - seq_printf(m, "0x%0*lx", (int)(sizeof(void *) * 2), tu->offset);
> - } else {
> - switch (sizeof(void *)) {
> - case 4:
> - seq_printf(m, "0x00000000");
> - break;
> - case 8:
> - default:
> - seq_printf(m, "0x0000000000000000");
> - break;
> - }
> - }
> + seq_printf(m, "%c:%s/%s %s:0x%0*lx", c, tu->tp.call.class->system,
> + trace_event_name(&tu->tp.call), tu->filename,
> + (int)(sizeof(void *) * 2), tu->offset);
>
> for (i = 0; i < tu->tp.nr_args; i++)
> seq_printf(m, " %s=%s", tu->tp.args[i].name, tu->tp.args[i].comm);
> --
> 2.13.6
>


--
Masami Hiramatsu <[email protected]>

2018-04-11 06:16:51

by Ravi Bangoria

Subject: Re: [PATCH v2 1/2] trace_uprobe: Use %lx to display offset

Hi Steve,

Can you please pull these patches.

Thanks,
Ravi

On 03/15/2018 01:57 PM, Ravi Bangoria wrote:
> tu->offset is unsigned long, not a pointer, thus %lx should
> be used to print it, not the %px.
>
> Fixes: 0e4d819d0893 ("trace_uprobe: Display correct offset in uprobe_events")
> Suggested-by: Kees Cook <[email protected]>
> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> v2 changelog:
> - Keep prefixed 0s as is.
>
> kernel/trace/trace_uprobe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 2014f4351ae0..0298bd15be83 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -608,7 +608,7 @@ static int probes_seq_show(struct seq_file *m, void *v)
>
> /* Don't print "0x (null)" when offset is 0 */
> if (tu->offset) {
> - seq_printf(m, "0x%px", (void *)tu->offset);
> + seq_printf(m, "0x%0*lx", (int)(sizeof(void *) * 2), tu->offset);
> } else {
> switch (sizeof(void *)) {
> case 4:

2018-04-11 12:49:52

by Steven Rostedt

Subject: Re: [PATCH v2 1/2] trace_uprobe: Use %lx to display offset

On Wed, 11 Apr 2018 11:42:51 +0530
Ravi Bangoria <[email protected]> wrote:

> Hi Steve,
>
> Can you please pull these patches.
>

Sure. I'll pull them in now and start testing them.

-- Steve