[PATCH 4/4] perf tools: Add dso__data_get/put_fd()

[Namhyung Kim]

Using dso__data_fd() in multi-thread environment is not safe since
returned fd can be closed and/or reused anytime.  So convert it to the
dso__data_get/put_fd() pair to protect the access with lock.

The original dso__data_fd() is deprecated and kept only for testing.

Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
---
 tools/perf/util/dso.c              | 44 +++++++++++++++++++++++++++++---------
 tools/perf/util/dso.h              |  9 ++++++--
 tools/perf/util/unwind-libunwind.c | 38 +++++++++++++++++++-------------
 3 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 21fae6908717..5227e41925c2 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -471,27 +471,49 @@ static void try_to_open_dso(struct dso *dso, struct machine *machine)
 }
 
 /**
- * dso__data_fd - Get dso's data file descriptor
+ * dso__data_get_fd - Get dso's data file descriptor
  * @dso: dso object
  * @machine: machine object
  *
  * External interface to find dso's file, open it and
- * returns file descriptor.
+ * returns file descriptor.  Should be paired with
+ * dso__data_put_fd().
  */
-int dso__data_fd(struct dso *dso, struct machine *machine)
+int dso__data_get_fd(struct dso *dso, struct machine *machine)
 {
+	pthread_mutex_lock(&dso__data_open_lock);
+
 	if (dso->data.status == DSO_DATA_STATUS_ERROR)
 		return -1;
 
-	pthread_mutex_lock(&dso__data_open_lock);
-
 	if (dso->data.fd < 0)
 		try_to_open_dso(dso, machine);
 
-	pthread_mutex_unlock(&dso__data_open_lock);
 	return dso->data.fd;
 }
 
+void dso__data_put_fd(struct dso *dso __maybe_unused)
+{
+	pthread_mutex_unlock(&dso__data_open_lock);
+}
+
+/**
+ * dso__data_get_fd - Get dso's data file descriptor
+ * @dso: dso object
+ * @machine: machine object
+ *
+ * Obsolete interface to find dso's file, open it and
+ * returns file descriptor.  It's not thread-safe in that
+ * the returned fd may be reused for other file.
+ */
+int dso__data_fd(struct dso *dso, struct machine *machine)
+{
+	int fd = dso__data_get_fd(dso, machine);
+
+	dso__data_put_fd(dso);
+	return fd;
+}
+
 bool dso__data_status_seen(struct dso *dso, enum dso_data_status_seen by)
 {
 	u32 flag = 1 << by;
@@ -1198,12 +1220,14 @@ size_t dso__fprintf(struct dso *dso, enum map_type type, FILE *fp)
 enum dso_type dso__type(struct dso *dso, struct machine *machine)
 {
 	int fd;
+	enum dso_type type = DSO__TYPE_UNKNOWN;
 
-	fd = dso__data_fd(dso, machine);
-	if (fd < 0)
-		return DSO__TYPE_UNKNOWN;
+	fd = dso__data_get_fd(dso, machine);
+	if (fd >= 0)
+		type = dso__type_fd(fd);
+	dso__data_put_fd(dso);
 
-	return dso__type_fd(fd);
+	return type;
 }
 
 int dso__strerror_load(struct dso *dso, char *buf, size_t buflen)
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index b26ec3ab1336..de9d98c44ae2 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -240,7 +240,9 @@ int __kmod_path__parse(struct kmod_path *m, const char *path,
 
 /*
  * The dso__data_* external interface provides following functions:
- *   dso__data_fd
+ *   dso__data_fd (obsolete)
+ *   dso__data_get_fd
+ *   dso__data_put_fd
  *   dso__data_close
  *   dso__data_size
  *   dso__data_read_offset
@@ -257,8 +259,9 @@ int __kmod_path__parse(struct kmod_path *m, const char *path,
  * The current usage of the dso__data_* interface is as follows:
  *
  * Get DSO's fd:
- *   int fd = dso__data_fd(dso, machine);
+ *   int fd = dso__data_get_fd(dso, machine);
  *   USE 'fd' SOMEHOW
+ *   dso__data_put_fd(dso)
  *
  * Read DSO's data:
  *   n = dso__data_read_offset(dso_0, &machine, 0, buf, BUFSIZE);
@@ -278,6 +281,8 @@ int __kmod_path__parse(struct kmod_path *m, const char *path,
  * TODO
 */
 int dso__data_fd(struct dso *dso, struct machine *machine);
+int dso__data_get_fd(struct dso *dso, struct machine *machine);
+void dso__data_put_fd(struct dso *dso);
 void dso__data_close(struct dso *dso);
 
 off_t dso__data_size(struct dso *dso, struct machine *machine);
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 7b09a443a280..6d4ab3251729 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -269,13 +269,13 @@ static int read_unwind_spec_eh_frame(struct dso *dso, struct machine *machine,
 	u64 offset = dso->data.eh_frame_hdr_offset;
 
 	if (offset == 0) {
-		fd = dso__data_fd(dso, machine);
-		if (fd < 0)
-			return -EINVAL;
-
-		/* Check the .eh_frame section for unwinding info */
-		offset = elf_section_offset(fd, ".eh_frame_hdr");
-		dso->data.eh_frame_hdr_offset = offset;
+		fd = dso__data_get_fd(dso, machine);
+		if (fd >= 0) {
+			/* Check the .eh_frame section for unwinding info */
+			offset = elf_section_offset(fd, ".eh_frame_hdr");
+			dso->data.eh_frame_hdr_offset = offset;
+		}
+		dso__data_put_fd(dso);
 	}
 
 	if (offset)
@@ -294,13 +294,19 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
 	u64 ofs = dso->data.debug_frame_offset;
 
 	if (ofs == 0) {
-		fd = dso__data_fd(dso, machine);
-		if (fd < 0)
-			return -EINVAL;
-
-		/* Check the .debug_frame section for unwinding info */
-		ofs = elf_section_offset(fd, ".debug_frame");
-		dso->data.debug_frame_offset = ofs;
+		int ret = 0;
+
+		fd = dso__data_get_fd(dso, machine);
+		if (fd >= 0) {
+			/* Check the .debug_frame section for unwinding info */
+			ofs = elf_section_offset(fd, ".debug_frame");
+			dso->data.debug_frame_offset = ofs;
+		} else
+			ret = -EINVAL;
+
+		dso__data_put_fd(dso);
+		if (ret)
+			return ret;
 	}
 
 	*offset = ofs;
@@ -353,10 +359,12 @@ find_proc_info(unw_addr_space_t as, unw_word_t ip, unw_proc_info_t *pi,
 #ifndef NO_LIBUNWIND_DEBUG_FRAME
 	/* Check the .debug_frame section for unwinding info */
 	if (!read_unwind_spec_debug_frame(map->dso, ui->machine, &segbase)) {
-		int fd = dso__data_fd(map->dso, ui->machine);
+		int fd = dso__data_get_fd(map->dso, ui->machine);
 		int is_exec = elf_is_exec(fd, map->dso->name);
 		unw_word_t base = is_exec ? 0 : map->start;
 
+		dso__data_put_fd(map->dso);
+
 		memset(&di, 0, sizeof(di));
 		if (dwarf_find_debug_frame(0, &di, ip, base, map->dso->name,
 					   map->start, map->end))
-- 
2.4.0

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