From 8e810e06fbbaaf70c71cd573b4822367b55e4e18 Mon Sep 17 00:00:00 2001 From: "Eunji, Lee" Date: Fri, 4 Mar 2016 16:56:04 +0900 Subject: [PATCH] fix fd handle leak in atrace.cpp and ttrace.c Change-Id: I1112a5f039cb147b8b64dc428f42e9e97523b2c5 Signed-off-by: Eunji, Lee --- packaging/ttrace_tag | 1 - src/atrace/atrace.cpp | 17 ++++++++++------- src/ttrace.c | 45 ++++++++++++++++++++++----------------------- 3 files changed, 32 insertions(+), 31 deletions(-) delete mode 100755 packaging/ttrace_tag diff --git a/packaging/ttrace_tag b/packaging/ttrace_tag deleted file mode 100755 index c227083..0000000 --- a/packaging/ttrace_tag +++ /dev/null @@ -1 +0,0 @@ -0 \ No newline at end of file diff --git a/src/atrace/atrace.cpp b/src/atrace/atrace.cpp index 2ac43cf..06de03f 100755 --- a/src/atrace/atrace.cpp +++ b/src/atrace/atrace.cpp @@ -74,13 +74,15 @@ struct CommonNode { typedef enum { TTRACE_TAG_IDX = 0, DEBUG_FS_IDX, + TRACING_FS_IDX, TRACE_MARKER_IDX, - ESSENCE_NODE_IDX + ESSENCE_NODE_IDX } commonNodeIdx; static const CommonNode commonNodes[] = { { ENABLED_TAG_FILE, 0664 }, { "/sys/kernel/debug", 0755 }, + { "/sys/kernel/debug/tracing", 0755 }, { "/sys/kernel/debug/tracing/trace_marker", 0222 }, { "/sys/kernel/debug/tracing/trace_clock", 0666 }, { "/sys/kernel/debug/tracing/buffer_size_kb", 0666 }, @@ -273,7 +275,7 @@ static bool fileIsWritable(const char* filename) { return access(filename, W_OK) != -1; } -static bool setFilePermisstion (const char *path, const mode_t perms) { +static bool setFilePermission (const char *path, const mode_t perms) { //fprintf(stderr, "path: %s, perms: %d, gid: %d\n", path,perms, group_dev.gr_gid); if (0 > chown(path, 0, group_dev.gr_gid)) return false; if (0 > chmod(path, perms)) return false; @@ -287,12 +289,12 @@ static bool initSysfsPermission() { const CommonNode &node = commonNodes[i]; printf("initsysfsperm: path- %s, perms- %d\n", node.path, node.perms); if (fileExists(node.path)) { - if (i == DEBUG_FS_IDX) { + if (i == DEBUG_FS_IDX || i == TRACING_FS_IDX) { if(0 > chmod(node.path, node.perms)) return false; } else { - if (!setFilePermisstion(node.path, node.perms)) + if (!setFilePermission(node.path, node.perms)) return false; } } @@ -503,16 +505,16 @@ static bool setTagsProperty(uint64_t tags) if(g_init_exec) { if(0 != getgrnam_r("developer", &group_dev, buf, sizeof(buf), &group_ptr)) return false; - + fd = open("/tmp/tmp_tag", O_CREAT | O_RDWR | O_CLOEXEC, 0666); if(fd < 0){ fprintf(stderr, "Fail to open enabled_tag file: %s(%d)\n", strerror_r(errno, str_error, sizeof(str_error)), errno); return false; } //set file permission, smack label to "/tmp/tmp_tag" and then change it's name to "/tmp/ttrace_tag" - if (!setFilePermisstion("/tmp/tmp_tag", tag_node.perms)) + if (!setFilePermission("/tmp/tmp_tag", tag_node.perms)) { - fprintf(stderr, "error: setFilePermisstion failed(%s): /tmp/tmp_tag\n", strerror_r(errno, str_error, sizeof(str_error))); + fprintf(stderr, "error: setFilePermission failed(%s): /tmp/tmp_tag\n", strerror_r(errno, str_error, sizeof(str_error))); close(fd); return false; } @@ -532,6 +534,7 @@ static bool setTagsProperty(uint64_t tags) if(!initSysfsPermission()) { fprintf(stderr, "Fail to init sysfs permisions: %s(%d)\n", strerror_r(errno, str_error, sizeof(str_error)), errno); + munmap(sm_for_enabled_tag, sizeof(uint64_t)); close(fd); return false; } diff --git a/src/ttrace.c b/src/ttrace.c index 7b62bb7..4fb4011 100755 --- a/src/ttrace.c +++ b/src/ttrace.c @@ -63,43 +63,42 @@ uint64_t *cur_enabled_tag = (void *)&dummy; static uint64_t traceInit() { uint64_t *sm_for_enabled_tag; + TTRACE_LOG("traceInit: %p %p", cur_enabled_tag, ((void *)&dummy)); if (cur_enabled_tag == ((void *)&dummy)) { g_enabled_tag_fd = open(ENABLED_TAG_FILE, O_RDONLY | O_CLOEXEC); if (g_enabled_tag_fd < 0) { TTRACE_LOG("Fail to open enabled_tag file: %s(%d)", strerror(errno), errno); - if (errno == ENOENT) - g_enabled_tag_fd = TRACE_FILE_NOT_EXIST; return 0; } - - /* access trace_marker after ensuring tag file creation */ - if(g_trace_handle_fd == FD_INITIAL_VALUE) { - g_trace_handle_fd = open(TRACE_FILE, O_WRONLY); - if (g_trace_handle_fd < 0) { - TTRACE_LOG("Fail to open trace file: %s(%d)", strerror(errno), errno); - /* in case ftrace debugfs is not mounted, ttrace does not call traceInit() anymore. */ - if (errno == ENOENT) - g_trace_handle_fd = TRACE_FILE_NOT_EXIST; - - set_last_result(TRACE_ERROR_IO_ERROR); - return 0; - } - } - sm_for_enabled_tag = mmap(NULL, sizeof(uint64_t), PROT_READ, MAP_SHARED, g_enabled_tag_fd, 0); if (sm_for_enabled_tag == MAP_FAILED) { TTRACE_LOG("error: mmap() failed(%s)\n", strerror(errno)); + close(g_enabled_tag_fd); return 0; } cur_enabled_tag = sm_for_enabled_tag; - TTRACE_LOG("cur_enabled_tag: %u %p", *cur_enabled_tag, cur_enabled_tag); + } - return *cur_enabled_tag; - } else { - TTRACE_LOG("traceInit: %u", *cur_enabled_tag); - return *cur_enabled_tag; + /* access trace_marker after ensuring tag file creation */ + if(g_trace_handle_fd == FD_INITIAL_VALUE) { + g_trace_handle_fd = open(TRACE_FILE, O_WRONLY); + if (g_trace_handle_fd < 0) { + TTRACE_LOG("Fail to open trace file: %s(%d)", strerror(errno), errno); + /* + * If ftrace debugfs is not mounted, ttrace does not call traceInit() anymore. + * we should decide how to handle if file permission is not given properly. keep try? or Nerver try agin? + */ + if (errno == ENOENT) + g_trace_handle_fd = TRACE_FILE_NOT_EXIST; + + set_last_result(TRACE_ERROR_IO_ERROR); + return 0; + } } + + TTRACE_LOG("traceInit:: cur_enabled_tag >> %u", *cur_enabled_tag); + return *cur_enabled_tag; } static inline uint64_t isTagEnabled(uint64_t cur_tag) @@ -109,7 +108,7 @@ static inline uint64_t isTagEnabled(uint64_t cur_tag) /* if no tag is enabled, trace all tags. */ cur_tag |= TTRACE_TAG_ALWAYS; - if (g_trace_handle_fd < 0 || cur_enabled_tag == ((void *)&dummy)) + if (g_trace_handle_fd == FD_INITIAL_VALUE || cur_enabled_tag == ((void *)&dummy)) return (cur_tag & traceInit()); return (cur_tag & *cur_enabled_tag); -- 2.7.4