Skip to content

Commit

Permalink
launch: list directories alphabetically
Browse files Browse the repository at this point in the history
When enumerating directories ensure that we iterate the entries in
alphabetical order.

We used to follow what the reference-implementation does, and simply
read directories in semi-random on-disk order. This is rather brittle
to debug and exposes behavior that we really don't want to expose.
Instead, we want predictable enumerations so policy and service-files
can be more easily traced and debugged.

The downside of ordering directory entries is that we have to collect
all entries first, rather than using the streaming API of the kernel.
Fortunately, this is what we do, anyway, since we already have 1-to-1
mappings of the directory entries to internal data-structures. Hence,
in all our current use-cases, it is not an issue to stream everything
into a collection first.

Signed-off-by: David Rheinsberg <[email protected]>
  • Loading branch information
dvdhrm committed Dec 14, 2023
1 parent 0c8be71 commit cb73746
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 10 deletions.
15 changes: 11 additions & 4 deletions src/launch/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "util/common.h"
#include "util/dirwatch.h"
#include "util/error.h"
#include "util/fs.h"
#include "util/selinux.h"
#include "util/string.h"

Expand Down Expand Up @@ -1058,10 +1059,11 @@ static void config_parser_end_fn(void *userdata, const XML_Char *name) {
break;

case CONFIG_NODE_INCLUDEDIR: {
_c_cleanup_(fs_dirlist_freep) FsDirlist *list = NULL;
_c_cleanup_(c_closedirp) DIR *dir = NULL;
static const char suffix[] = ".conf";
struct dirent *de;
size_t n;
size_t i, n;

r = config_path_new_dir(&state->current->includedir.dir,
state->file,
Expand Down Expand Up @@ -1091,11 +1093,16 @@ static void config_parser_end_fn(void *userdata, const XML_Char *name) {
return;
}

for (errno = 0, de = readdir(dir);
de;
errno = 0, de = readdir(dir)) {
r = fs_dir_list(dir, &list, 0);
if (r) {
state->error = error_fold(r);
return;
}

for (i = 0; i < list->n_entries; ++i) {
_c_cleanup_(config_node_freep) ConfigNode *node = NULL;

de = list->entries[i];
n = strlen(de->d_name);

if (n <= strlen(suffix))
Expand Down
15 changes: 9 additions & 6 deletions src/launch/launcher.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "util/audit.h"
#include "util/dirwatch.h"
#include "util/error.h"
#include "util/fs.h"
#include "util/log.h"
#include "util/misc.h"
#include "util/string.h"
Expand Down Expand Up @@ -736,10 +737,11 @@ static int launcher_load_service_file(Launcher *launcher, const char *path, cons

static int launcher_load_service_dir(Launcher *launcher, const char *dirpath, NSSCache *nss_cache) {
const char suffix[] = ".service";
_c_cleanup_(fs_dirlist_freep) FsDirlist *list = NULL;
_c_cleanup_(c_closedirp) DIR *dir = NULL;
struct dirent *de;
char *path;
size_t n;
size_t i, n;
int r;

dir = opendir(dirpath);
Expand Down Expand Up @@ -767,11 +769,12 @@ static int launcher_load_service_dir(Launcher *launcher, const char *dirpath, NS
if (r)
return error_fold(r);

for (errno = 0, de = readdir(dir);
de;
errno = 0, de = readdir(dir)) {
if (de->d_name[0] == '.')
continue;
r = fs_dir_list(dir, &list, FS_DIR_FLAG_NO_HIDDEN);
if (r)
return error_fold(r);

for (i = 0; i < list->n_entries; ++i) {
de = list->entries[i];

n = strlen(de->d_name);
if (n <= strlen(suffix))
Expand Down

0 comments on commit cb73746

Please sign in to comment.