From patchwork Mon Nov 30 11:56:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Ife X-Patchwork-Id: 11940309 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8CEA5C64E90 for ; Mon, 30 Nov 2020 12:05:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 02AE9206D8 for ; Mon, 30 Nov 2020 12:05:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ife.onl header.i=@ife.onl header.b="nsCZQQ1p" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726828AbgK3MFR (ORCPT ); Mon, 30 Nov 2020 07:05:17 -0500 Received: from pub2.ife.onl ([108.61.167.28]:32780 "EHLO mail.ife.onl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726158AbgK3MES (ORCPT ); Mon, 30 Nov 2020 07:04:18 -0500 X-Greylist: delayed 424 seconds by postgrey-1.27 at vger.kernel.org; Mon, 30 Nov 2020 07:04:16 EST Received: from home.home.ife.onl (unknown [IPv6:2a0a:e5c1:18c:2::2]) by mail.ife.onl (Postfix) with ESMTPSA id 1E03E41C13 for ; Mon, 30 Nov 2020 11:56:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ife.onl 1E03E41C13 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ife.onl; s=default; t=1606737389; bh=aNovztkb1OuHXzXyTcQn3DrYAOBznxIQZ4Ti4sq3JXE=; h=Subject:From:To:Date:From; b=nsCZQQ1pCVxe99AOX+VvgWVIVjyLzkjqhwRX6fki6FCdlye7v87qFN+G65UsYIMKM HuyWBfEJxLlBz6T10le5PPh83blSb8fkpQJvrKxK+s+IoNmfEttk9HlbXMumSS5tic 4Lal7I23IdFCGJIRs4xBN4AWEOzuRv1wbkRjXaxs= Message-ID: <30bf5dca94595b9807b2c79be3f2ea9db4feb0cd.camel@ife.onl> Subject: Fix Checkmodule when Writing down to older Module Versions From: Matthew Ife To: selinux@vger.kernel.org Date: Mon, 30 Nov 2020 11:56:27 +0000 User-Agent: Evolution 3.38.1 (3.38.1-1.fc33) MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org Stumbled over this one when writing a module from F33 to EL6. Steps to reproduce: Try to build the following module then make a module from an older release: module test 1.0.0; require { type default_t; } attribute_role new_atrole; Build $ checkmodule -M -m -c 12 -o test.mod test.te $ semodule_package -o test.pp -m test.mod $ semodule_package: Error while reading policy module from test.mod With fix: $ checkmodule -o test.mod -M -m -c12 test.te libsepol.policydb_write: Discarding role attribute rules $ semodule_package -o test.pp -m test.mod Failure occurs when the current module gets written out as the scope declaration remains intact. semodule_package files correctly at policydb.c:3913 doing a hash table search on a scope key that is not in the symbol table. This patch fixes the problem by removing the hashtable entries and scope declarations properly prior to module write and emits a warning to the user of the unsupported statements. Also altered hashtab_map slightly to allow it to be used for hashtab_remove calls in order to support the patch. I submitted a pull request for this (there is some spacing/tabbing issues actually so I can resent a new pull request if necessary). https://github.com/SELinuxProject/selinux/pull/273 diff --git a/libsepol/src/hashtab.c b/libsepol/src/hashtab.c index 76b977a9..ff7ef63f 100644 --- a/libsepol/src/hashtab.c +++ b/libsepol/src/hashtab.c @@ -230,7 +230,7 @@ int hashtab_map(hashtab_t h, for (i = 0; i < h->size; i++) { cur = h->htable[i]; - while (curi != NULL) { + while (cur != NULL) { next = cur->next; ret = apply(cur->key, cur->datum, args); if (ret) diff --git a/libsepol/src/write.c b/libsepol/src/write.c index dd418317..6a59a0c3 100644 --- a/libsepol/src/write.c +++ b/libsepol/src/write.c @@ -2170,7 +2170,7 @@ static void scope_write_destroy(hashtab_key_t key __attribute__ ((unused)), free(cur); } -static void type_attr_filter(hashtab_key_t key, +static int type_attr_filter(hashtab_key_t key, hashtab_datum_t datum, void *args) { type_datum_t *typdatum = datum; @@ -2186,9 +2186,11 @@ static void type_attr_filter(hashtab_key_t key, if (scope) hashtab_remove(scopetbl, key, scope_write_destroy, scope); } + + return 0; } -static void role_attr_filter(hashtab_key_t key, +static int role_attr_filter(hashtab_key_t key, hashtab_datum_t datum, void *args) { role_datum_t *role = datum; @@ -2204,6 +2206,8 @@ static void role_attr_filter(hashtab_key_t key, if (scope) hashtab_remove(scopetbl, key, scope_write_destroy, scope); } + + return 0; } /* @@ -2337,36 +2341,36 @@ int policydb_write(policydb_t * p, struct policy_file *fp) buf[0] = cpu_to_le32(p->symtab[i].nprim); buf[1] = p->symtab[i].table->nel; - /* - * A special case when writing type/attribute symbol table. - * The kernel policy version less than 24 does not support - * to load entries of attribute, so we filter the entries - * from the table. - */ - if (i == SYM_TYPES && - p->policyvers < POLICYDB_VERSION_BOUNDARY && - p->policy_type == POLICY_KERN) { - (void)hashtab_map(p->symtab[i].table, type_attr_filter, p); - if (buf[1] != p->symtab[i].table->nel) + /* + * A special case when writing type/attribute symbol table. + * The kernel policy version less than 24 does not support + * to load entries of attribute, so we filter the entries + * from the table. + */ + if (i == SYM_TYPES && + p->policyvers < POLICYDB_VERSION_BOUNDARY && + p->policy_type == POLICY_KERN) { + (void)hashtab_map(p->symtab[i].table, type_attr_filter, p); + if (buf[1] != p->symtab[i].table->nel) WARN(fp->handle, "Discarding type attribute rules"); - buf[1] = p->symtab[i].table->nel; - } - - /* - * Another special case when writing role/attribute symbol - * table, role attributes are redundant for policy.X, or - * when the pp's version is not big enough. So filter the entries - * from the table. - */ - if ((i == SYM_ROLES) && - ((p->policy_type == POLICY_KERN) || - (p->policy_type != POLICY_KERN && - p->policyvers < MOD_POLICYDB_VERSION_ROLEATTRIB))) { - (void)hashtab_map(p->symtab[i].table, role_attr_filter, p); + buf[1] = p->symtab[i].table->nel; + } + + /* + * Another special case when writing role/attribute symbol + * table, role attributes are redundant for policy.X, or + * when the pp's version is not big enough. So filter the entries + * from the table. + */ + if ((i == SYM_ROLES) && + ((p->policy_type == POLICY_KERN) || + (p->policy_type != POLICY_KERN && + p->policyvers < MOD_POLICYDB_VERSION_ROLEATTRIB))) { + (void)hashtab_map(p->symtab[i].table, role_attr_filter, p); if (buf[1] != p->symtab[i].table->nel) WARN(fp->handle, "Discarding role attribute rules"); - buf[1] = p->symtab[i].table->nel; - } + buf[1] = p->symtab[i].table->nel; + } buf[1] = cpu_to_le32(buf[1]); items = put_entry(buf, sizeof(uint32_t), 2, fp);