From 9f0fe8ee09003ba0dc526b69749d0a27902ce6f4 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Mon, 8 May 2023 18:38:10 -0700 Subject: lib/fs: fix file leak in task_get_name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the problem identified -fanalyzer. Why did rdma choose to reimplement the same function as exiting glibc pthread_getname(). fs.c: In function ‘get_task_name’: fs.c:355:12: warning: leak of FILE ‘f’ [CWE-775] [-Wanalyzer-file-leak] 355 | if (!fgets(name, len, f)) | ^ ‘get_task_name’: events 1-9 | | 345 | if (!pid) | | ^ | | | | | (1) following ‘false’ branch (when ‘pid != 0’)... |...... | 348 | if (snprintf(path, sizeof(path), "/proc/%d/comm", pid) >= sizeof(path)) | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | || | | |(2) ...to here | | (3) following ‘false’ branch... |...... | 351 | f = fopen(path, "r"); | | ~~~~~~~~~~~~~~~~ | | | | | (4) ...to here | | (5) opened here | 352 | if (!f) | | ~ | | | | | (6) assuming ‘f’ is non-NULL | | (7) following ‘false’ branch (when ‘f’ is non-NULL)... |...... | 355 | if (!fgets(name, len, f)) | | ~ ~~~~~~~~~~~~~~~~~~~ | | | | | | | (8) ...to here | | (9) following ‘true’ branch... | ‘get_task_name’: event 10 | |cc1: | (10): ...to here | ‘get_task_name’: event 11 | | 355 | if (!fgets(name, len, f)) | | ^ | | | | | (11) ‘f’ leaks here; was opened at (5) | fs.c:355:12: warning: leak of ‘f’ [CWE-401] [-Wanalyzer-malloc-leak] ‘get_task_name’: events 1-9 | | 345 | if (!pid) | | ^ | | | | | (1) following ‘false’ branch (when ‘pid != 0’)... |...... | 348 | if (snprintf(path, sizeof(path), "/proc/%d/comm", pid) >= sizeof(path)) | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | || | | |(2) ...to here | | (3) following ‘false’ branch... |...... | 351 | f = fopen(path, "r"); | | ~~~~~~~~~~~~~~~~ | | | | | (4) ...to here | | (5) allocated here | 352 | if (!f) | | ~ | | | | | (6) assuming ‘f’ is non-NULL | | (7) following ‘false’ branch (when ‘f’ is non-NULL)... |...... | 355 | if (!fgets(name, len, f)) | | ~ ~~~~~~~~~~~~~~~~~~~ | | | | | | | (8) ...to here | | (9) following ‘true’ branch... | ‘get_task_name’: event 10 | |cc1: | (10): ...to here | ‘get_task_name’: event 11 | | 355 | if (!fgets(name, len, f)) | | ^ | | | | | (11) ‘f’ leaks here; was allocated at (5) Fixes: 81bfd01a4c9e ("lib: move get_task_name() from rdma") Signed-off-by: Stephen Hemminger --- lib/fs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/fs.c b/lib/fs.c index 22d4af75..7f4b159c 100644 --- a/lib/fs.c +++ b/lib/fs.c @@ -352,8 +352,10 @@ int get_task_name(pid_t pid, char *name, size_t len) if (!f) return -1; - if (!fgets(name, len, f)) + if (!fgets(name, len, f)) { + fclose(f); return -1; + } /* comm ends in \n, get rid of it */ name[strcspn(name, "\n")] = '\0'; -- cgit v1.2.1