From patchwork Wed Aug 31 13:55:46 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: 9307315 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 0411760487 for ; Wed, 31 Aug 2016 13:59:07 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E524828F2F for ; Wed, 31 Aug 2016 13:59:06 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D97E628F37; Wed, 31 Aug 2016 13:59:06 +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 88A6F28F2F for ; Wed, 31 Aug 2016 13:59:04 +0000 (UTC) X-IronPort-AV: E=Sophos;i="5.30,262,1470700800"; d="scan'208";a="17118563" IronPort-PHdr: =?us-ascii?q?9a23=3A0bzOaxwxWB6k+IfXCy+O+j09IxM/srCxBDY+r6Qd?= =?us-ascii?q?0OkTIJqq85mqBkHD//Il1AaPBtSCra4UwLSI++C4ACpbsM7H6ChDOLV3FDY9wf?= =?us-ascii?q?0MmAIhBMPXQWbaF9XNKxIAIcJZSVV+9Gu6O0UGUOz3ZlnVv2HgpWVKQka3CwN5?= =?us-ascii?q?K6zPF5LIiIzvjqbpqsSVP1UD2mT1Iesrak7n9UOJ7oheqLAhA5558gHOrHpMdr?= =?us-ascii?q?Ye7kJTDnXXoSzB4Nyt9oVo6SVatqFp3cdBVaLnY/ZwFuQAX3wbKWR92OnH/VmG?= =?us-ascii?q?E0rcrkcbB34blhtOHhjt8ADxXpC3tDDz8OV6xm3SJsD/S7wuXjWuqqNqUwPAlD?= =?us-ascii?q?YMNzl/9nrezMN3kuYTux66jwBuyI7TJoeOPbxxeb2ZNdEFTmNbQpx5Sz1KAoT6?= =?us-ascii?q?aZAGSeUGI7V2tY748kQPqR+/DAzqD6X1zTVFnGPt9aw8z+klVwrB2V9zV+kSuW?= =?us-ascii?q?jZ+Y2mfJwZVvq4meyWwA=3D=3D?= X-IPAS-Result: =?us-ascii?q?A2HEAwCQ4cZX/wHyM5BdGgEBAQECAQEBARUBAQEBAgEBAQG?= =?us-ascii?q?DCgEBAQEBHld8uiQcC4dDTAEBAQEBAQEBAgECWyeCMgQDEwUFORBVAg1mAhcNE?= =?us-ascii?q?xQgDgMJAhcIIQgIAwEtFRgHCwUYBIglDr19AR8FiC+GXxEBhXgFiC2BdYQCdoo?= =?us-ascii?q?2hiCJEAKJcIVjAkiPeVSCRxuBbVABhD8NFwdagScBAQE?= Received: from unknown (HELO tarius.tycho.ncsc.mil) ([144.51.242.1]) by emsm-gh1-uea10.nsa.gov with ESMTP; 31 Aug 2016 13:59:01 +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 u7VDu2L5024485; Wed, 31 Aug 2016 09:56:14 -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 u7VDtw54128666 for ; Wed, 31 Aug 2016 09:55:58 -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 u7VDtwDV024451; Wed, 31 Aug 2016 09:55:58 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A1DWAwBn4cZX/yNjr8ZdGgEBAQECAQEBAYMpAQEBAQEeV3y2F4QQFBCFeYFKTAECAQEBAQECXoUpDVIwgQ8SiEYOvXEBAQEBBgIgBYgviVcLgwcFiC2BdYQCdoo2hiCJEAKPUwJIj3lUgkcbgW1QAYQ/K4IBAQEB X-IPAS-Result: A1DWAwBn4cZX/yNjr8ZdGgEBAQECAQEBAYMpAQEBAQEeV3y2F4QQFBCFeYFKTAECAQEBAQECXoUpDVIwgQ8SiEYOvXEBAQEBBgIgBYgviVcLgwcFiC2BdYQCdoo2hiCJEAKPUwJIj3lUgkcbgW1QAYQ/K4IBAQEB X-IronPort-AV: E=Sophos;i="5.30,262,1470715200"; d="scan'208";a="5675134" 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; 31 Aug 2016 09:55:56 -0400 IronPort-PHdr: =?us-ascii?q?9a23=3AMjZnWxdB4rtkircT5BJdf2K0lGMj4u6mDksu8pMi?= =?us-ascii?q?zoh2WeGdxc+8Zh7h7PlgxGXEQZ/co6odzbGH6ua8CSdev97B6ClEK80UEUddyI?= =?us-ascii?q?0/pE8JOIa9E0r1LfrnPWQRPf9pcxtbxUy9KlVfA83kZlff8TWY5D8WHQjjZ0Iu?= =?us-ascii?q?frymUrDbg8n/7e2u4ZqbO1wO32vkJ+8iZ0vo5UWJ749N0NMkcv5wgjLy4VJwM9?= =?us-ascii?q?xMwm1pIV/B1z3d3eyXuKBZziJLpvg6/NRBW6ipN44xTLhfESh0ezttvJ6jnVD5?= =?us-ascii?q?QACO/noRVHkN2loNWlCdrULMZZDrrib2jOd22THAdY2qFfFnEQilurxmTB7ulT?= =?us-ascii?q?cvKy8y8GaRjNd5yq1cvlbpvBF2xYLOZ4CZcf5/Zb/1YcIRRW0HWN1YESNGHMf0?= =?us-ascii?q?dIcUJ/YQNuZf6Y/mrh0BqgX6TQuzD+r11mVgmm793ap81f8oVw7Bwl8OBdUL5W?= =?us-ascii?q?vVqNH0PaJUWqavy6PF1ynYR/JQxTr5roPPd0Ny6cqQVK59JJKCgXIkEBnI2xDO?= =?us-ascii?q?8YE=3D?= X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0HCAwAt4cZX/yNjr8ZdGgEBAQECAQEBA?= =?us-ascii?q?RUBAQEBAgEBAQGDCgEBAQEBHld8theEEBQQhXmBSkwBAQEBAQEBAQIBAlsngjI?= =?us-ascii?q?EARUFBTkQVQINfw1SMIEPEohGDr1xAQEBAQYCAR8FiC+JVwuDBwWILYF1hAJ2i?= =?us-ascii?q?jaGIIkQAo9TAkiPeVSCRxuBbVABhD8rQYFAAQEB?= X-IPAS-Result: =?us-ascii?q?A0HCAwAt4cZX/yNjr8ZdGgEBAQECAQEBARUBAQEBAgEBAQG?= =?us-ascii?q?DCgEBAQEBHld8theEEBQQhXmBSkwBAQEBAQEBAQIBAlsngjIEARUFBTkQVQINf?= =?us-ascii?q?w1SMIEPEohGDr1xAQEBAQYCAR8FiC+JVwuDBwWILYF1hAJ2ijaGIIkQAo9TAki?= =?us-ascii?q?PeVSCRxuBbVABhD8rQYFAAQEB?= X-IronPort-AV: E=Sophos;i="5.30,262,1470700800"; d="scan'208";a="18892344" Received: from fmsmga002-icc.fm.intel.com ([198.175.99.35]) by emsm-gh1-uea11.nsa.gov with ESMTP; 31 Aug 2016 13:55:53 +0000 Received: from fmsmga001-icc.fm.intel.com ([198.175.99.7]) by fmsmga002-icc.fm.intel.com with ESMTP; 31 Aug 2016 06:55:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.30,262,1470726000"; d="scan'208"; a="1033621523" Received: from wcrobert-mobl1.sc.intel.com ([10.232.0.127]) by fmsmga001.fm.intel.com with ESMTP; 31 Aug 2016 06:55:50 -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] [RFC] nodups_specs: speedup Date: Wed, 31 Aug 2016 06:55:46 -0700 Message-Id: <1472651746-4681-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 I noticed, via gprof, that the time spent in nodups_specs() accounts for 100% of the label_open() call. It seems as though the N^2 comparison using strcmp is very slow. Do two major things: 1. move the rec->validating check to the left, check the simple thing first before runnning the expensive strcmp(). strcmp() is used to check string equality, check lengths first. 2. strlen() is used all over to calculate lengths, just store it in a struct with the string so its usable elsewhere, rather than recalculating it. text 21.4% speedup: before text: 248 after text: 195 binary 24.6% speedup: before bin: 236 after bin: 178 Some things to ponder: 1. We can use C ABI safe pointer instead of len_str structure https://bitbucket.org/billcroberts/twist There are pros and cons to this approach, namely if someone calls free(x) instead of twist_free(x) It also currently has 0 support for stack based strings (simple enough to add). I think this approach is overkill here. 2. The location of the str_len struct and routines should likely move elsewhere. 3. The impact on Android is currently unmeasured, that's next. Also, bionic uses something from label_file.h for processing property_contexts for the Android property subsystem... so need to ensure that all works as advertised still. Things to do: 1. Cleanup the code locations, likely a util.h or a len_str.h, a better name would be nice. 2. Spell check this commit message Signed-off-by: William Roberts --- libselinux/src/label.c | 8 ++-- libselinux/src/label_android_property.c | 44 +++++++++---------- libselinux/src/label_db.c | 7 +-- libselinux/src/label_file.c | 63 ++++++++++++++------------ libselinux/src/label_file.h | 78 +++++++++++++++++++-------------- libselinux/src/label_internal.h | 22 +++++++++- libselinux/src/label_media.c | 5 ++- libselinux/src/label_support.c | 32 ++++++++------ libselinux/src/label_x.c | 5 ++- libselinux/src/matchpathcon.c | 2 +- 10 files changed, 159 insertions(+), 107 deletions(-) diff --git a/libselinux/src/label.c b/libselinux/src/label.c index 963bfcb..11324ea 100644 --- a/libselinux/src/label.c +++ b/libselinux/src/label.c @@ -209,7 +209,7 @@ int selabel_validate(struct selabel_handle *rec, if (!rec->validating || contexts->validated) goto out; - rc = selinux_validate(&contexts->ctx_raw); + rc = selinux_validate(&contexts->ctx_raw.str); if (rc < 0) goto out; @@ -248,7 +248,7 @@ static int selabel_fini(struct selabel_handle *rec, return -1; if (translating && !lr->ctx_trans && - selinux_raw_to_trans_context(lr->ctx_raw, &lr->ctx_trans)) + selinux_raw_to_trans_context(lr->ctx_raw.str, &lr->ctx_trans)) return -1; return 0; @@ -369,7 +369,7 @@ int selabel_lookup_raw(struct selabel_handle *rec, char **con, if (!lr) return -1; - *con = strdup(lr->ctx_raw); + *con = strdup(lr->ctx_raw.str); return *con ? 0 : -1; } @@ -429,7 +429,7 @@ int selabel_lookup_best_match_raw(struct selabel_handle *rec, char **con, if (!lr) return -1; - *con = strdup(lr->ctx_raw); + *con = strdup(lr->ctx_raw.str); return *con ? 0 : -1; } diff --git a/libselinux/src/label_android_property.c b/libselinux/src/label_android_property.c index 290b438..af0b9a8 100644 --- a/libselinux/src/label_android_property.c +++ b/libselinux/src/label_android_property.c @@ -16,7 +16,7 @@ /* A property security context specification. */ typedef struct spec { struct selabel_lookup_rec lr; /* holds contexts for lookup result */ - char *property_key; /* property key string */ + struct len_str property_key; /* property key string */ } spec_t; /* Our stored configuration */ @@ -33,13 +33,13 @@ static int cmp(const void *A, const void *B) { const struct spec *sp1 = A, *sp2 = B; - if (strncmp(sp1->property_key, "*", 1) == 0) + if (strncmp(sp1->property_key.str, "*", 1) == 0) return 1; - if (strncmp(sp2->property_key, "*", 1) == 0) + if (strncmp(sp2->property_key.str, "*", 1) == 0) return -1; - size_t L1 = strlen(sp1->property_key); - size_t L2 = strlen(sp2->property_key); + size_t L1 = sp1->property_key.len; + size_t L2 = sp2->property_key.len; return (L1 < L2) - (L1 > L2); } @@ -56,23 +56,23 @@ static int nodups_specs(struct saved_data *data, const char *path) for (ii = 0; ii < data->nspec; ii++) { curr_spec = &spec_arr[ii]; for (jj = ii + 1; jj < data->nspec; jj++) { - if (!strcmp(spec_arr[jj].property_key, - curr_spec->property_key)) { + if (len_str_eq(&spec_arr[jj].property_key, + &curr_spec->property_key)) { rc = -1; errno = EINVAL; - if (strcmp(spec_arr[jj].lr.ctx_raw, - curr_spec->lr.ctx_raw)) { + if (!len_str_eq(&spec_arr[jj].lr.ctx_raw, + &curr_spec->lr.ctx_raw)) { selinux_log (SELINUX_ERROR, "%s: Multiple different specifications for %s (%s and %s).\n", - path, curr_spec->property_key, - spec_arr[jj].lr.ctx_raw, - curr_spec->lr.ctx_raw); + path, curr_spec->property_key.str, + spec_arr[jj].lr.ctx_raw.str, + curr_spec->lr.ctx_raw.str); } else { selinux_log (SELINUX_ERROR, "%s: Multiple same specifications for %s.\n", - path, curr_spec->property_key); + path, curr_spec->property_key.str); } } } @@ -85,7 +85,7 @@ static int process_line(struct selabel_handle *rec, int pass, unsigned lineno) { int items; - char *prop = NULL, *context = NULL; + struct len_str *prop = { 0 }, *context = { 0 }; struct saved_data *data = (struct saved_data *)rec->data; spec_t *spec_arr = data->spec_arr; unsigned int nspec = data->nspec; @@ -118,14 +118,14 @@ static int process_line(struct selabel_handle *rec, free(context); } else if (pass == 1) { /* On the second pass, process and store the specification in spec. */ - spec_arr[nspec].property_key = prop; - spec_arr[nspec].lr.ctx_raw = context; + memcpy(&spec_arr[nspec].property_key, prop, sizeof(*prop)); + memcpy(&spec_arr[nspec].lr.ctx_raw, context, sizeof(*context)); if (rec->validating) { if (selabel_validate(rec, &spec_arr[nspec].lr) < 0) { selinux_log(SELINUX_ERROR, "%s: line %u has invalid context %s\n", - path, lineno, spec_arr[nspec].lr.ctx_raw); + path, lineno, spec_arr[nspec].lr.ctx_raw.str); errno = EINVAL; return -1; } @@ -233,8 +233,8 @@ static void closef(struct selabel_handle *rec) for (i = 0; i < data->nspec; i++) { spec = &data->spec_arr[i]; - free(spec->property_key); - free(spec->lr.ctx_raw); + free(spec->property_key.str); + free(spec->lr.ctx_raw.str); free(spec->lr.ctx_trans); } @@ -259,11 +259,11 @@ static struct selabel_lookup_rec *lookup(struct selabel_handle *rec, } for (i = 0; i < data->nspec; i++) { - if (strncmp(spec_arr[i].property_key, key, - strlen(spec_arr[i].property_key)) == 0) { + if (strncmp(spec_arr[i].property_key.str, key, + spec_arr[i].property_key.len) == 0) { break; } - if (strncmp(spec_arr[i].property_key, "*", 1) == 0) + if (strncmp(spec_arr[i].property_key.str, "*", 1) == 0) break; } diff --git a/libselinux/src/label_db.c b/libselinux/src/label_db.c index 1155bcc..c700aca 100644 --- a/libselinux/src/label_db.c +++ b/libselinux/src/label_db.c @@ -153,7 +153,8 @@ process_line(const char *path, char *line_buf, unsigned int line_num, free(type); spec->key = key; - spec->lr.ctx_raw = context; + spec->lr.ctx_raw.str = context; + spec->lr.ctx_raw.len = strlen(context); catalog->nspec++; @@ -181,7 +182,7 @@ db_close(struct selabel_handle *rec) for (i = 0; i < catalog->nspec; i++) { spec = &catalog->specs[i]; free(spec->key); - free(spec->lr.ctx_raw); + free(spec->lr.ctx_raw.str); free(spec->lr.ctx_trans); } free(catalog); @@ -333,7 +334,7 @@ out_error: spec_t *spec = &catalog->specs[i]; free(spec->key); - free(spec->lr.ctx_raw); + free(spec->lr.ctx_raw.str); free(spec->lr.ctx_trans); } free(catalog); diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c index 68c566b..ade07d8 100644 --- a/libselinux/src/label_file.c +++ b/libselinux/src/label_file.c @@ -71,25 +71,24 @@ static int nodups_specs(struct saved_data *data, const char *path) for (ii = 0; ii < data->nspec; ii++) { curr_spec = &spec_arr[ii]; for (jj = ii + 1; jj < data->nspec; jj++) { - if ((!strcmp(spec_arr[jj].regex_str, - curr_spec->regex_str)) + if (len_str_eq(&spec_arr[jj].regex_str, &curr_spec->regex_str) && (!spec_arr[jj].mode || !curr_spec->mode || spec_arr[jj].mode == curr_spec->mode)) { rc = -1; errno = EINVAL; - if (strcmp(spec_arr[jj].lr.ctx_raw, - curr_spec->lr.ctx_raw)) { + if (!len_str_eq(&spec_arr[jj].lr.ctx_raw, + &curr_spec->lr.ctx_raw)) { COMPAT_LOG (SELINUX_ERROR, "%s: Multiple different specifications for %s (%s and %s).\n", - path, curr_spec->regex_str, - spec_arr[jj].lr.ctx_raw, - curr_spec->lr.ctx_raw); + path, curr_spec->regex_str.str, + spec_arr[jj].lr.ctx_raw.str, + curr_spec->lr.ctx_raw.str); } else { COMPAT_LOG (SELINUX_ERROR, "%s: Multiple same specifications for %s.\n", - path, curr_spec->regex_str); + path, curr_spec->regex_str.str); } } } @@ -475,12 +474,13 @@ static int read_binary_file(struct selabel_handle *rec) goto out; } - spec->lr.ctx_raw = str_buf; + spec->lr.ctx_raw.str = str_buf; + spec->lr.ctx_raw.len = entry_len -1; - if (strcmp(spec->lr.ctx_raw, "<>") && rec->validating) { + if (rec->validating && strcmp(spec->lr.ctx_raw.str, "<>")) { if (selabel_validate(rec, &spec->lr) < 0) { selinux_log(SELINUX_ERROR, "%s: context %s is invalid\n", - data->path, spec->lr.ctx_raw); + data->path, spec->lr.ctx_raw.str); errno = EINVAL; free(str_buf); goto out; @@ -492,14 +492,16 @@ static int read_binary_file(struct selabel_handle *rec) if (err < 0 || !entry_len) goto out; - spec->regex_str = (char *) map_area->next_addr; + spec->regex_str.str = (char *) map_area->next_addr; err = next_entry(NULL, map_area, entry_len); if (err < 0) goto out; - if (spec->regex_str[entry_len - 1] != '\0') + if (spec->regex_str.str[entry_len - 1] != '\0') goto out; + spec->regex_str.len = entry_len - 1; + /* Process mode */ if (version >= SELINUX_COMPILED_FCONTEXT_MODE) err = next_entry(&mode, map_area, sizeof(mode)); @@ -715,11 +717,13 @@ static void closef(struct selabel_handle *rec) for (i = 0; i < data->nspec; i++) { spec = &data->spec_arr[i]; free(spec->lr.ctx_trans); - free(spec->lr.ctx_raw); + free(spec->lr.ctx_raw.str); if (spec->from_mmap) continue; - free(spec->regex_str); - free(spec->type_str); + + free(spec->regex_str.str); + free(spec->type_str.str); + if (spec->regcomp) { pcre_free(spec->regex); pcre_free_study(spec->sd); @@ -767,6 +771,11 @@ static struct spec *lookup_common(struct selabel_handle *rec, const char *prev_slash, *next_slash; unsigned int sofar = 0; + struct len_str none = { + .str = (char *)"<>", + .len = 8 + }; + if (!data->nspec) { errno = ENOENT; goto finish; @@ -834,7 +843,7 @@ static struct spec *lookup_common(struct selabel_handle *rec, } } - if (i < 0 || strcmp(spec_arr[i].lr.ctx_raw, "<>") == 0) { + if (i < 0 || len_str_eq(&spec_arr[i].lr.ctx_raw, &none)) { /* No matching specification. */ errno = ENOENT; goto finish; @@ -926,8 +935,8 @@ static enum selabel_cmp_result incomp(struct spec *spec1, struct spec *spec2, co selinux_log(SELINUX_INFO, "selabel_cmp: mismatched %s on entry %d: (%s, %x, %s) vs entry %d: (%s, %x, %s)\n", reason, - i, spec1->regex_str, spec1->mode, spec1->lr.ctx_raw, - j, spec2->regex_str, spec2->mode, spec2->lr.ctx_raw); + i, spec1->regex_str.str, spec1->mode, spec1->lr.ctx_raw.str, + j, spec2->regex_str.str, spec2->mode, spec2->lr.ctx_raw.str); return SELABEL_INCOMPARABLE; } @@ -976,7 +985,7 @@ static enum selabel_cmp_result cmp(struct selabel_handle *h1, memcmp(spec1->regex, spec2->regex, len1)) return incomp(spec1, spec2, "regex", i, j); } else { - if (strcmp(spec1->regex_str, spec2->regex_str)) + if (strcmp(spec1->regex_str.str, spec2->regex_str.str)) return incomp(spec1, spec2, "regex_str", i, j); } @@ -995,7 +1004,7 @@ static enum selabel_cmp_result cmp(struct selabel_handle *h1, return incomp(spec1, spec2, "stem", i, j); } - if (strcmp(spec1->lr.ctx_raw, spec2->lr.ctx_raw)) + if (!len_str_eq(&spec1->lr.ctx_raw, &spec2->lr.ctx_raw)) return incomp(spec1, spec2, "ctx_raw", i, j); i++; @@ -1020,17 +1029,17 @@ static void stats(struct selabel_handle *rec) for (i = 0; i < nspec; i++) { if (spec_arr[i].matches == 0) { - if (spec_arr[i].type_str) { + if (spec_arr[i].type_str.str) { COMPAT_LOG(SELINUX_WARNING, "Warning! No matches for (%s, %s, %s)\n", - spec_arr[i].regex_str, - spec_arr[i].type_str, - spec_arr[i].lr.ctx_raw); + spec_arr[i].regex_str.str, + spec_arr[i].type_str.str, + spec_arr[i].lr.ctx_raw.str); } else { COMPAT_LOG(SELINUX_WARNING, "Warning! No matches for (%s, %s)\n", - spec_arr[i].regex_str, - spec_arr[i].lr.ctx_raw); + spec_arr[i].regex_str.str, + spec_arr[i].lr.ctx_raw.str); } } } diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h index da06164..cd021f4 100644 --- a/libselinux/src/label_file.h +++ b/libselinux/src/label_file.h @@ -26,10 +26,10 @@ /* A file security context specification. */ struct spec { - struct selabel_lookup_rec lr; /* holds contexts for lookup result */ - char *regex_str; /* regular expession string for diagnostics */ - char *type_str; /* type string for diagnostic messages */ - pcre *regex; /* compiled regular expression */ + struct selabel_lookup_rec lr; /* holds contexts for lookup result */ + struct len_str regex_str; /* regular expession string for diagnostics */ + struct len_str type_str; /* type string for diagnostic messages */ + pcre *regex; /* compiled regular expression */ union { pcre_extra *sd; /* pointer to extra compiled stuff */ pcre_extra lsd; /* used to hold the mmap'd version */ @@ -90,16 +90,19 @@ static inline pcre_extra *get_pcre_extra(struct spec *spec) return spec->sd; } -static inline mode_t string_to_mode(char *mode) +static inline mode_t string_to_mode(struct len_str *mode) { size_t len; if (!mode) return 0; - len = strlen(mode); - if (mode[0] != '-' || len != 2) + + len = mode->len; + + if (mode->str[0] != '-' || len != 2) return -1; - switch (mode[1]) { + + switch (mode->str[1]) { case 'b': return S_IFBLK; case 'c': @@ -153,8 +156,8 @@ static inline void spec_hasMetaChars(struct spec *spec) int len; char *end; - c = spec->regex_str; - len = strlen(spec->regex_str); + c = spec->regex_str.str; + len = spec->regex_str.len; end = c + len; spec->hasMetaChars = 0; @@ -175,7 +178,7 @@ static inline void spec_hasMetaChars(struct spec *spec) case '(': case '{': spec->hasMetaChars = 1; - spec->prefix_len = c - spec->regex_str; + spec->prefix_len = c - spec->regex_str.str; return; case '\\': /* skip the next character */ c++; @@ -327,13 +330,16 @@ static inline int compile_regex(struct saved_data *data, struct spec *spec, if (spec->regcomp) return 0; /* already done */ + len = spec->regex_str.len; + reg_buf = spec->regex_str.str; + /* Skip the fixed stem. */ - reg_buf = spec->regex_str; - if (spec->stem_id >= 0) + if (spec->stem_id >= 0) { reg_buf += stem_arr[spec->stem_id].len; + len -= stem_arr[spec->stem_id].len; + } /* Anchor the regular expression. */ - len = strlen(reg_buf); cp = anchored_regex = malloc(len + 3); if (!anchored_regex) return -1; @@ -375,7 +381,15 @@ static inline int process_line(struct selabel_handle *rec, char *line_buf, unsigned lineno) { int items, len, rc; - char *regex = NULL, *type = NULL, *context = NULL; + + struct len_str regex = { 0 } , type = { 0 }, context = { 0 }; + + /* XXX Export this as a const */ + struct len_str none = { + .str = (char *)"<>", + .len = 8 + }; + struct saved_data *data = (struct saved_data *)rec->data; struct spec *spec_arr; unsigned int nspec = data->nspec; @@ -399,21 +413,21 @@ static inline int process_line(struct selabel_handle *rec, "%s: line %u is missing fields\n", path, lineno); if (items == 1) - free(regex); + free(regex.str); errno = EINVAL; return -1; } else if (items == 2) { /* The type field is optional. */ context = type; - type = 0; + memset(&type, 0, sizeof(type)); } - len = get_stem_from_spec(regex); - if (len && prefix && strncmp(prefix, regex, len)) { + len = get_stem_from_spec(regex.str); + if (len && prefix && strncmp(prefix, regex.str, len)) { /* Stem of regex does not match requested prefix, discard. */ - free(regex); - free(type); - free(context); + free(regex.str); + free(type.str); + free(context.str); return 0; } @@ -424,13 +438,13 @@ static inline int process_line(struct selabel_handle *rec, spec_arr = data->spec_arr; /* process and store the specification in spec. */ - spec_arr[nspec].stem_id = find_stem_from_spec(data, regex); - spec_arr[nspec].regex_str = regex; - - spec_arr[nspec].type_str = type; + spec_arr[nspec].stem_id = find_stem_from_spec(data, regex.str); + memcpy(&spec_arr[nspec].regex_str, ®ex, sizeof(regex)); + memcpy(&spec_arr[nspec].lr.ctx_raw, &context, sizeof(context)); spec_arr[nspec].mode = 0; - spec_arr[nspec].lr.ctx_raw = context; + if (type.str) + memcpy(&spec_arr[nspec].type_str, &type, sizeof(type)); /* * bump data->nspecs to cause closef() to cover it in its free @@ -442,19 +456,19 @@ static inline int process_line(struct selabel_handle *rec, compile_regex(data, &spec_arr[nspec], &errbuf)) { COMPAT_LOG(SELINUX_ERROR, "%s: line %u has invalid regex %s: %s\n", - path, lineno, regex, + path, lineno, regex.str, (errbuf ? errbuf : "out of memory")); errno = EINVAL; return -1; } - if (type) { - mode_t mode = string_to_mode(type); + if (type.str) { + mode_t mode = string_to_mode(&type); if (mode == (mode_t)-1) { COMPAT_LOG(SELINUX_ERROR, "%s: line %u has invalid file type %s\n", - path, lineno, type); + path, lineno, type.str); errno = EINVAL; return -1; } @@ -465,7 +479,7 @@ static inline int process_line(struct selabel_handle *rec, * any meta characters in the RE */ spec_hasMetaChars(&spec_arr[nspec]); - if (strcmp(context, "<>") && rec->validating) + if (rec->validating && !len_str_eq(&context, &none)) compat_validate(rec, &spec_arr[nspec].lr, path, lineno); return 0; diff --git a/libselinux/src/label_internal.h b/libselinux/src/label_internal.h index aa48fff..51aa33d 100644 --- a/libselinux/src/label_internal.h +++ b/libselinux/src/label_internal.h @@ -71,8 +71,28 @@ extern struct selabel_sub *selabel_subs_init(const char *path, struct selabel_sub *list, struct selabel_digest *digest); +/* + * A simple struct for tracking string lengths + * with strings. + */ +struct len_str { + size_t len; + char *str; +}; + +static inline bool len_str_eq(struct len_str *a, struct len_str *b) +{ + if (a->len != b->len) + return false; + + if (a->str == b->str) + return true; + + return !strcmp(a->str, b->str); +} + struct selabel_lookup_rec { - char * ctx_raw; + struct len_str ctx_raw; char * ctx_trans; int validated; }; diff --git a/libselinux/src/label_media.c b/libselinux/src/label_media.c index 622741b..e85e973 100644 --- a/libselinux/src/label_media.c +++ b/libselinux/src/label_media.c @@ -56,7 +56,8 @@ static int process_line(const char *path, char *line_buf, int pass, if (pass == 1) { data->spec_arr[data->nspec].key = key; - data->spec_arr[data->nspec].lr.ctx_raw = context; + data->spec_arr[data->nspec].lr.ctx_raw.str = context; + data->spec_arr[data->nspec].lr.ctx_raw.len = strlen(context); } data->nspec++; @@ -159,7 +160,7 @@ static void close(struct selabel_handle *rec) for (i = 0; i < data->nspec; i++) { spec = &spec_arr[i]; free(spec->key); - free(spec->lr.ctx_raw); + free(spec->lr.ctx_raw.str); free(spec->lr.ctx_trans); } diff --git a/libselinux/src/label_support.c b/libselinux/src/label_support.c index 26f9ef1..e9b842f 100644 --- a/libselinux/src/label_support.c +++ b/libselinux/src/label_support.c @@ -22,16 +22,17 @@ * errno will be set. * */ -static inline int read_spec_entry(char **entry, char **ptr, int *len, const char **errbuf) +static inline int read_spec_entry(struct len_str *entry, char **ptr, const char **errbuf) { - *entry = NULL; + size_t len = 0; char *tmp_buf = NULL; + memset(entry, 0, sizeof(*entry)); + while (isspace(**ptr) && **ptr != '\0') (*ptr)++; tmp_buf = *ptr; - *len = 0; while (!isspace(**ptr) && **ptr != '\0') { if (!isascii(**ptr)) { @@ -40,13 +41,15 @@ static inline int read_spec_entry(char **entry, char **ptr, int *len, const char return -1; } (*ptr)++; - (*len)++; + len++; } - if (*len) { - *entry = strndup(tmp_buf, *len); - if (!*entry) + if (len) { + entry->str = strndup(tmp_buf, len); + if (!entry->str) return -1; + + entry->len = len; } return 0; @@ -56,7 +59,7 @@ static inline int read_spec_entry(char **entry, char **ptr, int *len, const char * line_buf - Buffer containing the spec entries . * errbuf - Double pointer used for passing back specific error messages. * num_args - The number of spec parameter entries to process. - * ... - A 'char **spec_entry' for each parameter. + * ... - A 'struct len_str **spec_entry' for each parameter. * returns - The number of items processed. On error, it returns -1 with errno * set and may set errbuf to a specific error message. * @@ -65,8 +68,9 @@ static inline int read_spec_entry(char **entry, char **ptr, int *len, const char */ int hidden read_spec_entries(char *line_buf, const char **errbuf, int num_args, ...) { - char **spec_entry, *buf_p; - int len, rc, items, entry_len = 0; + struct len_str *spec_entry; + char *buf_p; + int len, rc, items; va_list ap; *errbuf = NULL; @@ -93,20 +97,22 @@ int hidden read_spec_entries(char *line_buf, const char **errbuf, int num_args, items = 0; while (items < num_args) { - spec_entry = va_arg(ap, char **); + spec_entry = va_arg(ap, struct len_str *); if (len - 1 == buf_p - line_buf) { va_end(ap); return items; } - rc = read_spec_entry(spec_entry, &buf_p, &entry_len, errbuf); + rc = read_spec_entry(spec_entry, &buf_p, errbuf); if (rc < 0) { va_end(ap); return rc; } - if (entry_len) + + if (spec_entry->len) { items++; + } } va_end(ap); return items; diff --git a/libselinux/src/label_x.c b/libselinux/src/label_x.c index 700def1..cdbac8c 100644 --- a/libselinux/src/label_x.c +++ b/libselinux/src/label_x.c @@ -81,7 +81,8 @@ static int process_line(const char *path, char *line_buf, int pass, return 0; } data->spec_arr[data->nspec].key = key; - data->spec_arr[data->nspec].lr.ctx_raw = context; + data->spec_arr[data->nspec].lr.ctx_raw.str = context; + data->spec_arr[data->nspec].lr.ctx_raw.len = strlen(context); free(type); } @@ -186,7 +187,7 @@ static void close(struct selabel_handle *rec) for (i = 0; i < data->nspec; i++) { spec = &spec_arr[i]; free(spec->key); - free(spec->lr.ctx_raw); + free(spec->lr.ctx_raw.str); free(spec->lr.ctx_trans); } diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c index 4764ab7..96c39b4 100644 --- a/libselinux/src/matchpathcon.c +++ b/libselinux/src/matchpathcon.c @@ -541,7 +541,7 @@ int compat_validate(struct selabel_handle *rec, const char *path, unsigned lineno) { int rc; - char **ctx = &contexts->ctx_raw; + char **ctx = &contexts->ctx_raw.str; if (myinvalidcon) rc = myinvalidcon(path, lineno, *ctx);