[PATCH] libtracefs: Fix memory leak in tracefs_tracing_dir_is_mounted()

From: Bean Huo <[email protected]>

tfs_find_debug_tracing_dir() returns a newly allocated string on every
call, and the caller owns it -- tfs_find_tracing_dir() documents that the
result "must be freed by free()". But tracefs_tracing_dir_is_mounted()
passes that pointer straight out through @path, whose own documentation
says it "must not be freed".

The two contracts cannot both hold: the allocation is unreachable the
moment the function returns, and no caller is permitted to release it. The
result is a leak of the tracing directory path on every call, and two of
them when the directory was not mounted and @mount asked for it to be.

valgrind on an application that calls it once at startup:

  20 bytes in 1 blocks are definitely lost in loss record 2 of 3
     at 0x4848899: malloc
     by 0x4A2C58E: strdup (strdup.c:42)
     by 0x13014C: find_tracing_dir (tracefs-utils.c:155)
     by 0x1301A7: tracefs_tracing_dir_is_mounted (tracefs-utils.c:178)

The library already keeps exactly the cache this function needs:
tracefs_tracing_dir() memoises the path in a static, revalidates it with
test_dir(), and is documented as returning a string that must not be
freed. Use tfs_find_debug_tracing_dir() only to probe -- it is still needed
for that, because unlike tracefs_tracing_dir() it can be told not to mount,
which is what @mount == false requires -- then release the probe copy and
return the cached path.

Note this changes @path when a custom directory is in effect: it now
reports tracefs_set_tracing_dir()'s value rather than the detected mount
point. That makes @path agree with the directory the rest of the library
actually operates on, which seems the more useful answer, but it is a
visible change and not merely an internal cleanup. The return value is
unaffected -- it still comes from probing the real mount state.

Signed-off-by: Bean Huo <[email protected]>
---
 src/tracefs-utils.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/tracefs-utils.c b/src/tracefs-utils.c
index d1aced0..96a1fa9 100644
--- a/src/tracefs-utils.c
+++ b/src/tracefs-utils.c
@@ -173,12 +173,19 @@ __hidden char *tfs_find_debug_tracing_dir(bool debugfs, bool mount)
  */
 int tracefs_tracing_dir_is_mounted(bool mount, const char **path)
 {
-	const char *dir;
+	char *dir;
 
+	/*
+	 * tfs_find_debug_tracing_dir() allocates a new string on every call
+	 * and the caller owns it, but @path is documented as not freeable.
+	 * Use it only to probe (unlike tracefs_tracing_dir() it can be told
+	 * not to mount), then hand back the cached path the library owns.
+	 */
 	dir = tfs_find_debug_tracing_dir(false, false);
 	if (dir) {
+		free(dir);
 		if (path)
-			*path = dir;
+			*path = tracefs_tracing_dir();
 		return 1;
 	}
 	if (!mount)
@@ -187,8 +194,9 @@ int tracefs_tracing_dir_is_mounted(bool mount, const char **path)
 	dir = tfs_find_debug_tracing_dir(false, mount);
 	if (!dir)
 		return -1;
+	free(dir);
 	if (path)
-		*path = dir;
+		*path = tracefs_tracing_dir();
 	return 0;
 }
 
-- 
2.34.1