From patchwork Mon Sep 9 02:03:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Koichiro Den X-Patchwork-Id: 1982282 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4X29BK1zzTz1y1D for ; Mon, 9 Sep 2024 12:04:25 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1snTlJ-0001q2-93; Mon, 09 Sep 2024 02:04:17 +0000 Received: from smtp-relay-internal-0.internal ([10.131.114.225] helo=smtp-relay-internal-0.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1snTlH-0001pU-At for kernel-team@lists.ubuntu.com; Mon, 09 Sep 2024 02:04:15 +0000 Received: from mail-oi1-f198.google.com (mail-oi1-f198.google.com [209.85.167.198]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id D8CDA3F5AB for ; Mon, 9 Sep 2024 02:04:14 +0000 (UTC) Received: by mail-oi1-f198.google.com with SMTP id 5614622812f47-3de10cb25a9so4310993b6e.2 for ; Sun, 08 Sep 2024 19:04:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725847453; x=1726452253; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=bCtQYm18e8O2EGfwxaYBpA0wveyEBFW6MgLzfL2SA20=; b=H7rFqb/whE8YmURXoliSHdLy1MKCLQFXKsYvrFf2z1ythFQsoYOHu2pMCGEVVgzZKG 1OB0Z0YoMLIGL8N4/Wr8jE1MtZRoXAZQj4hNHstboM3FvtBKkT4+xoNjINYPvyqQ6TE4 BXW4kcRihC0KEvOksJ0Dvw1UJmY5TDQYTnhzvTgWniNYQtrXHzsc/bOo+cdaXz1Wk4bL CwwCv0+2EckLeMoxXfCnfImG10ECchFBPauCVpzMsn0VCE3tounTPyVBPt+lHoKu5gqr oDmLMrLMYOZaOR4pmBp7SIWgBd+K6dRzyf19lZDfvMtC2BO6vAd8xGHulpQY1eGgqObq yqdQ== X-Gm-Message-State: AOJu0YzLCrR53K4MQ5GBy6uQ1KxQ2Xj6UJxNAM+hwpI91uM8Q0Ljre+V 43malOw8E0fKYuqvfC1eLx+ZJzZpXzkTjGdrE/8rYFaCdHQZvNzjOIYcBfWz3SF39SX2nQ+36qA hAgJOZvJav9oE2HN0Bo4bRSSFUii18vNZG0oMCd9I3Wimpze9+JtlFKM5JlEuIBCIYCkuPseo7/ XnXMRp56jamg== X-Received: by 2002:a05:6808:4496:b0:3df:1556:e0b2 with SMTP id 5614622812f47-3e02a03ac9fmr13725760b6e.43.1725847452581; Sun, 08 Sep 2024 19:04:12 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGMQJeQyY8DJJRlgPW/m2+ngNclyKrnuGsSnja9nXDHvfBUAfyuTBrrAFhkXqtwgvtRsfLOHw== X-Received: by 2002:a05:6808:4496:b0:3df:1556:e0b2 with SMTP id 5614622812f47-3e02a03ac9fmr13725731b6e.43.1725847451833; Sun, 08 Sep 2024 19:04:11 -0700 (PDT) Received: from localhost.localdomain ([240f:74:7be:1:8492:650d:69e1:396a]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7d823736497sm2384187a12.10.2024.09.08.19.04.10 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 08 Sep 2024 19:04:11 -0700 (PDT) From: Koichiro Den To: kernel-team@lists.ubuntu.com Subject: [SRU][F][PATCH 1/2] selinux: convert cond_list to array Date: Mon, 9 Sep 2024 11:03:28 +0900 Message-ID: <20240909020347.960015-2-koichiro.den@canonical.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240909020347.960015-1-koichiro.den@canonical.com> References: <20240909020347.960015-1-koichiro.den@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Ondrej Mosnacek Since it is fixed-size after allocation and we know the size beforehand, using a plain old array is simpler and more efficient. While there, also fix signedness of some related variables/parameters. Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore (backported from commit 60abd3181db29ea81742106cc0ac2e27fd05b418) [koichiroden: Adjusted context due to missing commit 06c2efe2cf3a ("selinux: simplify evaluate_cond_node()")] CVE-2022-48740 Signed-off-by: Koichiro Den --- security/selinux/include/conditional.h | 8 ++-- security/selinux/selinuxfs.c | 4 +- security/selinux/ss/conditional.c | 54 ++++++++++---------------- security/selinux/ss/conditional.h | 3 +- security/selinux/ss/policydb.c | 2 +- security/selinux/ss/policydb.h | 3 +- security/selinux/ss/services.c | 28 ++++++------- 7 files changed, 43 insertions(+), 59 deletions(-) diff --git a/security/selinux/include/conditional.h b/security/selinux/include/conditional.h index 0ab316f61da0..539ab357707d 100644 --- a/security/selinux/include/conditional.h +++ b/security/selinux/include/conditional.h @@ -14,12 +14,10 @@ #include "security.h" int security_get_bools(struct selinux_state *state, - int *len, char ***names, int **values); + u32 *len, char ***names, int **values); -int security_set_bools(struct selinux_state *state, - int len, int *values); +int security_set_bools(struct selinux_state *state, u32 len, int *values); -int security_get_bool_value(struct selinux_state *state, - int index); +int security_get_bool_value(struct selinux_state *state, u32 index); #endif diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index e9eaff90cbcc..eca74bab4a12 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1325,14 +1325,14 @@ static void sel_remove_entries(struct dentry *de) static int sel_make_bools(struct selinux_fs_info *fsi) { - int i, ret; + int ret; ssize_t len; struct dentry *dentry = NULL; struct dentry *dir = fsi->bool_dir; struct inode *inode = NULL; struct inode_security_struct *isec; char **names = NULL, *page; - int num; + u32 i, num; int *values = NULL; u32 sid; diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index 70c378ee1a2f..be1be450fc57 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -120,6 +120,7 @@ int cond_policydb_init(struct policydb *p) p->bool_val_to_struct = NULL; p->cond_list = NULL; + p->cond_list_len = 0; rc = avtab_init(&p->te_cond_avtab); if (rc) @@ -148,27 +149,22 @@ static void cond_node_destroy(struct cond_node *node) } cond_av_list_destroy(node->true_list); cond_av_list_destroy(node->false_list); - kfree(node); } -static void cond_list_destroy(struct cond_node *list) +static void cond_list_destroy(struct policydb *p) { - struct cond_node *next, *cur; + u32 i; - if (list == NULL) - return; - - for (cur = list; cur; cur = next) { - next = cur->next; - cond_node_destroy(cur); - } + for (i = 0; i < p->cond_list_len; i++) + cond_node_destroy(&p->cond_list[i]); + kfree(p->cond_list); } void cond_policydb_destroy(struct policydb *p) { kfree(p->bool_val_to_struct); avtab_destroy(&p->te_cond_avtab); - cond_list_destroy(p->cond_list); + cond_list_destroy(p); } int cond_init_bool_indexes(struct policydb *p) @@ -448,7 +444,6 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp) int cond_read_list(struct policydb *p, void *fp) { - struct cond_node *node, *last = NULL; __le32 buf[1]; u32 i, len; int rc; @@ -459,29 +454,24 @@ int cond_read_list(struct policydb *p, void *fp) len = le32_to_cpu(buf[0]); + p->cond_list = kcalloc(len, sizeof(*p->cond_list), GFP_KERNEL); + if (!p->cond_list) + return rc; + rc = avtab_alloc(&(p->te_cond_avtab), p->te_avtab.nel); if (rc) goto err; - for (i = 0; i < len; i++) { - rc = -ENOMEM; - node = kzalloc(sizeof(*node), GFP_KERNEL); - if (!node) - goto err; + p->cond_list_len = len; - rc = cond_read_node(p, node, fp); + for (i = 0; i < len; i++) { + rc = cond_read_node(p, &p->cond_list[i], fp); if (rc) goto err; - - if (i == 0) - p->cond_list = node; - else - last->next = node; - last = node; } return 0; err: - cond_list_destroy(p->cond_list); + cond_list_destroy(p); p->cond_list = NULL; return rc; } @@ -586,23 +576,19 @@ static int cond_write_node(struct policydb *p, struct cond_node *node, return 0; } -int cond_write_list(struct policydb *p, struct cond_node *list, void *fp) +int cond_write_list(struct policydb *p, void *fp) { - struct cond_node *cur; - u32 len; + u32 i; __le32 buf[1]; int rc; - len = 0; - for (cur = list; cur != NULL; cur = cur->next) - len++; - buf[0] = cpu_to_le32(len); + buf[0] = cpu_to_le32(p->cond_list_len); rc = put_entry(buf, sizeof(u32), 1, fp); if (rc) return rc; - for (cur = list; cur != NULL; cur = cur->next) { - rc = cond_write_node(p, cur, fp); + for (i = 0; i < p->cond_list_len; i++) { + rc = cond_write_node(p, &p->cond_list[i], fp); if (rc) return rc; } diff --git a/security/selinux/ss/conditional.h b/security/selinux/ss/conditional.h index ec846e45904c..680d8ac298e4 100644 --- a/security/selinux/ss/conditional.h +++ b/security/selinux/ss/conditional.h @@ -55,7 +55,6 @@ struct cond_node { struct cond_expr *expr; struct cond_av_list *true_list; struct cond_av_list *false_list; - struct cond_node *next; }; int cond_policydb_init(struct policydb *p); @@ -69,7 +68,7 @@ int cond_index_bool(void *key, void *datum, void *datap); int cond_read_bool(struct policydb *p, struct hashtab *h, void *fp); int cond_read_list(struct policydb *p, void *fp); int cond_write_bool(void *key, void *datum, void *ptr); -int cond_write_list(struct policydb *p, struct cond_node *list, void *fp); +int cond_write_list(struct policydb *p, void *fp); void cond_compute_av(struct avtab *ctab, struct avtab_key *key, struct av_decision *avd, struct extended_perms *xperms); diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index dd7aabd94a92..7069829b9f3a 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -3474,7 +3474,7 @@ int policydb_write(struct policydb *p, void *fp) if (rc) return rc; - rc = cond_write_list(p, p->cond_list, fp); + rc = cond_write_list(p, fp); if (rc) return rc; diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index b18bc405f820..971b5e64d63f 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -271,8 +271,9 @@ struct policydb { struct cond_bool_datum **bool_val_to_struct; /* type enforcement conditional access vectors and transitions */ struct avtab te_cond_avtab; - /* linked list indexing te_cond_avtab by conditional */ + /* array indexing te_cond_avtab by conditional */ struct cond_node *cond_list; + u32 cond_list_len; /* role allows */ struct role_allow *role_allow; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index a0afe49309c8..44ce4207c139 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2808,10 +2808,11 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb) } int security_get_bools(struct selinux_state *state, - int *len, char ***names, int **values) + u32 *len, char ***names, int **values) { struct policydb *policydb; - int i, rc; + u32 i; + int rc; if (!state->initialized) { *len = 0; @@ -2869,12 +2870,11 @@ int security_get_bools(struct selinux_state *state, } -int security_set_bools(struct selinux_state *state, int len, int *values) +int security_set_bools(struct selinux_state *state, u32 len, int *values) { struct policydb *policydb; - int i, rc; - int lenp, seqno = 0; - struct cond_node *cur; + int rc; + u32 i, lenp, seqno = 0; write_lock_irq(&state->ss->policy_rwlock); @@ -2902,8 +2902,8 @@ int security_set_bools(struct selinux_state *state, int len, int *values) policydb->bool_val_to_struct[i]->state = 0; } - for (cur = policydb->cond_list; cur; cur = cur->next) { - rc = evaluate_cond_node(policydb, cur); + for (i = 0; i < policydb->cond_list_len; i++) { + rc = evaluate_cond_node(policydb, &policydb->cond_list[i]); if (rc) goto out; } @@ -2922,11 +2922,11 @@ int security_set_bools(struct selinux_state *state, int len, int *values) } int security_get_bool_value(struct selinux_state *state, - int index) + u32 index) { struct policydb *policydb; int rc; - int len; + u32 len; read_lock(&state->ss->policy_rwlock); @@ -2946,10 +2946,10 @@ int security_get_bool_value(struct selinux_state *state, static int security_preserve_bools(struct selinux_state *state, struct policydb *policydb) { - int rc, nbools = 0, *bvalues = NULL, i; + int rc, *bvalues = NULL; char **bnames = NULL; struct cond_bool_datum *booldatum; - struct cond_node *cur; + u32 i, nbools = 0; rc = security_get_bools(state, &nbools, &bnames, &bvalues); if (rc) @@ -2959,8 +2959,8 @@ static int security_preserve_bools(struct selinux_state *state, if (booldatum) booldatum->state = bvalues[i]; } - for (cur = policydb->cond_list; cur; cur = cur->next) { - rc = evaluate_cond_node(policydb, cur); + for (i = 0; i < policydb->cond_list_len; i++) { + rc = evaluate_cond_node(policydb, &policydb->cond_list[i]); if (rc) goto out; } From patchwork Mon Sep 9 02:03:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Koichiro Den X-Patchwork-Id: 1982283 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4X29BL0KMZz1y1p for ; Mon, 9 Sep 2024 12:04:26 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1snTlK-0001qr-Dl; Mon, 09 Sep 2024 02:04:18 +0000 Received: from smtp-relay-internal-1.internal ([10.131.114.114] helo=smtp-relay-internal-1.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1snTlH-0001pe-NC for kernel-team@lists.ubuntu.com; Mon, 09 Sep 2024 02:04:15 +0000 Received: from mail-il1-f198.google.com (mail-il1-f198.google.com [209.85.166.198]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id 3DC653F1E8 for ; Mon, 9 Sep 2024 02:04:15 +0000 (UTC) Received: by mail-il1-f198.google.com with SMTP id e9e14a558f8ab-3a04c2472f6so62081985ab.0 for ; Sun, 08 Sep 2024 19:04:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725847453; x=1726452253; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=lM1dFkEcAaUTLR7dpVmGRyiJpqcUYVhjDGKKGAAXN1A=; b=I/aJXysJSjqNQ7Bu53CWetUH1GGgCDSkykEP/BWLHiWcOtw7rNQ0mB3JkZbb8xjsI+ 4bnOBdftbzR1zEuOQdLrJE8R312e2j1wP16zo3vYIjnqpFjAwf/2jJiC9Vz9BbEIYuGL +n5HqEf/K+OhpV4PiPU5BB9YV/39f6Pid4jQlOcnkyqMAX2BfQ55CBvhAo5tv8CLHpho SYKXKF/1YOpvYHYZqRPdgaj03PPAdFRKY7H0eCpwqsHUwgWdLIecK0zgGMwpNi/sKudH jVP2ihV4Is67hRQ5u/ZZ59/T6LeEQgwXKlGXdwKA0soN5taiZ0P+DXa4cFQSaeuU5Y/E V58g== X-Gm-Message-State: AOJu0YzT0az2eqmVWzUrh78Vajz853XD4oXRm4tOYqE3T1dHiSwxE7pJ hSWr3gJPiCBIC3JtY7emijsSJCmJBI6zVctyuTKInOF7bwTrDsoVkgxXH8LdVKnae+2aOgaiXy6 x0RrbiObYxZuyQsYkl2kaIbK43t5/lghV1QNDi19n/i0fdlLEBqj0TPCGLQRXj/vG+PlUqyVSHA 5KkOw3jhaavA== X-Received: by 2002:a05:6e02:2142:b0:3a0:4db0:ddbf with SMTP id e9e14a558f8ab-3a04f07e179mr117005135ab.8.1725847453474; Sun, 08 Sep 2024 19:04:13 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEXBhmf1b6GxZyyNIHtsFYyMFSne2AGON69JOtwQ/HIyfenofj7KMpcQhiQXzUTjRX4bttm2A== X-Received: by 2002:a05:6e02:2142:b0:3a0:4db0:ddbf with SMTP id e9e14a558f8ab-3a04f07e179mr117004955ab.8.1725847453035; Sun, 08 Sep 2024 19:04:13 -0700 (PDT) Received: from localhost.localdomain ([240f:74:7be:1:8492:650d:69e1:396a]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7d823736497sm2384187a12.10.2024.09.08.19.04.12 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 08 Sep 2024 19:04:12 -0700 (PDT) From: Koichiro Den To: kernel-team@lists.ubuntu.com Subject: [SRU][F][PATCH 2/2] selinux: fix double free of cond_list on error paths Date: Mon, 9 Sep 2024 11:03:29 +0900 Message-ID: <20240909020347.960015-3-koichiro.den@canonical.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240909020347.960015-1-koichiro.den@canonical.com> References: <20240909020347.960015-1-koichiro.den@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Vratislav Bendel On error path from cond_read_list() and duplicate_policydb_cond_list() the cond_list_destroy() gets called a second time in caller functions, resulting in NULL pointer deref. Fix this by resetting the cond_list_len to 0 in cond_list_destroy(), making subsequent calls a noop. Also consistently reset the cond_list pointer to NULL after freeing. Cc: stable@vger.kernel.org Signed-off-by: Vratislav Bendel [PM: fix line lengths in the description] Signed-off-by: Paul Moore (cherry picked from commit 186edf7e368c40d06cf727a1ad14698ea67b74ad) CVE-2022-48740 Signed-off-by: Koichiro Den --- security/selinux/ss/conditional.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index be1be450fc57..2b504c169cd9 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -158,6 +158,8 @@ static void cond_list_destroy(struct policydb *p) for (i = 0; i < p->cond_list_len; i++) cond_node_destroy(&p->cond_list[i]); kfree(p->cond_list); + p->cond_list = NULL; + p->cond_list_len = 0; } void cond_policydb_destroy(struct policydb *p) @@ -472,7 +474,6 @@ int cond_read_list(struct policydb *p, void *fp) return 0; err: cond_list_destroy(p); - p->cond_list = NULL; return rc; }