From patchwork Tue Sep 6 15:51:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Roberts, William C" X-Patchwork-Id: 9317415 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 07E7B607D3 for ; Tue, 6 Sep 2016 15:53:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EBB9E28E0D for ; Tue, 6 Sep 2016 15:53:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DFDFC28E11; Tue, 6 Sep 2016 15:53:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00 autolearn=ham version=3.3.1 Received: from emsm-gh1-uea10.nsa.gov (smtp.nsa.gov [8.44.101.8]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 939B528E0D for ; Tue, 6 Sep 2016 15:53:26 +0000 (UTC) X-IronPort-AV: E=Sophos;i="5.30,292,1470700800"; d="scan'208";a="17272633" IronPort-PHdr: =?us-ascii?q?9a23=3AKjJNnR90E87fCP9uRHKM819IXTAuvvDOBiVQ1KB8?= =?us-ascii?q?0O0cTK2v8tzYMVDF4r011RmSDNydtqMP27ue8/i5HzdRudDZ6DFKWacPfidNsd?= =?us-ascii?q?8RkQ0kDZzNImzAB9muURYHGt9fXkRu5XCxPBsdMs//Y1rPvi/6tmZKSV3BPAZ4?= =?us-ascii?q?bt74BpTVx5zukbvjotuOMk4Y1XL9Oeo0d0Tu612J94E/ushLEu4J0BzHo39FKa?= =?us-ascii?q?x95FhDAhatpSv6/dq655V58i5d6LoL/s9EVrjmLexjFeQLRAk8NygJwOGj9Vya?= =?us-ascii?q?FUrcrkcbB34blhtOHhjt8ADxXpC3tDDz8OV6xm3SJsD/S7wuXjWuqqNqUwPAlD?= =?us-ascii?q?YMNzl/9nrezMN3kuYTux66jwBuyI7TJoeOPbxxeb2ZNdEFTmNbQpx5Sz1KAoT6?= =?us-ascii?q?aZAGSeUGI7V2tY748kQPqR+/DAzqD6X1zTVFnGPt9aw8z+klVwrB2V9zV+kSuW?= =?us-ascii?q?jZ+Y2mfJwZVvq4meyRlTg=3D?= X-IPAS-Result: =?us-ascii?q?A2GzBAB+5c5X/wHyM5BdGwEBAQMBAQEXAQEEAQGDDgEBAQE?= =?us-ascii?q?BHoFTpgWULx2HYUwBAQEBAQEBAQIBAlsngjIEAxMFghgCJBMUIA4DCQIXCCEIC?= =?us-ascii?q?AMBLRUYBwsFGASIKbw2ASSIL4ZgEQGFWx0FiC6GbYo4jzYCgWyIBgyFVwKQRlS?= =?us-ascii?q?CXAMbgW1QAYQ8DRcHWoEnAQEB?= Received: from unknown (HELO tarius.tycho.ncsc.mil) ([144.51.242.1]) by emsm-gh1-uea10.nsa.gov with ESMTP; 06 Sep 2016 15:53:24 +0000 Received: from prometheus.infosec.tycho.ncsc.mil (prometheus [192.168.25.40]) by tarius.tycho.ncsc.mil (8.14.4/8.14.4) with ESMTP id u86Fr16Y020940; Tue, 6 Sep 2016 11:53:04 -0400 Received: from tarius.tycho.ncsc.mil (tarius.infosec.tycho.ncsc.mil [144.51.242.1]) by prometheus.infosec.tycho.ncsc.mil (8.15.2/8.15.2) with ESMTP id u86FpCco046120 for ; Tue, 6 Sep 2016 11:51:12 -0400 Received: from goalie.tycho.ncsc.mil (goalie [144.51.242.250]) by tarius.tycho.ncsc.mil (8.14.4/8.14.4) with ESMTP id u86FpC41020792; Tue, 6 Sep 2016 11:51:12 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A1CtBADI5M5X/yNjr8ZdGgEBAQECAQEBAYMtAQEBAQEegVO2J4QRFIYIgV5MAQIBAQEBAQJehTZSMIEPEohKvCkBAQgniC+JWAuCah0FiC6GbYo4jzYCgWyIEoVXApBGVIJcAxuBbVABhDwrggEBAQE X-IPAS-Result: A1CtBADI5M5X/yNjr8ZdGgEBAQECAQEBAYMtAQEBAQEegVO2J4QRFIYIgV5MAQIBAQEBAQJehTZSMIEPEohKvCkBAQgniC+JWAuCah0FiC6GbYo4jzYCgWyIEoVXApBGVIJcAxuBbVABhDwrggEBAQE X-IronPort-AV: E=Sophos;i="5.30,292,1470715200"; d="scan'208";a="5686687" Received: from emsm-gh1-uea11.corp.nsa.gov (HELO emsm-gh1-uea11.nsa.gov) ([10.208.41.37]) by goalie.tycho.ncsc.mil with ESMTP; 06 Sep 2016 11:51:11 -0400 IronPort-PHdr: =?us-ascii?q?9a23=3AR4B7mxw8e1fNqzPXCy+O+j09IxM/srCxBDY+r6Qd?= =?us-ascii?q?0OwUIJqq85mqBkHD//Il1AaPBtSCra8VwLqH++C4ACpbsM7H6ChDOLV3FDY9wf?= =?us-ascii?q?0MmAIhBMPXQWbaF9XNKxIAIcJZSVV+9Gu6O0UGUOz3ZlnVv2HgpWVKQka3CwN5?= =?us-ascii?q?K6zPF5LIiIzvjqbpqsSVPlQD1Gv1Iesrak7n9UOJ7oheqLAhA5558gHOrHpMdr?= =?us-ascii?q?Ye7kJTDnXXoSzB4Nyt9oVo6SVatqFp3cdBVaLnY/ZwFuQAX3wOelo478zztBTF?= =?us-ascii?q?URDHpj5FCj1XwSxzBBLZ4R3AU5zwqnGy9rYlmXrSAcqjVr0wWDK//493WRToj2?= =?us-ascii?q?EBLDd//2bJzoRrgatdrAimphA6x4/PfKmJJfF+eeXbZtpcSm1fGo5KWzdpHpK3?= =?us-ascii?q?b4xJCfEIe+lfsc21pUQFpAGiXyGwFejvzXlOnXaw0qokgMo7FgSTww0kG9MHtT?= =?us-ascii?q?Lf6s/yPqoITfudza/UwDGFZPRTimS105TBbh10+aLEZrl3a8eEjBd3Gg=3D=3D?= X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0EDBAAJ5c5X/yNjr8ZdGgEBAQECAQEBA?= =?us-ascii?q?RYBAQEDAQEBgw4BAQEBAR6BU6YFkCKEERSGCIFeTAEBAQEBAQEBAgECWyeCMgQ?= =?us-ascii?q?BFQWCPlIwgQ8SiEq8KQEBCAIliC+MTR0FiC6GbYo4jzYCgWyIEoVXApBGVIJcA?= =?us-ascii?q?xuBbVABhDwrQYFAAQEB?= X-IPAS-Result: =?us-ascii?q?A0EDBAAJ5c5X/yNjr8ZdGgEBAQECAQEBARYBAQEDAQEBgw4?= =?us-ascii?q?BAQEBAR6BU6YFkCKEERSGCIFeTAEBAQEBAQEBAgECWyeCMgQBFQWCPlIwgQ8Si?= =?us-ascii?q?Eq8KQEBCAIliC+MTR0FiC6GbYo4jzYCgWyIEoVXApBGVIJcAxuBbVABhDwrQYF?= =?us-ascii?q?AAQEB?= X-IronPort-AV: E=Sophos;i="5.30,292,1470700800"; d="scan'208";a="19010046" Received: from fmsmga002-icc.fm.intel.com ([198.175.99.35]) by emsm-gh1-uea11.nsa.gov with ESMTP; 06 Sep 2016 15:51:10 +0000 Received: from fmsmga004-icc.fm.intel.com ([198.175.99.11]) by fmsmga002-icc.fm.intel.com with ESMTP; 06 Sep 2016 08:51:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,292,1470726000"; d="scan'208";a="165027154" Received: from tjsingh-mobl.amr.corp.intel.com (HELO wcrobert-MOBL1.amr.corp.intel.com) ([10.254.63.197]) by fmsmga004.fm.intel.com with ESMTP; 06 Sep 2016 08:51:09 -0700 From: william.c.roberts@intel.com To: selinux@tycho.nsa.gov, seandroid-list@tycho.nsa.gov, sds@tycho.nsa.gov, jwcart2@tycho.nsa.gov Subject: [PATCH] libselinux: clean up process file Date: Tue, 6 Sep 2016 08:51:06 -0700 Message-Id: <1473177066-25068-1-git-send-email-william.c.roberts@intel.com> X-Mailer: git-send-email 1.9.1 X-BeenThere: selinux@tycho.nsa.gov X-Mailman-Version: 2.1.20 Precedence: list List-Id: "Security-Enhanced Linux \(SELinux\) mailing list" List-Post: List-Help: MIME-Version: 1.0 Errors-To: selinux-bounces@tycho.nsa.gov Sender: "Selinux" X-Virus-Scanned: ClamAV using ClamSMTP From: William Roberts 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 --- 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, "<>") && 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)