From: William Roberts <william.c.roberts@xxxxxxxxx>
The current process_file() code will open the file
twice on the case of a binary file, correct this.
The general flow through process_file() was a bit
difficult to read, streamline the routine to be
more readable.
Detailed statistics of before and after:
Source lines of code reported by cloc on modified files:
before: 1090
after: 1102
Object size difference:
before: 195530 bytes
after: 195563 bytes
Signed-off-by: William Roberts <william.c.roberts@xxxxxxxxx>
---
libselinux/src/label_file.c | 264 ++++++++++++++++++++++++--------------------
libselinux/src/label_file.h | 1 +
2 files changed, 147 insertions(+), 118 deletions(-)
diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
index c89bb35..26b1b87 100644
--- a/libselinux/src/label_file.c
+++ b/libselinux/src/label_file.c
@@ -97,62 +97,44 @@ static int nodups_specs(struct saved_data *data, const char *path)
return rc;
}
-static int load_mmap(struct selabel_handle *rec, const char *path,
- struct stat *sb, bool isbinary,
- struct selabel_digest *digest)
+static int process_text_file(FILE *fp, const char *prefix, struct selabel_handle *rec)
+{
+ /*
+ * Then do detailed validation of the input and fill the spec array
+ */
+ int rc;
+ size_t line_len;
+ unsigned lineno = 0;
+ char *line_buf = NULL;
+ struct saved_data *data = (struct saved_data *)rec->data;
+
+ while (getline(&line_buf, &line_len, fp) > 0) {
+ rc = process_line(rec, data->path, prefix, line_buf, ++lineno);
+ if (rc)
+ goto out;
+ }
+ rc = 0;
+out:
+ free(line_buf);
+ return rc;
+}
+
+static int load_mmap(FILE *fp, size_t len, struct selabel_handle *rec)
{
struct saved_data *data = (struct saved_data *)rec->data;
- char mmap_path[PATH_MAX + 1];
- int mmapfd;
int rc;
- struct stat mmap_stat;
char *addr, *str_buf;
- size_t len;
int *stem_map;
struct mmap_area *mmap_area;
uint32_t i, magic, version;
uint32_t entry_len, stem_map_len, regex_array_len;
- if (isbinary) {
- len = strlen(path);
- if (len >= sizeof(mmap_path))
- return -1;
- strcpy(mmap_path, path);
- } else {
- rc = snprintf(mmap_path, sizeof(mmap_path), "%s.bin", path);
- if (rc >= (int)sizeof(mmap_path))
- return -1;
- }
-
- mmapfd = open(mmap_path, O_RDONLY | O_CLOEXEC);
- if (mmapfd < 0)
- return -1;
-
- rc = fstat(mmapfd, &mmap_stat);
- if (rc < 0) {
- close(mmapfd);
- return -1;
- }
-
- /* if mmap is old, ignore it */
- if (mmap_stat.st_mtime < sb->st_mtime) {
- close(mmapfd);
- return -1;
- }
-
- /* ok, read it in... */
- len = mmap_stat.st_size;
- len += (sysconf(_SC_PAGE_SIZE) - 1);
- len &= ~(sysconf(_SC_PAGE_SIZE) - 1);
-
mmap_area = malloc(sizeof(*mmap_area));
if (!mmap_area) {
- close(mmapfd);
return -1;
}
- addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, mmapfd, 0);
- close(mmapfd);
+ addr = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fileno(fp), 0);
if (addr == MAP_FAILED) {
free(mmap_area);
perror("mmap");
@@ -306,7 +288,7 @@ static int load_mmap(struct selabel_handle *rec, const char *path,
if (strcmp(spec->lr.ctx_raw, "<<none>>") && rec->validating) {
if (selabel_validate(rec, &spec->lr) < 0) {
selinux_log(SELINUX_ERROR,
- "%s: context %s is invalid\n", mmap_path, spec->lr.ctx_raw);
+ "%s: context %s is invalid\n", data->path, spec->lr.ctx_raw);
goto err;
}
}
@@ -408,105 +390,150 @@ static int load_mmap(struct selabel_handle *rec, const char *path,
data->nspec++;
}
- rc = digest_add_specfile(digest, NULL, addr, mmap_stat.st_size,
- mmap_path);
- if (rc)
- goto err;
-
err:
free(stem_map);
return rc;
}
-static int process_file(const char *path, const char *suffix,
- struct selabel_handle *rec,
- const char *prefix, struct selabel_digest *digest)
-{
- FILE *fp;
+struct file_details {
+ const char *suffix;
struct stat sb;
- unsigned int lineno;
- size_t line_len = 0;
- char *line_buf = NULL;
+};
+
+static char *rolling_append(char *current, const char *suffix, size_t max)
+{
+ size_t size;
+ size_t suffix_size;
+ size_t current_size;
+
+ if (!suffix)
+ return current;
+
+ /*
+ * Overflow check that the following
+ * arithmatec will not overflow or
+ */
+ current_size = strlen(current);
+ suffix_size = strlen(suffix);
+
+ size = current_size + suffix_size;
+ if (size < current_size || size < suffix_size)
+ return NULL;
+
+ /* ensure space for the '.' and the '\0' characters. */
+ if (size >= (SIZE_MAX - 2))
+ return NULL;
+
+ size += 2;
+
+ if (size > max)
+ return NULL;
+
+ /* Append any given suffix */
+ char *to = stpcpy(¤t[current_size], ".");
+ strcat(to, suffix);
+
+ return current;
+}
+
+static bool fcontext_is_binary(FILE *fp)
+{
+ uint32_t magic;
+
+ size_t len = fread(&magic, sizeof(magic), 1, fp);
+ rewind(fp);
+
+ return (len && (magic == SELINUX_MAGIC_COMPILED_FCONTEXT));
+}
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+
+static FILE *open_file(const char *path, const char *suffix,
+ char **save_path, struct stat *sb)
+{
+ unsigned i;
int rc;
char stack_path[PATH_MAX + 1];
- bool isbinary = false;
- uint32_t magic;
- /* append the path suffix if we have one */
- if (suffix) {
- rc = snprintf(stack_path, sizeof(stack_path),
- "%s.%s", path, suffix);
- if (rc >= (int)sizeof(stack_path)) {
- errno = ENAMETOOLONG;
- return -1;
- }
- path = stack_path;
+ struct file_details *found = NULL;
+ char found_path[sizeof(stack_path)];
+
+ /*
+ * Rolling append of suffix. Try to open with path.suffix then the
+ * next as path.suffix.suffix and so forth.
+ */
+ struct file_details fdetails[2] = {
+ { .suffix = suffix },
+ { .suffix = "bin" }
+ };
+
+ rc = snprintf(stack_path, sizeof(stack_path), "%s", path);
+ if (rc >= (int) sizeof(stack_path)) {
+ errno = ENAMETOOLONG;
+ return NULL;
}
- /* Open the specification file. */
- fp = fopen(path, "r");
- if (fp) {
- __fsetlocking(fp, FSETLOCKING_BYCALLER);
+ for (i = 0; i < ARRAY_SIZE(fdetails); i++) {
- if (fstat(fileno(fp), &sb) < 0)
- return -1;
- if (!S_ISREG(sb.st_mode)) {
- errno = EINVAL;
- return -1;
- }
+ /* This handles the case if suffix is null */
+ path = rolling_append(stack_path, fdetails[i].suffix,
+ sizeof(stack_path));
+ if (!path)
+ return NULL;
- magic = 0;
- if (fread(&magic, sizeof magic, 1, fp) != 1) {
- if (ferror(fp)) {
- errno = EINVAL;
- fclose(fp);
- return -1;
- }
- clearerr(fp);
+ rc = stat(path, &fdetails[i].sb);
+ if (rc)
+ continue;
+
+ /* first file thing found, just take it */
+ if (!found) {
+ strcpy(found_path, path);
+ found = &fdetails[i];
+ continue;
}
- if (magic == SELINUX_MAGIC_COMPILED_FCONTEXT) {
- /* file_contexts.bin format */
- fclose(fp);
- fp = NULL;
- isbinary = true;
- } else {
- rewind(fp);
+ /* next possible finds, keep picking the newest file */
+ if (fdetails[i].sb.st_mtime > found->sb.st_mtime) {
+ found = &fdetails[i];
+ strcpy(found_path, path);
}
- } else {
- /*
- * Text file does not exist, so clear the timestamp
- * so that we will always pass the timestamp comparison
- * with the bin file in load_mmap().
- */
- sb.st_mtime = 0;
}
- rc = load_mmap(rec, path, &sb, isbinary, digest);
- if (rc == 0)
- goto out;
+ if (!found) {
+ errno = ENOENT;
+ return NULL;
+ }
+ *save_path = strdup(found_path);
+ if (!*save_path)
+ return NULL;
- if (!fp)
- return -1; /* no text or bin file */
+ memcpy(sb, &found->sb, sizeof(*sb));
+ return fopen(found_path, "r");
+}
- /*
- * Then do detailed validation of the input and fill the spec array
- */
- lineno = 0;
- rc = 0;
- while (getline(&line_buf, &line_len, fp) > 0) {
- rc = process_line(rec, path, prefix, line_buf, ++lineno);
- if (rc)
- goto out;
- }
+static int process_file(const char *path, const char *suffix,
+ struct selabel_handle *rec,
+ const char *prefix, struct selabel_digest *digest)
+{
+ FILE *fp;
+ struct stat sb;
+ int rc;
+ struct saved_data *data = (struct saved_data *)rec->data;
- rc = digest_add_specfile(digest, fp, NULL, sb.st_size, path);
+ fp = open_file(path, suffix, &data->path, &sb);
+ if (fp == NULL)
+ return -1;
+ rc = fcontext_is_binary(fp) ?
+ load_mmap(fp, sb.st_size, rec) :
+ process_text_file(fp, prefix, rec);
+ if (rc < 0)
+ goto out;
+
+ rc = digest_add_specfile(digest, fp, NULL, sb.st_size, path);
out:
- free(line_buf);
- if (fp)
- fclose(fp);
+ fclose(fp);
return rc;
}
@@ -634,6 +661,7 @@ static void closef(struct selabel_handle *rec)
area = area->next;
free(last_area);
}
+ free(data->path);
free(data);
}
diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
index 6d1e890..fe5dc60 100644
--- a/libselinux/src/label_file.h
+++ b/libselinux/src/label_file.h
@@ -76,6 +76,7 @@ struct saved_data {
int num_stems;
int alloc_stems;
struct mmap_area *mmap_areas;
+ char *path;
};
static inline pcre_extra *get_pcre_extra(struct spec *spec)
--
1.9.1
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
