perf parse-events: Use asprintf() instead of strncpy() for tracepoints

PATCH Applied Arnaldo Carvalho de Melo 3 participants 4 messages Mar 2, 2020 – Mar 4, 2020 perf events

lore

Share & tools

lore

Arnaldo Carvalho de Melo

author

2020-03-02 14:55 UTC #0 lore raw

PATCH 1/1 perf parse-events: Use asprintf() instead of strncpy() for tracepoints

Hi,

Noticed this with gcc 10 on fedora rawhide:

In file included from /usr/include/string.h:495,
from util/parse-events.h:12,
from util/parse-events.c:18:
In function ‘strncpy’,
inlined from ‘tracepoint_id_to_path’ at util/parse-events.c:271:5:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ offset [275, 511] from the object at ‘sys_dirent’ is out of the bounds of referenced subobject ‘d_name’ with type ‘char[256]’ at offset 19 [-Werror=array-bounds]

  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In file included from /usr/include/dirent.h:61,
from util/parse-events.c:5:
util/parse-events.c: In function ‘tracepoint_id_to_path’:
/usr/include/bits/dirent.h:33:10: note: subobject ‘d_name’ declared here
33 | char d_name[256]; /* We must not include limits.h! */

      |          ^~~~~~

In file included from /usr/include/string.h:495,
from util/parse-events.h:12,
from util/parse-events.c:18:
In function ‘strncpy’,
inlined from ‘tracepoint_id_to_path’ at util/parse-events.c:273:5:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ offset [275, 511] from the object at ‘evt_dirent’ is out of the bounds of referenced subobject ‘d_name’ with type ‘char[256]’ at offset 19 [-Werror=array-bounds]

  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

      |          ^~~~~~

CC /tmp/build/perf/util/call-path.o

So I replaced it with asprintf to make the code shorter, use a bit less memory and deal with the above problem, ok?

- Arnaldo

tools/perf/util/parse-events.c +2 -8

index c01ba6f8fdad..a14995835d85 100644
@@ -257,21 +257,15 @@ struct tracepoint_path *tracepoint_id_to_path(u64 config)
 				path = zalloc(sizeof(*path));
 				if (!path)
 					return NULL;
-				path->system = malloc(MAX_EVENT_LENGTH);
-				if (!path->system) {
+				if (asprintf(&path->system, "%.*s", MAX_EVENT_LENGTH, sys_dirent->d_name) < 0) {
 					free(path);
 					return NULL;
 				}
-				path->name = malloc(MAX_EVENT_LENGTH);
-				if (!path->name) {
+				if (asprintf(&path->name, "%.*s", MAX_EVENT_LENGTH, evt_dirent->d_name) < 0) {
 					zfree(&path->system);
 					free(path);
 					return NULL;
 				}
-				strncpy(path->system, sys_dirent->d_name,
-					MAX_EVENT_LENGTH);
-				strncpy(path->name, evt_dirent->d_name,
-					MAX_EVENT_LENGTH);
 				return path;
 			}
 		}

Jiri Olsa

2020-03-02 15:27 UTC #1 lore raw

On Mon, Mar 02, 2020 at 11:55:35AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

Quoted 23 lines

      |          ^~~~~~

CC /tmp/build/perf/util/call-path.o

So I replaced it with asprintf to make the code shorter, use a bit less memory and deal with the above problem, ok?

- Arnaldo

tools/perf/util/parse-events.c +1 -3

index c01ba6f8fdad..a14995835d85 100644
@@ -257,21 +257,15 @@ struct tracepoint_path *tracepoint_id_to_path(u64 config)
 				path = zalloc(sizeof(*path));
 				if (!path)
 					return NULL;
-				path->system = malloc(MAX_EVENT_LENGTH);
-				if (!path->system) {
+				if (asprintf(&path->system, "%.*s", MAX_EVENT_LENGTH, sys_dirent->d_name) < 0) {
 					free(path);
 					return NULL;
 				}
-				path->name = malloc(MAX_EVENT_LENGTH);

tools/perf/util/parse-events.c struct tracepoint_path *tracepoint_id_to_path(u64 config) · line 274

Quoted 10 lines

tools/perf/util/parse-events.c +1 -5

-				if (!path->name) {
+				if (asprintf(&path->name, "%.*s", MAX_EVENT_LENGTH, evt_dirent->d_name) < 0) {
 					zfree(&path->system);
 					free(path);
 					return NULL;
 				}
-				strncpy(path->system, sys_dirent->d_name,
-					MAX_EVENT_LENGTH);
-				strncpy(path->name, evt_dirent->d_name,
-					MAX_EVENT_LENGTH);

looks good to me, and we can probably remove MAX_EVENT_LENGTH as well?

jirka

Arnaldo Carvalho de Melo

author

2020-03-02 18:27 UTC #2 lore raw

Em Mon, Mar 02, 2020 at 04:27:41PM +0100, Jiri Olsa escreveu:

Quoted 31 lines · Arnaldo Carvalho de Melo

SNIP

Quoted 27 lines

      |          ^~~~~~

CC /tmp/build/perf/util/call-path.o

So I replaced it with asprintf to make the code shorter, use a bit less memory and deal with the above problem, ok?

- Arnaldo

tools/perf/util/parse-events.c +2 -4

index c01ba6f8fdad..a14995835d85 100644
@@ -257,21 +257,15 @@ struct tracepoint_path *tracepoint_id_to_path(u64 config)
 				path = zalloc(sizeof(*path));
 				if (!path)
 					return NULL;
-				path->system = malloc(MAX_EVENT_LENGTH);
-				if (!path->system) {
+				if (asprintf(&path->system, "%.*s", MAX_EVENT_LENGTH, sys_dirent->d_name) < 0) {
 					free(path);
 					return NULL;
 				}
-				path->name = malloc(MAX_EVENT_LENGTH);
-				if (!path->name) {
+				if (asprintf(&path->name, "%.*s", MAX_EVENT_LENGTH, evt_dirent->d_name) < 0) {
 					zfree(&path->system);
 					free(path);

Quoted 8 lines

tools/perf/util/parse-events.c struct tracepoint_path *tracepoint_id_to_path(u64 config) · line 274
Quoted 6 lines
tools/perf/util/parse-events.c +0 -4
 					return NULL;
 				}
-				strncpy(path->system, sys_dirent->d_name,
-					MAX_EVENT_LENGTH);
-				strncpy(path->name, evt_dirent->d_name,
-					MAX_EVENT_LENGTH);
looks good to me, and we can probably remove MAX_EVENT_LENGTH as well?

I left it there, we can remove it if the need arises, this code is not that exercised from what I could look.

- Arnaldo

tip-bot2 for Arnaldo Carvalho de Melo

merge bot Applied

2020-03-04 11:02 UTC #3 lore raw

[tip: perf/urgent] perf parse-events: Use asprintf() instead of strncpy() to read tracepoint files

In reply to Arnaldo Carvalho de Melo

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     7125f204501ed55493593209c6c71ac7c38f6b6c
Gitweb:        https://git.kernel.org/tip/7125f204501ed55493593209c6c71ac7c38f6b6c
Author:        Arnaldo Carvalho de Melo <[email protected]>
AuthorDate:    Mon, 02 Mar 2020 11:55:47 -03:00
Committer:     Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Mon, 02 Mar 2020 11:55:47 -03:00

perf parse-events: Use asprintf() instead of strncpy() to read tracepoint files

Make the code more compact by using asprintf() instead of malloc()+strncpy() which also uses less memory and avoids these warnings with gcc 10:

CC /tmp/build/perf/util/cloexec.o
In file included from /usr/include/string.h:495,
from util/parse-events.h:12,
from util/parse-events.c:18:
In function ‘strncpy’,
inlined from ‘tracepoint_id_to_path’ at util/parse-events.c:271:5:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ offset [275, 511] from the object at ‘sys_dirent’ is out of the bounds of referenced subobject ‘d_name’ with type ‘char[256]’ at offset 19 [-Werror=array-bounds]

    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  In file included from /usr/include/dirent.h:61,
                   from util/parse-events.c:5:
  util/parse-events.c: In function ‘tracepoint_id_to_path’:
  /usr/include/bits/dirent.h:33:10: note: subobject ‘d_name’ declared here
     33 |     char d_name[256];  /* We must not include limits.h! */
        |          ^~~~~~
  In file included from /usr/include/string.h:495,
                   from util/parse-events.h:12,
                   from util/parse-events.c:18:
  In function ‘strncpy’,
      inlined from ‘tracepoint_id_to_path’ at util/parse-events.c:273:5:
  /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ offset [275, 511] from the object at ‘evt_dirent’ is out of the bounds of referenced subobject ‘d_name’ with type ‘char[256]’ at offset 19 [-Werror=array-bounds]
    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  In file included from /usr/include/dirent.h:61,
                   from util/parse-events.c:5:
  util/parse-events.c: In function ‘tracepoint_id_to_path’:
  /usr/include/bits/dirent.h:33:10: note: subobject ‘d_name’ declared here
     33 |     char d_name[256];  /* We must not include limits.h! */
        |          ^~~~~~
    CC       /tmp/build/perf/util/call-path.o

Cc Adrian Hunter <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Link http://lore.kernel.org/lkml/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/parse-events.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

tools/perf/util/parse-events.c +2 -8

index c01ba6f..a149958 100644
@@ -257,21 +257,15 @@ struct tracepoint_path *tracepoint_id_to_path(u64 config)
 				path = zalloc(sizeof(*path));
 				if (!path)
 					return NULL;
-				path->system = malloc(MAX_EVENT_LENGTH);
-				if (!path->system) {
+				if (asprintf(&path->system, "%.*s", MAX_EVENT_LENGTH, sys_dirent->d_name) < 0) {
 					free(path);
 					return NULL;
 				}
-				path->name = malloc(MAX_EVENT_LENGTH);
-				if (!path->name) {
+				if (asprintf(&path->name, "%.*s", MAX_EVENT_LENGTH, evt_dirent->d_name) < 0) {
 					zfree(&path->system);
 					free(path);
 					return NULL;
 				}
-				strncpy(path->system, sys_dirent->d_name,
-					MAX_EVENT_LENGTH);
-				strncpy(path->name, evt_dirent->d_name,
-					MAX_EVENT_LENGTH);
 				return path;
 			}
 		}