[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250721002322.703-3-mailto.luca.kellermann@gmail.com>
Date: Mon, 21 Jul 2025 02:23:21 +0200
From: Luca Kellermann <mailto.luca.kellermann@...il.com>
To: Rich Felker <dalias@...c.org>
Cc: Markus Wichmann <nullplan@....net>,
musl@...ts.openwall.com
Subject: [PATCH v2 3/4] scandir: disable cancellation around cancellation points
opendir() or closedir() might act upon a cancellation request. because
scandir() did not disable cancellation or install a cancellation
cleanup handler, this could lead to memory and file descriptor leaks.
the following program demonstrates the leaks:
#include <dirent.h>
#include <errno.h>
#include <pthread.h>
#include <semaphore.h>
#include <stdio.h>
#include <stdlib.h>
static void die(const char *msg)
{
int cs, e = errno;
if (!pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs)) {
errno = e;
perror(msg);
}
_Exit(EXIT_FAILURE);
}
static int posted;
static sem_t sem;
static int sel(const struct dirent *e)
{
if (!posted && sem_post(&sem)) die("sem_post");
posted = 1;
return 1;
}
static void *run(void *arg)
{
posted = 0;
struct dirent **es = 0;
int n = scandir(".", &es, sel, alphasort);
if (n == -1) die("scandir");
while (n > 0) free(es[--n]);
free(es);
return 0;
}
int main(void)
{
if (sem_init(&sem, 0, 0)) die("sem_init");
void *res = 0;
do {
pthread_t tid;
if (errno = pthread_create(&tid, 0, run, 0))
die("pthread_create");
if (sem_wait(&sem)) die("sem_wait");
if (errno = pthread_cancel(tid))
die("pthread_cancel");
if (errno = pthread_join(tid, &res))
die("pthread_join");
} while (res != PTHREAD_CANCELED);
if (sem_destroy(&sem)) die("sem_destroy");
}
$ valgrind -q --leak-check=full --show-leak-kinds=all ./scandir-leak
==31293== 296 bytes in 9 blocks are indirectly lost in loss record 1 of 3
==31293== at 0x48AF828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==31293== by 0x401C500: scandir (scandir.c:32)
==31293== by 0x10933F: run (scandir-leak.c:28)
==31293== by 0x405E653: start (pthread_create.c:207)
==31293== by 0x4060DEA: ??? (clone.s:22)
==31293==
==31293== 416 (120 direct, 296 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 3
==31293== at 0x48B6B80: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==31293== by 0x401C4EA: scandir (scandir.c:28)
==31293== by 0x10933F: run (scandir-leak.c:28)
==31293== by 0x405E653: start (pthread_create.c:207)
==31293== by 0x4060DEA: ??? (clone.s:22)
==31293==
==31293== 2,072 bytes in 1 blocks are definitely lost in loss record 3 of 3
==31293== at 0x48B6953: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==31293== by 0x401C2E0: opendir (opendir.c:15)
==31293== by 0x401C491: scandir (scandir.c:12)
==31293== by 0x10933F: run (scandir-leak.c:28)
==31293== by 0x405E653: start (pthread_create.c:207)
==31293== by 0x4060DEA: ??? (clone.s:22)
==31293==
$ strace -fe t=open,close ./scandir-leak
strace: Process 27136 attached
[pid 27136] open(".", O_RDONLY|O_LARGEFILE|O_CLOEXEC|O_DIRECTORY) = 3
[pid 27136] --- SIGRT_1 {si_signo=SIGRT_1, si_code=SI_TKILL, si_pid=27135, si_uid=1000} ---
[pid 27136] +++ exited with 0 +++
+++ exited with 0 +++
---
src/dirent/scandir.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/dirent/scandir.c b/src/dirent/scandir.c
index cf668535..4f91af9c 100644
--- a/src/dirent/scandir.c
+++ b/src/dirent/scandir.c
@@ -1,4 +1,5 @@
#include <dirent.h>
+#include <pthread.h>
#include <string.h>
#include <stdlib.h>
#include <stdint.h>
@@ -9,11 +10,20 @@ int scandir(const char *path, struct dirent ***res,
int (*sel)(const struct dirent *),
int (*cmp)(const struct dirent **, const struct dirent **))
{
- DIR *d = opendir(path);
+ DIR *d;
struct dirent *de, **names=0, **tmp;
size_t cnt=0, len=0;
- int old_errno = errno, err;
+ int old_errno = errno, err, cs;
+ /* opendir() and closedir() are cancellation points. scandir() is also
+ * allowed to be a cancellation point but we choose not to make it one.
+ * To avoid calling sel() and cmp() with altered thread state,
+ * cancellation is not explicitly disabled for those calls. This means
+ * if either of the callbacks acts upon a cancellation request, there
+ * can be memory and file descriptor leaks. */
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
+ d = opendir(path);
+ pthread_setcancelstate(cs, 0);
if (!d) return -1;
while ((errno=0), (de = readdir(d))) {
@@ -39,7 +49,9 @@ int scandir(const char *path, struct dirent ***res,
* "failure" of closedir() as a failure of scandir(). */
err = errno;
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
closedir(d);
+ pthread_setcancelstate(cs, 0);
if (err) {
if (names) while (cnt-->0) free(names[cnt]);
--
2.43.0
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
