Skip to contentSkip to conversation

perf probe: convert_name_to_addr() allocated the wrong size buffer for a function name

PATCH In review Hyeoncheol Lee 3 participants 5 messages Oct 16, 2012 perf events

lore
Share & tools
lore

Hyeoncheol Lee

author
#0 lore raw

PATCH 1/1 perf probe: convert_name_to_addr() allocated the wrong size buffer for a function name

convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS bytes for a function name

Cc Masami Hiramatsu <[email protected]>
Cc Srikar Dronamraju <[email protected]>
Signed-off-by Hyeoncheol Lee <[email protected]>
---
 tools/perf/util/probe-event.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
tools/perf/util/probe-event.c +3 -2
index 49a256e..bb40ed4 100644
@@ -2352,13 +2352,14 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
 		free(exec_copy);
 	}
 	free(pp->function);
-	pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
+	pp->function = zalloc(sizeof(char) *
+			      (3 + sizeof(unsigned long long) * 2));
 	if (!pp->function) {
 		ret = -ENOMEM;
 		pr_warning("Failed to allocate memory by zalloc.\n");
 		goto out;
 	}
-	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
+	sprintf(pp->function, "0x%llx", vaddr);
 	ret = 0;
 
 out:
Signature · 1 line
1.7.10.4
#1 lore raw
Quoted 2 lines · Hyeoncheol Lee

convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS bytes for a function name

Yeah, that one was from my laziness...

Quoted 13 lines
Cc Masami Hiramatsu <[email protected]>
Cc Srikar Dronamraju <[email protected]>
Signed-off-by Hyeoncheol Lee <[email protected]>
---
 tools/perf/util/probe-event.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
tools/perf/util/probe-event.c +0 -0
index 49a256e..bb40ed4 100644

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2356

Quoted 6 lines
tools/perf/util/probe-event.c +2 -1
@@ -2352,13 +2352,14 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
 		free(exec_copy);
 	}
 	free(pp->function);
-	pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
+	pp->function = zalloc(sizeof(char) *
+			      (3 + sizeof(unsigned long long) * 2));

Could you comment that this is enough long here?

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2362

Quoted 7 lines
tools/perf/util/probe-event.c +1 -1
 	if (!pp->function) {
 		ret = -ENOMEM;
 		pr_warning("Failed to allocate memory by zalloc.\n");
 		goto out;
 	}
-	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
+	sprintf(pp->function, "0x%llx", vaddr);

And at least we should use snprintf instead of sprintf... (I think ret = e_snprintf(...) is better)

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2365

Quoted 4 lines
tools/perf/util/probe-event.c +0 -0
 	ret = 0;
 
 out:

Thank you,

Signature · 4 lines
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#2 lore raw
Quoted 6 lines · Masami Hiramatsu
Quoted 2 lines · Hyeoncheol Lee

convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS bytes for a function name

Yeah, that one was from my laziness...

Guess not your fault, but mine.

Quoted 8 lines
Quoted 8 lines
Cc Masami Hiramatsu <[email protected]>
Cc Srikar Dronamraju <[email protected]>
Signed-off-by Hyeoncheol Lee <[email protected]>
---
 tools/perf/util/probe-event.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 13 lines

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2356

Quoted 11 lines
tools/perf/util/probe-event.c +2 -1
index 49a256e..bb40ed4 100644
@@ -2352,13 +2352,14 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
 		free(exec_copy);
 	}
 	free(pp->function);
-	pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
+	pp->function = zalloc(sizeof(char) *
+			      (3 + sizeof(unsigned long long) * 2));

Could you comment that this is enough long here?

Also can we move the arith into a macro?

Quoted 12 lines

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2362

Quoted 8 lines
tools/perf/util/probe-event.c +1 -1
 	if (!pp->function) {
 		ret = -ENOMEM;
 		pr_warning("Failed to allocate memory by zalloc.\n");
 		goto out;
 	}
-	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
+	sprintf(pp->function, "0x%llx", vaddr);

And at least we should use snprintf instead of sprintf... (I think ret = e_snprintf(...) is better)

Agree.

Quoted 5 lines
Quoted 4 lines
tools/perf/util/probe-event.c +0 -0
 	ret = 0;
 
 out:
Signature · 2 lines
Thanks and Regards
Srikar
author
#3 lore raw

Hi,

2012/10/16 Srikar Dronamraju <[email protected]>:

Quoted 33 lines · Masami Hiramatsu
Quoted 7 lines
Quoted 2 lines · Hyeoncheol Lee

convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS bytes for a function name

Yeah, that one was from my laziness...

Guess not your fault, but mine.

Quoted 8 lines
Quoted 8 lines
Cc Masami Hiramatsu <[email protected]>
Cc Srikar Dronamraju <[email protected]>
Signed-off-by Hyeoncheol Lee <[email protected]>
---
 tools/perf/util/probe-event.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 13 lines

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2356

Quoted 11 lines
tools/perf/util/probe-event.c +2 -1
index 49a256e..bb40ed4 100644
@@ -2352,13 +2352,14 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
            free(exec_copy);
    }
    free(pp->function);
-   pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
+   pp->function = zalloc(sizeof(char) *
+                         (3 + sizeof(unsigned long long) * 2));

Could you comment that this is enough long here?

Quoted 2 lines

Also can we move the arith into a macro?

I will do.

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
tools/perf/util/probe-event.c +1 -1
    if (!pp->function) {
            ret = -ENOMEM;
            pr_warning("Failed to allocate memory by zalloc.\n");
            goto out;
    }
-   e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
+   sprintf(pp->function, "0x%llx", vaddr);
Quoted 5 lines
Quoted 3 lines

And at least we should use snprintf instead of sprintf... (I think ret = e_snprintf(...) is better)

Agree.

Yes

Quoted 11 lines
Quoted 6 lines

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2365

Quoted 5 lines
tools/perf/util/probe-event.c +0 -0
    ret = 0;
 
 out:

--
Thanks and Regards
Srikar

Thank you for your comment.

author
#4 lore raw
In reply to Masami Hiramatsu

Hi,

2012/10/16 Masami Hiramatsu <[email protected]>:

Quoted 14 lines
Quoted 2 lines · Hyeoncheol Lee

convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS bytes for a function name

Yeah, that one was from my laziness...

Quoted 8 lines
Cc Masami Hiramatsu <[email protected]>
Cc Srikar Dronamraju <[email protected]>
Signed-off-by Hyeoncheol Lee <[email protected]>
---
 tools/perf/util/probe-event.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Quoted 14 lines

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2356

Quoted 11 lines
tools/perf/util/probe-event.c +2 -1
index 49a256e..bb40ed4 100644
@@ -2352,13 +2352,14 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
              free(exec_copy);
      }
      free(pp->function);
-     pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
+     pp->function = zalloc(sizeof(char) *
+                           (3 + sizeof(unsigned long long) * 2));

Could you comment that this is enough long here?

Because a hexadecimal address that starts with "0x" will be stored in pp->function. sizeof(unsigned long long) * 2 is maximum length of hexadecimal number of variable "vaddr" and 3 bytes are for "0x" and null character.

Quoted 11 lines

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2362

Quoted 7 lines
tools/perf/util/probe-event.c +1 -1
      if (!pp->function) {
              ret = -ENOMEM;
              pr_warning("Failed to allocate memory by zalloc.\n");
              goto out;
      }
-     e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
+     sprintf(pp->function, "0x%llx", vaddr);

And at least we should use snprintf instead of sprintf... (I think ret = e_snprintf(...) is better)

You are right, but I didn't want to write down the length of "pp->function" again.

Quoted 5 lines

tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2365

Quoted 4 lines
tools/perf/util/probe-event.c +0 -0
      ret = 0;
 
 out:
Quoted 9 lines

Thank you,

--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

Thank you very much!