From: William Roberts <william.c.robe...@intel.com>

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.robe...@intel.com>
---
 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(&current[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

_______________________________________________
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Reply via email to