Skip to content

Commit 8f3bb00

Browse files
committed
Refactor: executor: Avoid TOCTOU when cleaning up remote schema dir
This is rather trivial and should not pose a real safety/reliability risk. However, Coverity identified it as a TOCTOU bug, and cleaning it up seems like an improvement. Signed-off-by: Reid Wahl <[email protected]>
1 parent 7e9b8cc commit 8f3bb00

File tree

1 file changed

+26
-36
lines changed

1 file changed

+26
-36
lines changed

daemons/execd/remoted_schemas.c

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023-2024 the Pacemaker project contributors
2+
* Copyright 2023-2025 the Pacemaker project contributors
33
*
44
* The version control history for this file may have further details.
55
*
@@ -25,17 +25,21 @@
2525
static pid_t schema_fetch_pid = 0;
2626

2727
static int
28-
rm_files(const char *pathname, const struct stat *sbuf, int type, struct FTW *ftwb)
28+
rm_files(const char *fpath, const struct stat *sb, int typeflag,
29+
struct FTW *ftwbuf)
2930
{
30-
/* Don't delete PCMK__REMOTE_SCHEMA_DIR . */
31-
if (ftwb->level == 0) {
32-
return 0;
33-
}
31+
if (ftwbuf->level == 0) {
32+
// Don't delete the remote schema directory
33+
if (typeflag != FTW_DP) {
34+
crm_err("%s already exists but is not a directory", fpath);
35+
return ENOTDIR;
36+
}
3437

35-
if (remove(pathname) != 0) {
38+
} else if (remove(fpath) != 0) {
3639
int rc = errno;
37-
crm_err("Could not remove %s: %s", pathname, pcmk_rc_str(rc));
38-
return -1;
40+
41+
crm_err("Could not remove %s: %s", fpath, pcmk_rc_str(rc));
42+
return rc;
3943
}
4044

4145
return 0;
@@ -45,41 +49,27 @@ static void
4549
clean_up_extra_schema_files(void)
4650
{
4751
const char *remote_schema_dir = pcmk__remote_schema_dir();
48-
struct stat sb;
49-
int rc;
52+
int rc = nftw(remote_schema_dir, rm_files, 10,
53+
FTW_DEPTH|FTW_MOUNT|FTW_PHYS);
5054

51-
rc = stat(remote_schema_dir, &sb);
52-
53-
if (rc == -1) {
54-
if (errno == ENOENT) {
55-
/* If the directory doesn't exist, try to make it first. */
56-
if (mkdir(remote_schema_dir, 0755) != 0) {
57-
rc = errno;
58-
crm_err("Could not create directory for schemas: %s",
59-
pcmk_rc_str(rc));
60-
}
55+
if (rc != -1) {
56+
// Either we succeeded (rc == 0) or rm_files() failed (rc > 0)
57+
return;
58+
}
6159

62-
} else {
60+
// rc == -1: nftw() itself failed
61+
if (errno == ENOENT) {
62+
// remote_schema_dir doesn't exist, so try to create it
63+
if (mkdir(remote_schema_dir, 0755) != 0) {
6364
rc = errno;
6465
crm_err("Could not create directory for schemas: %s",
6566
pcmk_rc_str(rc));
6667
}
6768

68-
} else if (!S_ISDIR(sb.st_mode)) {
69-
/* If something exists with the same name that's not a directory, that's
70-
* an error.
71-
*/
72-
crm_err("%s already exists but is not a directory", remote_schema_dir);
73-
7469
} else {
75-
/* It's a directory - clear it out so we can download potentially new
76-
* schema files.
77-
*/
78-
rc = nftw(remote_schema_dir, rm_files, 10, FTW_DEPTH|FTW_MOUNT|FTW_PHYS);
79-
80-
if (rc != 0) {
81-
crm_err("Could not remove %s: %s", remote_schema_dir, pcmk_rc_str(rc));
82-
}
70+
rc = errno;
71+
crm_err("Could not clean up directory for schemas: %s",
72+
pcmk_rc_str(rc));
8373
}
8474
}
8575

0 commit comments

Comments
 (0)