* [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c
@ 2025-05-21 14:16 Wei Gao via ltp
2025-05-22 19:39 ` Petr Vorel
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Wei Gao via ltp @ 2025-05-21 14:16 UTC (permalink / raw)
To: ltp
Using sprintf without length checking in tst_tmpdir may lead to buffer overflow.
So in this patch use openat() instead of open().
Fixs:1241
Signed-off-by: Wei Gao <wegao@suse.com>
---
lib/tst_tmpdir.c | 119 +++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 111 insertions(+), 8 deletions(-)
diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index 6ed2367b9..7bd55022d 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -102,6 +102,7 @@ static char test_start_work_dir[PATH_MAX];
extern futex_t *tst_futexes;
static int rmobj(const char *obj, char **errmsg);
+static int rmobjat(int dir_fd, const char *obj, char **errmsg);
int tst_tmpdir_created(void)
{
@@ -149,8 +150,8 @@ static int purge_dir(const char *path, char **errptr)
int ret_val = 0;
DIR *dir;
struct dirent *dir_ent;
- char dirobj[PATH_MAX];
static char err_msg[PATH_MAX + 1280];
+ int dir_fd = -1;
/* Do NOT perform the request if the directory is "/" */
if (!strcmp(path, "/")) {
@@ -167,7 +168,7 @@ static int purge_dir(const char *path, char **errptr)
/* Open the directory to get access to what is in it */
if (!(dir = opendir(path))) {
if (errptr) {
- sprintf(err_msg,
+ snprintf(err_msg, sizeof(err_msg),
"Cannot open directory %s; errno=%d: %s",
path, errno, tst_strerrno(errno));
*errptr = err_msg;
@@ -175,6 +176,18 @@ static int purge_dir(const char *path, char **errptr)
return -1;
}
+ dir_fd = dirfd(dir);
+ if (dir_fd == -1) {
+ closedir(dir);
+ if (errptr) {
+ snprintf(err_msg, sizeof(err_msg),
+ "Cannot get file descriptor for directory %s; errno=%d: %s",
+ path, errno, tst_strerrno(errno));
+ *errptr = err_msg;
+ }
+ return -1;
+ }
+
/* Loop through the entries in the directory, removing each one */
for (dir_ent = readdir(dir); dir_ent; dir_ent = readdir(dir)) {
/* Don't remove "." or ".." */
@@ -183,8 +196,57 @@ static int purge_dir(const char *path, char **errptr)
continue;
/* Recursively remove the current entry */
- sprintf(dirobj, "%s/%s", path, dir_ent->d_name);
- if (rmobj(dirobj, errptr) != 0)
+ if (rmobjat(dir_fd, dir_ent->d_name, errptr) != 0)
+ ret_val = -1;
+ }
+
+ closedir(dir);
+ return ret_val;
+}
+
+static int purge_dirat(int dir_fd, const char *path, char **errptr)
+{
+ int ret_val = 0;
+ DIR *dir;
+ struct dirent *dir_ent;
+ static char err_msg[PATH_MAX + 1280];
+ int subdir_fd;
+
+ errno = 0;
+
+ /* Open the subdirectory using openat */
+ subdir_fd = openat(dir_fd, path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
+ if (subdir_fd < 0) {
+ if (errptr) {
+ snprintf(err_msg, sizeof(err_msg),
+ "Cannot open subdirectory %s (via fd %d); errno=%d: %s",
+ path, dir_fd, errno, tst_strerrno(errno));
+ *errptr = err_msg;
+ }
+ return -1;
+ }
+
+ dir = fdopendir(subdir_fd);
+ if (!dir) {
+ if (errptr) {
+ snprintf(err_msg, sizeof(err_msg),
+ "Cannot open directory stream for %s (via fd %d); errno=%d: %s",
+ path, dir_fd, errno, tst_strerrno(errno));
+ *errptr = err_msg;
+ }
+ close(subdir_fd);
+ return -1;
+ }
+
+ /* Loop through the entries in the directory, removing each one */
+ for (dir_ent = readdir(dir); dir_ent; dir_ent = readdir(dir)) {
+ /* Don't remove "." or ".." */
+ if (!strcmp(dir_ent->d_name, ".")
+ || !strcmp(dir_ent->d_name, ".."))
+ continue;
+
+ /* Recursively remove the current entry */
+ if (rmobjat(subdir_fd, dir_ent->d_name, errptr) != 0)
ret_val = -1;
}
@@ -212,7 +274,7 @@ static int rmobj(const char *obj, char **errmsg)
/* Get the link count, now that all the entries have been removed */
if (lstat(obj, &statbuf) < 0) {
if (errmsg != NULL) {
- sprintf(err_msg,
+ snprintf(err_msg, sizeof(err_msg),
"lstat(%s) failed; errno=%d: %s", obj,
errno, tst_strerrno(errno));
*errmsg = err_msg;
@@ -225,7 +287,7 @@ static int rmobj(const char *obj, char **errmsg)
/* The directory is linked; unlink() must be used */
if (unlink(obj) < 0) {
if (errmsg != NULL) {
- sprintf(err_msg,
+ snprintf(err_msg, sizeof(err_msg),
"unlink(%s) failed; errno=%d: %s",
obj, errno, tst_strerrno(errno));
*errmsg = err_msg;
@@ -236,7 +298,7 @@ static int rmobj(const char *obj, char **errmsg)
/* The directory is not linked; remove() can be used */
if (remove(obj) < 0) {
if (errmsg != NULL) {
- sprintf(err_msg,
+ snprintf(err_msg, sizeof(err_msg),
"remove(%s) failed; errno=%d: %s",
obj, errno, tst_strerrno(errno));
*errmsg = err_msg;
@@ -247,7 +309,7 @@ static int rmobj(const char *obj, char **errmsg)
} else {
if (unlink(obj) < 0) {
if (errmsg != NULL) {
- sprintf(err_msg,
+ snprintf(err_msg, sizeof(err_msg),
"unlink(%s) failed; errno=%d: %s", obj,
errno, tst_strerrno(errno));
*errmsg = err_msg;
@@ -259,6 +321,47 @@ static int rmobj(const char *obj, char **errmsg)
return 0;
}
+static int rmobjat(int dir_fd, const char *obj, char **errmsg)
+{
+ int ret_val = 0;
+ struct stat statbuf;
+ static char err_msg[PATH_MAX + 1280];
+ int fd;
+
+ fd = openat(dir_fd, obj, O_DIRECTORY | O_NOFOLLOW);
+ if (fd >= 0) {
+ close(fd);
+ ret_val = purge_dirat(dir_fd, obj, errmsg);
+
+ /* If there were problems removing an entry, don't attempt to
+ remove the directory itself */
+ if (ret_val == -1)
+ return -1;
+
+ if (unlinkat(dir_fd, obj, AT_REMOVEDIR) < 0) {
+ if (errmsg != NULL) {
+ snprintf(err_msg, sizeof(err_msg),
+ "remove(%s) failed; errno=%d: %s",
+ obj, errno, tst_strerrno(errno));
+ *errmsg = err_msg;
+ }
+ return -1;
+ }
+ } else {
+ if (unlinkat(dir_fd, obj, 0) < 0) {
+ if (errmsg != NULL) {
+ snprintf(err_msg, sizeof(err_msg),
+ "unlinkat(%s) failed; errno=%d: %s", obj,
+ errno, tst_strerrno(errno));
+ *errmsg = err_msg;
+ }
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
void tst_tmpdir(void)
{
char template[PATH_MAX];
--
2.49.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c
2025-05-21 14:16 [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c Wei Gao via ltp
@ 2025-05-22 19:39 ` Petr Vorel
2025-05-23 6:01 ` Jan Stancek via ltp
2025-05-23 21:09 ` [LTP] [PATCH v2] " Wei Gao via ltp
2025-09-01 15:08 ` [LTP] [PATCH v1] " Cyril Hrubis
2 siblings, 1 reply; 7+ messages in thread
From: Petr Vorel @ 2025-05-22 19:39 UTC (permalink / raw)
To: Wei Gao; +Cc: ltp
Hi Wei, all,
@all: is this a candidate for a release? It'd be nice to get it fixed.
> Using sprintf without length checking in tst_tmpdir may lead to buffer overflow.
> So in this patch use openat() instead of open().
LGTM, but it'd be nice if we could use only rmobjat().
Could you please remove the unused variable?
tst_tmpdir.c: In function ‘rmobjat’:
tst_tmpdir.c:327:21: warning: unused variable ‘statbuf’ [-Wunused-variable]
327 | struct stat statbuf;
| ^~~~~~~
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> Fixs:1241
This is better as it shows link in GitHub web:
Fixes: #1241
Or, IMHO better
Fixes: https://github.com/linux-test-project/ltp/issues/1241
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c
2025-05-22 19:39 ` Petr Vorel
@ 2025-05-23 6:01 ` Jan Stancek via ltp
2025-05-23 6:29 ` Petr Vorel
2025-05-23 6:29 ` Cyril Hrubis
0 siblings, 2 replies; 7+ messages in thread
From: Jan Stancek via ltp @ 2025-05-23 6:01 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
On Thu, May 22, 2025 at 9:39 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Wei, all,
>
> @all: is this a candidate for a release? It'd be nice to get it fixed.
I'd wait after release, it's not a trivial change and other than static analysis
there are no reports of it actually happening. And it also allows more time
for review.
>
> > Using sprintf without length checking in tst_tmpdir may lead to buffer overflow.
> > So in this patch use openat() instead of open().
>
> LGTM, but it'd be nice if we could use only rmobjat().
>
> Could you please remove the unused variable?
>
> tst_tmpdir.c: In function ‘rmobjat’:
> tst_tmpdir.c:327:21: warning: unused variable ‘statbuf’ [-Wunused-variable]
> 327 | struct stat statbuf;
> | ^~~~~~~
>
>
> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
>
> > Fixs:1241
>
> This is better as it shows link in GitHub web:
> Fixes: #1241
>
> Or, IMHO better
> Fixes: https://github.com/linux-test-project/ltp/issues/1241
>
> Kind regards,
> Petr
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c
2025-05-23 6:01 ` Jan Stancek via ltp
@ 2025-05-23 6:29 ` Petr Vorel
2025-05-23 6:29 ` Cyril Hrubis
1 sibling, 0 replies; 7+ messages in thread
From: Petr Vorel @ 2025-05-23 6:29 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp
Hi Jan, all,
> On Thu, May 22, 2025 at 9:39 PM Petr Vorel <pvorel@suse.cz> wrote:
> > Hi Wei, all,
> > @all: is this a candidate for a release? It'd be nice to get it fixed.
> I'd wait after release, it's not a trivial change and other than static analysis
> there are no reports of it actually happening. And it also allows more time
> for review.
Makes sense.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c
2025-05-23 6:01 ` Jan Stancek via ltp
2025-05-23 6:29 ` Petr Vorel
@ 2025-05-23 6:29 ` Cyril Hrubis
1 sibling, 0 replies; 7+ messages in thread
From: Cyril Hrubis @ 2025-05-23 6:29 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp
Hi!
> > @all: is this a candidate for a release? It'd be nice to get it fixed.
>
> I'd wait after release, it's not a trivial change and other than static analysis
> there are no reports of it actually happening. And it also allows more time
> for review.
Agree here.
I guess that it could be triggered if we pass long enough TMPDIR= to the
tests, but for changes like this we need a proper testing phase and we
are too close to the release for that to happen.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* [LTP] [PATCH v2] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c
2025-05-21 14:16 [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c Wei Gao via ltp
2025-05-22 19:39 ` Petr Vorel
@ 2025-05-23 21:09 ` Wei Gao via ltp
2025-09-01 15:08 ` [LTP] [PATCH v1] " Cyril Hrubis
2 siblings, 0 replies; 7+ messages in thread
From: Wei Gao via ltp @ 2025-05-23 21:09 UTC (permalink / raw)
To: ltp
Using sprintf without length checking in tst_tmpdir may lead to buffer overflow.
So in this patch use openat() instead of open().
Fixes: https://github.com/linux-test-project/ltp/issues/1241
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Wei Gao <wegao@suse.com>
---
lib/tst_tmpdir.c | 122 ++++++++++++++++++++++++++++++-----------------
1 file changed, 78 insertions(+), 44 deletions(-)
diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index 6ed2367b9..bcb9534c5 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -101,7 +101,7 @@ static char test_start_work_dir[PATH_MAX];
/* lib/tst_checkpoint.c */
extern futex_t *tst_futexes;
-static int rmobj(const char *obj, char **errmsg);
+static int rmobjat(int dir_fd, const char *obj, char **errmsg);
int tst_tmpdir_created(void)
{
@@ -149,8 +149,8 @@ static int purge_dir(const char *path, char **errptr)
int ret_val = 0;
DIR *dir;
struct dirent *dir_ent;
- char dirobj[PATH_MAX];
static char err_msg[PATH_MAX + 1280];
+ int dir_fd = -1;
/* Do NOT perform the request if the directory is "/" */
if (!strcmp(path, "/")) {
@@ -167,7 +167,7 @@ static int purge_dir(const char *path, char **errptr)
/* Open the directory to get access to what is in it */
if (!(dir = opendir(path))) {
if (errptr) {
- sprintf(err_msg,
+ snprintf(err_msg, sizeof(err_msg),
"Cannot open directory %s; errno=%d: %s",
path, errno, tst_strerrno(errno));
*errptr = err_msg;
@@ -175,6 +175,68 @@ static int purge_dir(const char *path, char **errptr)
return -1;
}
+ dir_fd = dirfd(dir);
+ if (dir_fd == -1) {
+ closedir(dir);
+ if (errptr) {
+ snprintf(err_msg, sizeof(err_msg),
+ "Cannot get file descriptor for directory %s; errno=%d: %s",
+ path, errno, tst_strerrno(errno));
+ *errptr = err_msg;
+ }
+ return -1;
+ }
+
+ /* Loop through the entries in the directory, removing each one */
+ for (dir_ent = readdir(dir); dir_ent; dir_ent = readdir(dir)) {
+ /* Don't remove "." or ".." */
+ if (!strcmp(dir_ent->d_name, ".")
+ || !strcmp(dir_ent->d_name, ".."))
+ continue;
+
+ /* Recursively remove the current entry */
+ if (rmobjat(dir_fd, dir_ent->d_name, errptr) != 0)
+ ret_val = -1;
+ }
+
+ closedir(dir);
+ return ret_val;
+}
+
+static int purge_dirat(int dir_fd, const char *path, char **errptr)
+{
+ int ret_val = 0;
+ DIR *dir;
+ struct dirent *dir_ent;
+ static char err_msg[PATH_MAX + 1280];
+ int subdir_fd;
+
+ errno = 0;
+
+ /* Open the subdirectory using openat */
+ subdir_fd = openat(dir_fd, path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
+ if (subdir_fd < 0) {
+ if (errptr) {
+ snprintf(err_msg, sizeof(err_msg),
+ "Cannot open subdirectory %s (via fd %d); errno=%d: %s",
+ path, dir_fd, errno, tst_strerrno(errno));
+ *errptr = err_msg;
+ }
+ return -1;
+ }
+
+ dir = fdopendir(subdir_fd);
+ if (!dir) {
+ if (errptr) {
+ snprintf(err_msg, sizeof(err_msg),
+ "Cannot open directory stream for %s (via fd %d); errno=%d: %s",
+ path, dir_fd, errno, tst_strerrno(errno));
+ *errptr = err_msg;
+ }
+ close(subdir_fd);
+ return -1;
+ }
+
/* Loop through the entries in the directory, removing each one */
for (dir_ent = readdir(dir); dir_ent; dir_ent = readdir(dir)) {
/* Don't remove "." or ".." */
@@ -183,8 +245,7 @@ static int purge_dir(const char *path, char **errptr)
continue;
/* Recursively remove the current entry */
- sprintf(dirobj, "%s/%s", path, dir_ent->d_name);
- if (rmobj(dirobj, errptr) != 0)
+ if (rmobjat(subdir_fd, dir_ent->d_name, errptr) != 0)
ret_val = -1;
}
@@ -192,63 +253,36 @@ static int purge_dir(const char *path, char **errptr)
return ret_val;
}
-static int rmobj(const char *obj, char **errmsg)
+static int rmobjat(int dir_fd, const char *obj, char **errmsg)
{
int ret_val = 0;
- struct stat statbuf;
static char err_msg[PATH_MAX + 1280];
int fd;
- fd = open(obj, O_DIRECTORY | O_NOFOLLOW);
+ fd = openat(dir_fd, obj, O_DIRECTORY | O_NOFOLLOW);
if (fd >= 0) {
close(fd);
- ret_val = purge_dir(obj, errmsg);
+ ret_val = purge_dirat(dir_fd, obj, errmsg);
/* If there were problems removing an entry, don't attempt to
remove the directory itself */
if (ret_val == -1)
return -1;
- /* Get the link count, now that all the entries have been removed */
- if (lstat(obj, &statbuf) < 0) {
+ if (unlinkat(dir_fd, obj, AT_REMOVEDIR) < 0) {
if (errmsg != NULL) {
- sprintf(err_msg,
- "lstat(%s) failed; errno=%d: %s", obj,
- errno, tst_strerrno(errno));
- *errmsg = err_msg;
- }
- return -1;
- }
-
- /* Remove the directory itself */
- if (statbuf.st_nlink >= 3) {
- /* The directory is linked; unlink() must be used */
- if (unlink(obj) < 0) {
- if (errmsg != NULL) {
- sprintf(err_msg,
- "unlink(%s) failed; errno=%d: %s",
- obj, errno, tst_strerrno(errno));
- *errmsg = err_msg;
- }
- return -1;
- }
- } else {
- /* The directory is not linked; remove() can be used */
- if (remove(obj) < 0) {
- if (errmsg != NULL) {
- sprintf(err_msg,
+ snprintf(err_msg, sizeof(err_msg),
"remove(%s) failed; errno=%d: %s",
obj, errno, tst_strerrno(errno));
- *errmsg = err_msg;
- }
- return -1;
+ *errmsg = err_msg;
}
+ return -1;
}
} else {
- if (unlink(obj) < 0) {
+ if (unlinkat(dir_fd, obj, 0) < 0) {
if (errmsg != NULL) {
- sprintf(err_msg,
- "unlink(%s) failed; errno=%d: %s", obj,
+ snprintf(err_msg, sizeof(err_msg),
+ "unlinkat(%s) failed; errno=%d: %s", obj,
errno, tst_strerrno(errno));
*errmsg = err_msg;
}
@@ -305,7 +339,7 @@ void tst_tmpdir(void)
tst_resm(TERRNO, "%s: chdir(%s) failed", __func__, TESTDIR);
/* Try to remove the directory */
- if (rmobj(TESTDIR, &errmsg) == -1) {
+ if (rmobjat(0, TESTDIR, &errmsg) == -1) {
tst_resm(TWARN, "%s: rmobj(%s) failed: %s",
__func__, TESTDIR, errmsg);
}
@@ -343,7 +377,7 @@ void tst_rmdir(void)
/*
* Attempt to remove the "TESTDIR" directory, using rmobj().
*/
- if (rmobj(TESTDIR, &errmsg) == -1) {
+ if (rmobjat(0, TESTDIR, &errmsg) == -1) {
tst_resm(TWARN, "%s: rmobj(%s) failed: %s",
__func__, TESTDIR, errmsg);
}
--
2.49.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c
2025-05-21 14:16 [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c Wei Gao via ltp
2025-05-22 19:39 ` Petr Vorel
2025-05-23 21:09 ` [LTP] [PATCH v2] " Wei Gao via ltp
@ 2025-09-01 15:08 ` Cyril Hrubis
2 siblings, 0 replies; 7+ messages in thread
From: Cyril Hrubis @ 2025-09-01 15:08 UTC (permalink / raw)
To: Wei Gao; +Cc: ltp
Hi!
> Using sprintf without length checking in tst_tmpdir may lead to buffer overflow.
> So in this patch use openat() instead of open().
>
> Fixs:1241
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
> lib/tst_tmpdir.c | 119 +++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 111 insertions(+), 8 deletions(-)
>
> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> index 6ed2367b9..7bd55022d 100644
> --- a/lib/tst_tmpdir.c
> +++ b/lib/tst_tmpdir.c
> @@ -102,6 +102,7 @@ static char test_start_work_dir[PATH_MAX];
> extern futex_t *tst_futexes;
>
> static int rmobj(const char *obj, char **errmsg);
We shouldn't keep the old rmobj() purge_dir() calls, these should be
replaced by rmobjat() and purge_dirat(). Hint: use AT_FDCWD when there
is no dir_fd to pass.
> +static int rmobjat(int dir_fd, const char *obj, char **errmsg);
>
> int tst_tmpdir_created(void)
> {
> @@ -149,8 +150,8 @@ static int purge_dir(const char *path, char **errptr)
> int ret_val = 0;
> DIR *dir;
> struct dirent *dir_ent;
> - char dirobj[PATH_MAX];
> static char err_msg[PATH_MAX + 1280];
> + int dir_fd = -1;
>
> /* Do NOT perform the request if the directory is "/" */
> if (!strcmp(path, "/")) {
> @@ -167,7 +168,7 @@ static int purge_dir(const char *path, char **errptr)
> /* Open the directory to get access to what is in it */
> if (!(dir = opendir(path))) {
> if (errptr) {
> - sprintf(err_msg,
> + snprintf(err_msg, sizeof(err_msg),
> "Cannot open directory %s; errno=%d: %s",
> path, errno, tst_strerrno(errno));
> *errptr = err_msg;
> @@ -175,6 +176,18 @@ static int purge_dir(const char *path, char **errptr)
> return -1;
> }
>
> + dir_fd = dirfd(dir);
> + if (dir_fd == -1) {
> + closedir(dir);
> + if (errptr) {
> + snprintf(err_msg, sizeof(err_msg),
> + "Cannot get file descriptor for directory %s; errno=%d: %s",
> + path, errno, tst_strerrno(errno));
> + *errptr = err_msg;
> + }
> + return -1;
> + }
> +
> /* Loop through the entries in the directory, removing each one */
> for (dir_ent = readdir(dir); dir_ent; dir_ent = readdir(dir)) {
> /* Don't remove "." or ".." */
> @@ -183,8 +196,57 @@ static int purge_dir(const char *path, char **errptr)
> continue;
>
> /* Recursively remove the current entry */
> - sprintf(dirobj, "%s/%s", path, dir_ent->d_name);
> - if (rmobj(dirobj, errptr) != 0)
> + if (rmobjat(dir_fd, dir_ent->d_name, errptr) != 0)
> + ret_val = -1;
> + }
close(dir_fd) here?
> + closedir(dir);
> + return ret_val;
> +}
> +
> +static int purge_dirat(int dir_fd, const char *path, char **errptr)
> +{
> + int ret_val = 0;
> + DIR *dir;
> + struct dirent *dir_ent;
> + static char err_msg[PATH_MAX + 1280];
> + int subdir_fd;
> +
> + errno = 0;
> +
> + /* Open the subdirectory using openat */
> + subdir_fd = openat(dir_fd, path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
> + if (subdir_fd < 0) {
> + if (errptr) {
> + snprintf(err_msg, sizeof(err_msg),
> + "Cannot open subdirectory %s (via fd %d); errno=%d: %s",
> + path, dir_fd, errno, tst_strerrno(errno));
> + *errptr = err_msg;
> + }
> + return -1;
> + }
> +
> + dir = fdopendir(subdir_fd);
> + if (!dir) {
> + if (errptr) {
> + snprintf(err_msg, sizeof(err_msg),
> + "Cannot open directory stream for %s (via fd %d); errno=%d: %s",
> + path, dir_fd, errno, tst_strerrno(errno));
> + *errptr = err_msg;
> + }
> + close(subdir_fd);
> + return -1;
> + }
> +
> + /* Loop through the entries in the directory, removing each one */
> + for (dir_ent = readdir(dir); dir_ent; dir_ent = readdir(dir)) {
> + /* Don't remove "." or ".." */
> + if (!strcmp(dir_ent->d_name, ".")
> + || !strcmp(dir_ent->d_name, ".."))
> + continue;
> +
> + /* Recursively remove the current entry */
> + if (rmobjat(subdir_fd, dir_ent->d_name, errptr) != 0)
> ret_val = -1;
> }
>
> @@ -212,7 +274,7 @@ static int rmobj(const char *obj, char **errmsg)
> /* Get the link count, now that all the entries have been removed */
> if (lstat(obj, &statbuf) < 0) {
> if (errmsg != NULL) {
> - sprintf(err_msg,
> + snprintf(err_msg, sizeof(err_msg),
> "lstat(%s) failed; errno=%d: %s", obj,
> errno, tst_strerrno(errno));
> *errmsg = err_msg;
> @@ -225,7 +287,7 @@ static int rmobj(const char *obj, char **errmsg)
> /* The directory is linked; unlink() must be used */
> if (unlink(obj) < 0) {
> if (errmsg != NULL) {
> - sprintf(err_msg,
> + snprintf(err_msg, sizeof(err_msg),
> "unlink(%s) failed; errno=%d: %s",
> obj, errno, tst_strerrno(errno));
> *errmsg = err_msg;
> @@ -236,7 +298,7 @@ static int rmobj(const char *obj, char **errmsg)
> /* The directory is not linked; remove() can be used */
> if (remove(obj) < 0) {
> if (errmsg != NULL) {
> - sprintf(err_msg,
> + snprintf(err_msg, sizeof(err_msg),
> "remove(%s) failed; errno=%d: %s",
> obj, errno, tst_strerrno(errno));
> *errmsg = err_msg;
> @@ -247,7 +309,7 @@ static int rmobj(const char *obj, char **errmsg)
> } else {
> if (unlink(obj) < 0) {
> if (errmsg != NULL) {
> - sprintf(err_msg,
> + snprintf(err_msg, sizeof(err_msg),
> "unlink(%s) failed; errno=%d: %s", obj,
> errno, tst_strerrno(errno));
> *errmsg = err_msg;
> @@ -259,6 +321,47 @@ static int rmobj(const char *obj, char **errmsg)
> return 0;
> }
>
> +static int rmobjat(int dir_fd, const char *obj, char **errmsg)
> +{
> + int ret_val = 0;
> + struct stat statbuf;
> + static char err_msg[PATH_MAX + 1280];
> + int fd;
> +
> + fd = openat(dir_fd, obj, O_DIRECTORY | O_NOFOLLOW);
> + if (fd >= 0) {
> + close(fd);
We close the fd here just to open it in purge_dirat(), shouldn't we pass
it to the purge_dirat() function instead?
> + ret_val = purge_dirat(dir_fd, obj, errmsg);
> +
> + /* If there were problems removing an entry, don't attempt to
> + remove the directory itself */
> + if (ret_val == -1)
> + return -1;
> +
> + if (unlinkat(dir_fd, obj, AT_REMOVEDIR) < 0) {
> + if (errmsg != NULL) {
> + snprintf(err_msg, sizeof(err_msg),
> + "remove(%s) failed; errno=%d: %s",
> + obj, errno, tst_strerrno(errno));
> + *errmsg = err_msg;
> + }
> + return -1;
> + }
> + } else {
> + if (unlinkat(dir_fd, obj, 0) < 0) {
> + if (errmsg != NULL) {
> + snprintf(err_msg, sizeof(err_msg),
> + "unlinkat(%s) failed; errno=%d: %s", obj,
> + errno, tst_strerrno(errno));
> + *errmsg = err_msg;
> + }
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> void tst_tmpdir(void)
> {
> char template[PATH_MAX];
> --
> 2.49.0
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-09-01 15:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-21 14:16 [LTP] [PATCH v1] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c Wei Gao via ltp
2025-05-22 19:39 ` Petr Vorel
2025-05-23 6:01 ` Jan Stancek via ltp
2025-05-23 6:29 ` Petr Vorel
2025-05-23 6:29 ` Cyril Hrubis
2025-05-23 21:09 ` [LTP] [PATCH v2] " Wei Gao via ltp
2025-09-01 15:08 ` [LTP] [PATCH v1] " Cyril Hrubis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).