From patchwork Tue Nov 5 21:06:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Christian_G=C3=B6ttsche?= X-Patchwork-Id: 13863542 X-Patchwork-Delegate: plautrba@redhat.com Received: from server02.seltendoof.de (server02.seltendoof.de [168.119.48.163]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 05381215C7D for ; Tue, 5 Nov 2024 21:06:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=168.119.48.163 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730840789; cv=none; b=Xp70g8mdI5zM8Zv9iulRdsT7x444N+B444WG1VHaxdTYjObRS+qNqxA3yFBuvBT4Y1wuYOoSL1w7Li91jZ56ckPewWZU5ZUndx9H1bJv0xjO0J1au3ujbAgehdxCKTG0zs1D75b2DvxMm0fWwJ31cIxJgJBWFTtjqxk2IK8U1tQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730840789; c=relaxed/simple; bh=UqZOgpgDSwaWrimtTgU859k5NgX5VGMulVo3nwMa0A4=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type; b=jNoU8jK2Vyrj00K+sxvbetN4oPC3kVDpFpfENEpG5BYwcVEITkaVmSOSCyE48AE7rKh9Ysapm9kMkYdSRijgBF6hiB6FlMcg3sI3vYpJ+zS2KdKEFZ5VbyxCkoI//rbM+l6Ckot0fTAXs5N3jUFj3CV1fXSYmDVLnKDLeFI1pmo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=seltendoof.de; spf=pass smtp.mailfrom=seltendoof.de; dkim=pass (2048-bit key) header.d=seltendoof.de header.i=@seltendoof.de header.b=a83n0MbA; arc=none smtp.client-ip=168.119.48.163 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=seltendoof.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=seltendoof.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=seltendoof.de header.i=@seltendoof.de header.b="a83n0MbA" From: =?utf-8?q?Christian_G=C3=B6ttsche?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=seltendoof.de; s=2023072701; t=1730840781; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=mxCyGb4rGlyJ92DD2hRJlZgKGlWSNV58Ovr7ihpyefs=; b=a83n0MbAPTqhx5lsYUJT/EEe2a/9JU3ukdWAFZ11J4XQAnac1rMfIz3Hl6jLgdaLW2Pk6W 3Nxh8KNASsMhKnuOapYtYwa2F1M1sWZhw93tmgbwceLXDG2ecBVsdtTaZ1Kyfl5ziXV4dP pjdnz/46H7i08CeOZGOVDLtbmZUpeUtNZBm2tFgL6/FWlgznL0XPMOIeVuzjieKa4P7qbI StBWVJIGlHrUUnKQP2SjCNDWhwWG3yUPGznY05cd3FL7cWLMRYtJ6s6oTfKsLhHOJ/KUCE PXYNbBiSsGqK+ZuXjrEst2blPPH2/+KLoGQ9HudyCDA4K2MAFn4OetH32BnHfA== To: selinux@vger.kernel.org Cc: =?utf-8?q?Christian_G=C3=B6ttsche?= Subject: [PATCH] checkpolicy: avoid memory leaks on redeclarations Date: Tue, 5 Nov 2024 22:06:18 +0100 Message-ID: <20241105210618.326533-1-cgoettsche@seltendoof.de> Reply-To: cgzones@googlemail.com Precedence: bulk X-Mailing-List: selinux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Christian Göttsche If declare_symbol() returns 1 the id and the datum are already defined and not consumed by the function, so it they must be free'd by the caller. Example policy (generated by fuzzer): class s sid e class s{i}optional{require{bool K;}bool K true; Reported-by: oss-fuzz (issue 377544445) Signed-off-by: Christian Göttsche Acked-by: James Carter --- checkpolicy/module_compiler.c | 2 +- checkpolicy/module_compiler.h | 2 +- checkpolicy/policy_define.c | 72 +++++++++++++++++++++++------------ 3 files changed, 49 insertions(+), 27 deletions(-) diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c index 4efd77bf..3a7ad1bb 100644 --- a/checkpolicy/module_compiler.c +++ b/checkpolicy/module_compiler.c @@ -190,7 +190,7 @@ static int create_symbol(uint32_t symbol_type, hashtab_key_t key, hashtab_datum_ * not be restricted pointers. */ int declare_symbol(uint32_t symbol_type, hashtab_key_t key, hashtab_datum_t datum, - uint32_t * dest_value, uint32_t * datum_value) + uint32_t * dest_value, const uint32_t * datum_value) { avrule_decl_t *decl = stack_top->decl; int ret = create_symbol(symbol_type, key, datum, dest_value, SCOPE_DECL); diff --git a/checkpolicy/module_compiler.h b/checkpolicy/module_compiler.h index e43bc6c0..2fe3455d 100644 --- a/checkpolicy/module_compiler.h +++ b/checkpolicy/module_compiler.h @@ -28,7 +28,7 @@ int define_policy(int pass, int module_header_given); */ int declare_symbol(uint32_t symbol_type, hashtab_key_t key, hashtab_datum_t datum, - uint32_t * dest_value, uint32_t * datum_value); + uint32_t * dest_value, const uint32_t * datum_value); role_datum_t *declare_role(unsigned char isattr); type_datum_t *declare_type(unsigned char primary, unsigned char isattr); diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index f8a10154..9aae8378 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -156,9 +156,9 @@ static int id_has_dot(const char *id) int define_class(void) { - char *id = 0; - class_datum_t *datum = 0; - int ret; + char *id = NULL; + class_datum_t *datum = NULL; + int ret = -1; uint32_t value; if (pass == 2) { @@ -175,27 +175,29 @@ int define_class(void) datum = (class_datum_t *) malloc(sizeof(class_datum_t)); if (!datum) { yyerror("out of memory"); - goto bad; + goto cleanup; } memset(datum, 0, sizeof(class_datum_t)); ret = declare_symbol(SYM_CLASSES, id, datum, &value, &value); switch (ret) { case -3:{ yyerror("Out of memory!"); - goto bad; + goto cleanup; } case -2:{ yyerror2("duplicate declaration of class %s", id); - goto bad; + goto cleanup; } case -1:{ yyerror("could not declare class here"); - goto bad; + goto cleanup; } - case 0: - case 1:{ + case 0: { break; } + case 1:{ + goto cleanup; + } default:{ assert(0); /* should never get here */ } @@ -203,12 +205,10 @@ int define_class(void) datum->s.value = value; return 0; - bad: - if (id) - free(id); - if (datum) - free(datum); - return -1; + cleanup: + free(id); + free(datum); + return ret == 1 ? 0 : -1; } int define_permissive(void) @@ -785,8 +785,13 @@ int define_sens(void) yyerror2("could not declare sensitivity level %s here", id); goto bad; } - case 0: + case 0: { + break; + } case 1:{ + level_datum_destroy(datum); + free(datum); + free(id); break; } default:{ @@ -827,8 +832,13 @@ int define_sens(void) ("could not declare sensitivity alias %s here", id); goto bad_alias; } - case 0: + case 0: { + break; + } case 1:{ + level_datum_destroy(aliasdatum); + free(aliasdatum); + free(id); break; } default:{ @@ -957,15 +967,20 @@ int define_category(void) yyerror2("could not declare category %s here", id); goto bad; } - case 0: + case 0:{ + datum->s.value = value; + break; + } case 1:{ + cat_datum_destroy(datum); + free(datum); + free(id); break; } default:{ assert(0); /* should never get here */ } } - datum->s.value = value; while ((id = queue_remove(id_queue))) { if (id_has_dot(id)) { @@ -981,11 +996,11 @@ int define_category(void) } cat_datum_init(aliasdatum); aliasdatum->isalias = TRUE; - aliasdatum->s.value = datum->s.value; + aliasdatum->s.value = value; ret = declare_symbol(SYM_CATS, id, aliasdatum, NULL, - &datum->s.value); + &value); switch (ret) { case -3:{ yyerror("Out of memory!"); @@ -1001,8 +1016,13 @@ int define_category(void) ("could not declare category alias %s here", id); goto bad_alias; } - case 0: + case 0:{ + break; + } case 1:{ + cat_datum_destroy(aliasdatum); + free(aliasdatum); + free(id); break; } default:{ @@ -1833,10 +1853,12 @@ int define_bool_tunable(int is_tunable) yyerror2("could not declare boolean %s here", id); goto cleanup; } - case 0: - case 1:{ + case 0:{ break; } + case 1:{ + goto cleanup; + } default:{ assert(0); /* should never get here */ } @@ -1854,7 +1876,7 @@ int define_bool_tunable(int is_tunable) return 0; cleanup: cond_destroy_bool(id, datum, NULL); - return -1; + return ret == 1 ? 0 : -1; } avrule_t *define_cond_pol_list(avrule_t * avlist, avrule_t * sl)