From patchwork Tue Oct 8 16:57:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roberto Sassu X-Patchwork-Id: 13826732 X-Patchwork-Delegate: paul@paul-moore.com Received: from frasgout11.his.huawei.com (frasgout11.his.huawei.com [14.137.139.23]) (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 5AE631DF964; Tue, 8 Oct 2024 16:57:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=14.137.139.23 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728406681; cv=none; b=pnpRzi0i38rmOVQYgbQZM76q2IhPp2M+XkLORZMZPc88JBMpLSYk92b8Q7Cx3AAMxZqYFcqqc4h/egV0RTOYKcNW6c5ZqnruwwQG14Y86mnWisFelugrChtsQ53mRsrX6VoRI4veOOYCxW7YPbzI4uwrHjNEcnYzxGaZEN+u+II= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728406681; c=relaxed/simple; bh=X59DJW+OtGgKWLWosloDcJz8o4IgMFFhEe6CwNhc4dQ=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=QR6rtCoJtMk3X3l9BlKRVM4IEH/fTkDXwfNmc04TYldq6Di6g4Qs7hKF7wxuYZJaizSIcvrQKd3gqUheqVdf9H0Yo5swsePHFlp7ATsfLQSr0eLONW7HbBqG6rB5Si8f4EHOUd5WYFnJ0GGiAsB4jZVv+PiVvrrefW2XYXSz4G0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=huaweicloud.com; spf=pass smtp.mailfrom=huaweicloud.com; arc=none smtp.client-ip=14.137.139.23 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=huaweicloud.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huaweicloud.com Received: from mail.maildlp.com (unknown [172.18.186.29]) by frasgout11.his.huawei.com (SkyGuard) with ESMTP id 4XNMBf6XQYz9v7Hm; Wed, 9 Oct 2024 00:37:46 +0800 (CST) Received: from mail02.huawei.com (unknown [7.182.16.27]) by mail.maildlp.com (Postfix) with ESMTP id 727D61401F0; Wed, 9 Oct 2024 00:57:44 +0800 (CST) Received: from huaweicloud.com (unknown [10.204.63.22]) by APP2 (Coremail) with SMTP id GxC2BwDnlseAZAVng2B7Ag--.64194S2; Tue, 08 Oct 2024 17:57:43 +0100 (CET) From: Roberto Sassu To: zohar@linux.ibm.com, dmitry.kasatkin@gmail.com, eric.snowberg@oracle.com, paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com Cc: linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, ebpqwerty472123@gmail.com, Roberto Sassu Subject: [PATCH 1/3] ima: Remove inode lock Date: Tue, 8 Oct 2024 18:57:30 +0200 Message-Id: <20241008165732.2603647-1-roberto.sassu@huaweicloud.com> X-Mailer: git-send-email 2.34.1 Precedence: bulk X-Mailing-List: linux-security-module@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-CM-TRANSID: GxC2BwDnlseAZAVng2B7Ag--.64194S2 X-Coremail-Antispam: 1UD129KBjvAXoWfGry3tF13ZFyxXFWkCw1xXwb_yoW8JrW7Ko WSy3sxJrn8WrySyay8Ww1SyFW8u39xG3yfCrs5XFnrK3W2kryUX347W3W3JFW3Xr4rGr1q k3s7Jw4kJF9rJ3Wkn29KB7ZKAUJUUUU5529EdanIXcx71UUUUU7v73VFW2AGmfu7bjvjm3 AaLaJ3UjIYCTnIWjp_UUUYI7kC6x804xWl14x267AKxVW8JVW5JwAFc2x0x2IEx4CE42xK 8VAvwI8IcIk0rVWrJVCq3wAFIxvE14AKwVWUJVWUGwA2ocxC64kIII0Yj41l84x0c7CEw4 AK67xGY2AK021l84ACjcxK6xIIjxv20xvE14v26r1j6r1xM28EF7xvwVC0I7IYx2IY6xkF 7I0E14v26F4j6r4UJwA2z4x0Y4vEx4A2jsIE14v26r4j6F4UM28EF7xvwVC2z280aVCY1x 0267AKxVW8JVW8Jr1le2I262IYc4CY6c8Ij28IcVAaY2xG8wAqx4xG64xvF2IEw4CE5I8C rVC2j2WlYx0E2Ix0cI8IcVAFwI0_JrI_JrylYx0Ex4A2jsIE14v26r1j6r4UMcvjeVCFs4 IE7xkEbVWUJVW8JwACjcxG0xvY0x0EwIxGrwACI402YVCY1x02628vn2kIc2xKxwCY1x02 62kKe7AKxVW8ZVWrXwCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwC20s 026c02F40E14v26r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_ Jw0_GFylIxkGc2Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20x vEc7CjxVAFwI0_Cr0_Gr1UMIIF0xvE42xK8VAvwI8IcIk0rVWUJVWUCwCI42IY6I8E87Iv 67AKxVWUJVW8JwCI42IY6I8E87Iv6xkF7I0E14v26r4j6r4UJbIYCTnIWIevJa73UjIFyT uYvjxU4NB_UUUUU X-CM-SenderInfo: purev21wro2thvvxqx5xdzvxpfor3voofrz/1tbiAQAKBGcElPgLNgAAs9 From: Roberto Sassu Move out the mutex in the ima_iint_cache structure to a new structure called ima_iint_cache_lock, so that a lock can be taken regardless of whether or not inode integrity metadata are stored in the inode. Introduce ima_inode_security() to simplify accessing the new structure in the inode security blob. Move the mutex initialization and annotation in the new function ima_inode_alloc_security() and introduce ima_iint_lock() and ima_iint_unlock() to respectively lock and unlock the mutex. Finally, expand the critical region in process_measurement() guarded by iint->mutex up to where the inode was locked, use only one iint lock in __ima_inode_hash(), since the mutex is now in the inode security blob, and replace the inode_lock()/inode_unlock() calls in ima_check_last_writer(). Signed-off-by: Roberto Sassu Reviewed-by: Paul Moore Signed-off-by: Kirill A. Shutemov --- security/integrity/ima/ima.h | 26 ++++++++--- security/integrity/ima/ima_api.c | 4 +- security/integrity/ima/ima_iint.c | 77 ++++++++++++++++++++++++++----- security/integrity/ima/ima_main.c | 39 +++++++--------- 4 files changed, 104 insertions(+), 42 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 3c323ca213d4..6474a15b584a 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -182,7 +182,6 @@ struct ima_kexec_hdr { /* IMA integrity metadata associated with an inode */ struct ima_iint_cache { - struct mutex mutex; /* protects: version, flags, digest */ struct integrity_inode_attributes real_inode; unsigned long flags; unsigned long measured_pcrs; @@ -195,35 +194,48 @@ struct ima_iint_cache { struct ima_digest_data *ima_hash; }; +struct ima_iint_cache_lock { + struct mutex mutex; /* protects: version, flags, digest */ + struct ima_iint_cache *iint; +}; + extern struct lsm_blob_sizes ima_blob_sizes; +static inline struct ima_iint_cache_lock *ima_inode_security(void *i_security) +{ + return i_security + ima_blob_sizes.lbs_inode; +} + static inline struct ima_iint_cache * ima_inode_get_iint(const struct inode *inode) { - struct ima_iint_cache **iint_sec; + struct ima_iint_cache_lock *iint_lock; if (unlikely(!inode->i_security)) return NULL; - iint_sec = inode->i_security + ima_blob_sizes.lbs_inode; - return *iint_sec; + iint_lock = ima_inode_security(inode->i_security); + return iint_lock->iint; } static inline void ima_inode_set_iint(const struct inode *inode, struct ima_iint_cache *iint) { - struct ima_iint_cache **iint_sec; + struct ima_iint_cache_lock *iint_lock; if (unlikely(!inode->i_security)) return; - iint_sec = inode->i_security + ima_blob_sizes.lbs_inode; - *iint_sec = iint; + iint_lock = ima_inode_security(inode->i_security); + iint_lock->iint = iint; } struct ima_iint_cache *ima_iint_find(struct inode *inode); struct ima_iint_cache *ima_inode_get(struct inode *inode); +int ima_inode_alloc_security(struct inode *inode); void ima_inode_free_rcu(void *inode_security); +void ima_iint_lock(struct inode *inode); +void ima_iint_unlock(struct inode *inode); void __init ima_iintcache_init(void); extern const int read_idmap[]; diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 984e861f6e33..37c2a228f0e1 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -234,7 +234,7 @@ static bool ima_get_verity_digest(struct ima_iint_cache *iint, * Calculate the file hash, if it doesn't already exist, * storing the measurement and i_version in the iint. * - * Must be called with iint->mutex held. + * Must be called with iint mutex held. * * Return 0 on success, error code otherwise */ @@ -343,7 +343,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file, * - the inode was previously flushed as well as the iint info, * containing the hashing info. * - * Must be called with iint->mutex held. + * Must be called with iint mutex held. */ void ima_store_measurement(struct ima_iint_cache *iint, struct file *file, const unsigned char *filename, diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c index 00b249101f98..c176fd0faae7 100644 --- a/security/integrity/ima/ima_iint.c +++ b/security/integrity/ima/ima_iint.c @@ -40,18 +40,18 @@ struct ima_iint_cache *ima_iint_find(struct inode *inode) * mutex to avoid lockdep false positives related to IMA + overlayfs. * See ovl_lockdep_annotate_inode_mutex_key() for more details. */ -static inline void ima_iint_lockdep_annotate(struct ima_iint_cache *iint, - struct inode *inode) +static inline void ima_iint_lock_lockdep_annotate(struct mutex *mutex, + struct inode *inode) { #ifdef CONFIG_LOCKDEP - static struct lock_class_key ima_iint_mutex_key[IMA_MAX_NESTING]; + static struct lock_class_key ima_iint_lock_mutex_key[IMA_MAX_NESTING]; int depth = inode->i_sb->s_stack_depth; if (WARN_ON_ONCE(depth < 0 || depth >= IMA_MAX_NESTING)) depth = 0; - lockdep_set_class(&iint->mutex, &ima_iint_mutex_key[depth]); + lockdep_set_class(mutex, &ima_iint_lock_mutex_key[depth]); #endif } @@ -68,14 +68,11 @@ static void ima_iint_init_always(struct ima_iint_cache *iint, iint->ima_read_status = INTEGRITY_UNKNOWN; iint->ima_creds_status = INTEGRITY_UNKNOWN; iint->measured_pcrs = 0; - mutex_init(&iint->mutex); - ima_iint_lockdep_annotate(iint, inode); } static void ima_iint_free(struct ima_iint_cache *iint) { kfree(iint->ima_hash); - mutex_destroy(&iint->mutex); kmem_cache_free(ima_iint_cache, iint); } @@ -108,6 +105,26 @@ struct ima_iint_cache *ima_inode_get(struct inode *inode) return iint; } +/** + * ima_inode_alloc_security - Called to init an inode + * @inode: Pointer to the inode + * + * Initialize and annotate the mutex in the ima_iint_cache_lock structure. + * + * Return: Zero. + */ +int ima_inode_alloc_security(struct inode *inode) +{ + struct ima_iint_cache_lock *iint_lock; + + iint_lock = ima_inode_security(inode->i_security); + + mutex_init(&iint_lock->mutex); + ima_iint_lock_lockdep_annotate(&iint_lock->mutex, inode); + + return 0; +} + /** * ima_inode_free_rcu - Called to free an inode via a RCU callback * @inode_security: The inode->i_security pointer @@ -116,11 +133,49 @@ struct ima_iint_cache *ima_inode_get(struct inode *inode) */ void ima_inode_free_rcu(void *inode_security) { - struct ima_iint_cache **iint_p = inode_security + ima_blob_sizes.lbs_inode; + struct ima_iint_cache_lock *iint_lock; + + iint_lock = ima_inode_security(inode_security); + + mutex_destroy(&iint_lock->mutex); + + /* iint_lock->iint should be NULL if !IS_IMA(inode) */ + if (iint_lock->iint) + ima_iint_free(iint_lock->iint); +} + +/** + * ima_iint_lock - Lock integrity metadata + * @inode: Pointer to the inode + * + * Lock integrity metadata. + */ +void ima_iint_lock(struct inode *inode) +{ + struct ima_iint_cache_lock *iint_lock; + + iint_lock = ima_inode_security(inode->i_security); + + /* Only inodes with i_security are processed by IMA. */ + if (iint_lock) + mutex_lock(&iint_lock->mutex); +} + +/** + * ima_iint_unlock - Unlock integrity metadata + * @inode: Pointer to the inode + * + * Unlock integrity metadata. + */ +void ima_iint_unlock(struct inode *inode) +{ + struct ima_iint_cache_lock *iint_lock; + + iint_lock = ima_inode_security(inode->i_security); - /* *iint_p should be NULL if !IS_IMA(inode) */ - if (*iint_p) - ima_iint_free(*iint_p); + /* Only inodes with i_security are processed by IMA. */ + if (iint_lock) + mutex_unlock(&iint_lock->mutex); } static void ima_iint_init_once(void *foo) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 06132cf47016..7852212c43ce 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -163,7 +163,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint, if (!(mode & FMODE_WRITE)) return; - mutex_lock(&iint->mutex); + ima_iint_lock(inode); if (atomic_read(&inode->i_writecount) == 1) { struct kstat stat; @@ -181,7 +181,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint, ima_update_xattr(iint, file); } } - mutex_unlock(&iint->mutex); + ima_iint_unlock(inode); } /** @@ -247,7 +247,7 @@ static int process_measurement(struct file *file, const struct cred *cred, if (action & IMA_FILE_APPRAISE) func = FILE_CHECK; - inode_lock(inode); + ima_iint_lock(inode); if (action) { iint = ima_inode_get(inode); @@ -259,15 +259,11 @@ static int process_measurement(struct file *file, const struct cred *cred, ima_rdwr_violation_check(file, iint, action & IMA_MEASURE, &pathbuf, &pathname, filename); - inode_unlock(inode); - if (rc) goto out; if (!action) goto out; - mutex_lock(&iint->mutex); - if (test_and_clear_bit(IMA_CHANGE_ATTR, &iint->atomic_flags)) /* reset appraisal flags if ima_inode_post_setattr was called */ iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED | @@ -412,10 +408,10 @@ static int process_measurement(struct file *file, const struct cred *cred, if ((mask & MAY_WRITE) && test_bit(IMA_DIGSIG, &iint->atomic_flags) && !(iint->flags & IMA_NEW_FILE)) rc = -EACCES; - mutex_unlock(&iint->mutex); kfree(xattr_value); ima_free_modsig(modsig); out: + ima_iint_unlock(inode); if (pathbuf) __putname(pathbuf); if (must_appraise) { @@ -580,18 +576,13 @@ static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf, struct ima_iint_cache *iint = NULL, tmp_iint; int rc, hash_algo; - if (ima_policy_flag) { + ima_iint_lock(inode); + + if (ima_policy_flag) iint = ima_iint_find(inode); - if (iint) - mutex_lock(&iint->mutex); - } if ((!iint || !(iint->flags & IMA_COLLECTED)) && file) { - if (iint) - mutex_unlock(&iint->mutex); - memset(&tmp_iint, 0, sizeof(tmp_iint)); - mutex_init(&tmp_iint.mutex); rc = ima_collect_measurement(&tmp_iint, file, NULL, 0, ima_hash_algo, NULL); @@ -600,22 +591,24 @@ static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf, if (rc != -ENOMEM) kfree(tmp_iint.ima_hash); + ima_iint_unlock(inode); return -EOPNOTSUPP; } iint = &tmp_iint; - mutex_lock(&iint->mutex); } - if (!iint) + if (!iint) { + ima_iint_unlock(inode); return -EOPNOTSUPP; + } /* * ima_file_hash can be called when ima_collect_measurement has still * not been called, we might not always have a hash. */ if (!iint->ima_hash || !(iint->flags & IMA_COLLECTED)) { - mutex_unlock(&iint->mutex); + ima_iint_unlock(inode); return -EOPNOTSUPP; } @@ -626,11 +619,12 @@ static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf, memcpy(buf, iint->ima_hash->digest, copied_size); } hash_algo = iint->ima_hash->algo; - mutex_unlock(&iint->mutex); if (iint == &tmp_iint) kfree(iint->ima_hash); + ima_iint_unlock(inode); + return hash_algo; } @@ -1118,7 +1112,7 @@ EXPORT_SYMBOL_GPL(ima_measure_critical_data); * @kmod_name: kernel module name * * Avoid a verification loop where verifying the signature of the modprobe - * binary requires executing modprobe itself. Since the modprobe iint->mutex + * binary requires executing modprobe itself. Since the modprobe iint mutex * is already held when the signature verification is performed, a deadlock * occurs as soon as modprobe is executed within the critical region, since * the same lock cannot be taken again. @@ -1193,6 +1187,7 @@ static struct security_hook_list ima_hooks[] __ro_after_init = { #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS LSM_HOOK_INIT(kernel_module_request, ima_kernel_module_request), #endif + LSM_HOOK_INIT(inode_alloc_security, ima_inode_alloc_security), LSM_HOOK_INIT(inode_free_security_rcu, ima_inode_free_rcu), }; @@ -1210,7 +1205,7 @@ static int __init init_ima_lsm(void) } struct lsm_blob_sizes ima_blob_sizes __ro_after_init = { - .lbs_inode = sizeof(struct ima_iint_cache *), + .lbs_inode = sizeof(struct ima_iint_cache_lock), }; DEFINE_LSM(ima) = { From patchwork Tue Oct 8 16:57:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roberto Sassu X-Patchwork-Id: 13826731 X-Patchwork-Delegate: paul@paul-moore.com Received: from frasgout11.his.huawei.com (frasgout11.his.huawei.com [14.137.139.23]) (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 5ADEB1DEFDC; Tue, 8 Oct 2024 16:57:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=14.137.139.23 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728406681; cv=none; b=Wa7QQA270e+jRHj5CVVLjya/F50GvQppqIXROEcJrdMoV6SXKYpYFYGjoES5QatsW87UPG1wjw1e6qr36DXmevE3TcsfIPHyf+n3fhZF8y9rpK+bjbKDN4BxHANAjFzwpn0iKrQBII5kt4yChdqQKFrRJzleHqGc84EeuHczfkI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728406681; c=relaxed/simple; bh=cABlkqAmSXMHna6jw/aL76yVMPEfA1rryVgcyRi5B6M=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=mxspwvVj+gnFCoVTpo/FcKf99RAd4LJ5he5l4MgxLJ3xKCziiKBmZq/b2uCSR97INymxxTkL8Bnr8pC0X6u8EB0u5YGQaYABeAUA6SIY9ZGqyVaeChJ8xjPFmS+s64UDIIjmW9pVC1SvrWbjM+c3MX68y1IoJXT62DVkSFsRm2I= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=huaweicloud.com; spf=pass smtp.mailfrom=huaweicloud.com; arc=none smtp.client-ip=14.137.139.23 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=huaweicloud.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huaweicloud.com Received: from mail.maildlp.com (unknown [172.18.186.29]) by frasgout11.his.huawei.com (SkyGuard) with ESMTP id 4XNMBn15J3z9v7Hm; Wed, 9 Oct 2024 00:37:53 +0800 (CST) Received: from mail02.huawei.com (unknown [7.182.16.27]) by mail.maildlp.com (Postfix) with ESMTP id ABCEF1401EA; Wed, 9 Oct 2024 00:57:50 +0800 (CST) Received: from huaweicloud.com (unknown [10.204.63.22]) by APP2 (Coremail) with SMTP id GxC2BwDnlseAZAVng2B7Ag--.64194S3; Tue, 08 Oct 2024 17:57:50 +0100 (CET) From: Roberto Sassu To: zohar@linux.ibm.com, dmitry.kasatkin@gmail.com, eric.snowberg@oracle.com, paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com Cc: linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, ebpqwerty472123@gmail.com, Roberto Sassu Subject: [PATCH 2/3] ima: Ensure lock is held when setting iint pointer in inode security blob Date: Tue, 8 Oct 2024 18:57:31 +0200 Message-Id: <20241008165732.2603647-2-roberto.sassu@huaweicloud.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20241008165732.2603647-1-roberto.sassu@huaweicloud.com> References: <20241008165732.2603647-1-roberto.sassu@huaweicloud.com> Precedence: bulk X-Mailing-List: linux-security-module@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-CM-TRANSID: GxC2BwDnlseAZAVng2B7Ag--.64194S3 X-Coremail-Antispam: 1UD129KBjvJXoWxZF48tryfJw4rtr4UJw47Jwb_yoW5GF17pa 1qga4UJ34UXFZ7Wrs5AasF9r4fK3yIgFy8Gw45Aw1qyFsrJr1jqr40yry7ury5Gr4rtwna vr1UKws8ua1qyr7anT9S1TB71UUUUUDqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUP2b4IE77IF4wAFF20E14v26rWj6s0DM7CY07I20VC2zVCF04k2 6cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28IrcIa0xkI8VA2jI8067AKxVWUGw A2048vs2IY020Ec7CjxVAFwI0_Gr0_Xr1l8cAvFVAK0II2c7xJM28CjxkF64kEwVA0rcxS w2x7M28EF7xvwVC0I7IYx2IY67AKxVWUJVWUCwA2z4x0Y4vE2Ix0cI8IcVCY1x0267AKxV WxJVW8Jr1l84ACjcxK6I8E87Iv67AKxVW8JVWxJwA2z4x0Y4vEx4A2jsIEc7CjxVAFwI0_ Gr0_Gr1UM2AIxVAIcxkEcVAq07x20xvEncxIr21l5I8CrVACY4xI64kE6c02F40Ex7xfMc Ij6xIIjxv20xvE14v26r106r15McIj6I8E87Iv67AKxVWUJVW8JwAm72CE4IkC6x0Yz7v_ Jr0_Gr1lF7xvr2IYc2Ij64vIr41lFIxGxcIEc7CjxVA2Y2ka0xkIwI1lc7CjxVAaw2AFwI 0_GFv_Wryl42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1lx2IqxVAqx4xG 67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14v26r1q6r43MI IYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7IYx2IY6xkF7I0E 14v26F4j6r4UJwCI42IY6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280aVAFwI0_Jr 0_Gr1lIxAIcVC2z280aVCY1x0267AKxVW8JVW8JrUvcSsGvfC2KfnxnUUI43ZEXa7IU0GY LDUUUUU== X-CM-SenderInfo: purev21wro2thvvxqx5xdzvxpfor3voofrz/1tbiAgAKBGcElX4LAwAAsM From: Roberto Sassu IMA stores a pointer of the ima_iint_cache structure, containing integrity metadata, in the inode security blob. However, check and assignment of this pointer is not atomic, and it might happen that two tasks both see that the iint pointer is NULL and try to set it, causing a memory leak. Ensure that the iint check and assignment is guarded, by adding a lockdep assertion in ima_inode_get(). Consequently, guard the remaining ima_inode_get() calls, in ima_post_create_tmpfile() and ima_post_path_mknod(), to avoid the lockdep warnings. Signed-off-by: Roberto Sassu --- security/integrity/ima/ima_iint.c | 5 +++++ security/integrity/ima/ima_main.c | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c index c176fd0faae7..fe676ccec32f 100644 --- a/security/integrity/ima/ima_iint.c +++ b/security/integrity/ima/ima_iint.c @@ -87,8 +87,13 @@ static void ima_iint_free(struct ima_iint_cache *iint) */ struct ima_iint_cache *ima_inode_get(struct inode *inode) { + struct ima_iint_cache_lock *iint_lock; struct ima_iint_cache *iint; + iint_lock = ima_inode_security(inode->i_security); + if (iint_lock) + lockdep_assert_held(&iint_lock->mutex); + iint = ima_iint_find(inode); if (iint) return iint; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 7852212c43ce..2425067b887d 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -705,14 +705,19 @@ static void ima_post_create_tmpfile(struct mnt_idmap *idmap, if (!must_appraise) return; + ima_iint_lock(inode); + /* Nothing to do if we can't allocate memory */ iint = ima_inode_get(inode); - if (!iint) + if (!iint) { + ima_iint_unlock(inode); return; + } /* needed for writing the security xattrs */ set_bit(IMA_UPDATE_XATTR, &iint->atomic_flags); iint->ima_file_status = INTEGRITY_PASS; + ima_iint_unlock(inode); } /** @@ -737,13 +742,18 @@ static void ima_post_path_mknod(struct mnt_idmap *idmap, struct dentry *dentry) if (!must_appraise) return; + ima_iint_lock(inode); + /* Nothing to do if we can't allocate memory */ iint = ima_inode_get(inode); - if (!iint) + if (!iint) { + ima_iint_unlock(inode); return; + } /* needed for re-opening empty files */ iint->flags |= IMA_NEW_FILE; + ima_iint_unlock(inode); } /** From patchwork Tue Oct 8 16:57:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roberto Sassu X-Patchwork-Id: 13826733 X-Patchwork-Delegate: paul@paul-moore.com Received: from frasgout11.his.huawei.com (frasgout11.his.huawei.com [14.137.139.23]) (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 E2B501E0DE2; Tue, 8 Oct 2024 16:57:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=14.137.139.23 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728406683; cv=none; b=NWpYC/FR2lA0arzovyXUfwwu7vCn64RKJ0d3g7+snw8vYO5BZbqe3st5+mMePhzBve6171Ao7LZKoiKr0mRwE7ct3Hp2KmgQoYh1OS28Cgt3zQgOu5J4sOpCLfFaRyQbBh/Ahh9TKA35jdK7kGcaOMLzX0CYqHPLuydnzbd93dw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728406683; c=relaxed/simple; bh=72BH+7dd3CukjfZzS71uAos02pWQ3nHiqzJ+LRPT6q0=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=ho7atEBdy9lGs3YEhKz6SiZXRtOIQZNQ3RqFkBXSbpViXqbVQ+yuW25ZlQctQ+gPyz2jVRrviC+hTdZcLJvvsnGMJU+vykK+VfoiFXzmkEN8jR79F3lcgkQH1s75qNXxs5QtsjWESeWHAcJkXx9rA+UEFDc3GfSpu6ehw0HJnOo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=huaweicloud.com; spf=pass smtp.mailfrom=huaweicloud.com; arc=none smtp.client-ip=14.137.139.23 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=huaweicloud.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huaweicloud.com Received: from mail.maildlp.com (unknown [172.18.186.51]) by frasgout11.his.huawei.com (SkyGuard) with ESMTP id 4XNMBp327Fz9v7Hl; Wed, 9 Oct 2024 00:37:54 +0800 (CST) Received: from mail02.huawei.com (unknown [7.182.16.27]) by mail.maildlp.com (Postfix) with ESMTP id F1593140156; Wed, 9 Oct 2024 00:57:56 +0800 (CST) Received: from huaweicloud.com (unknown [10.204.63.22]) by APP2 (Coremail) with SMTP id GxC2BwDnlseAZAVng2B7Ag--.64194S4; Tue, 08 Oct 2024 17:57:56 +0100 (CET) From: Roberto Sassu To: zohar@linux.ibm.com, dmitry.kasatkin@gmail.com, eric.snowberg@oracle.com, paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com Cc: linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, ebpqwerty472123@gmail.com, Roberto Sassu Subject: [PATCH 3/3] ima: Mark concurrent accesses to the iint pointer in the inode security blob Date: Tue, 8 Oct 2024 18:57:32 +0200 Message-Id: <20241008165732.2603647-3-roberto.sassu@huaweicloud.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20241008165732.2603647-1-roberto.sassu@huaweicloud.com> References: <20241008165732.2603647-1-roberto.sassu@huaweicloud.com> Precedence: bulk X-Mailing-List: linux-security-module@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-CM-TRANSID: GxC2BwDnlseAZAVng2B7Ag--.64194S4 X-Coremail-Antispam: 1UD129KBjvdXoWrKFyUGF4kZFyfAr43CryfWFg_yoWkKrX_uw nYvr1Utw15uFZ3u3yDAa4aqayv9Fy8Cr48Ka4ftanrA345Jr98XrWUJFnaqFy8Xr42gan8 Grnakry3t3ZrWjkaLaAFLSUrUUUUbb8apTn2vfkv8UJUUUU8Yxn0WfASr-VFAUDa7-sFnT 9fnUUIcSsGvfJTRUUUbqxYFVCjjxCrM7AC8VAFwI0_Wr0E3s1l1xkIjI8I6I8E6xAIw20E Y4v20xvaj40_Wr0E3s1l1IIY67AEw4v_Jr0_Jr4l82xGYIkIc2x26280x7IE14v26r15M2 8IrcIa0xkI8VCY1x0267AKxVW5JVCq3wA2ocxC64kIII0Yj41l84x0c7CEw4AK67xGY2AK 021l84ACjcxK6xIIjxv20xvE14v26r1I6r4UM28EF7xvwVC0I7IYx2IY6xkF7I0E14v26F 4j6r4UJwA2z4x0Y4vEx4A2jsIE14v26r4j6F4UM28EF7xvwVC2z280aVCY1x0267AKxVW8 Jr0_Cr1UM2AIxVAIcxkEcVAq07x20xvEncxIr21l5I8CrVACY4xI64kE6c02F40Ex7xfMc Ij6xIIjxv20xvE14v26r106r15McIj6I8E87Iv67AKxVWUJVW8JwAm72CE4IkC6x0Yz7v_ Jr0_Gr1lF7xvr2IYc2Ij64vIr41lFIxGxcIEc7CjxVA2Y2ka0xkIwI1lc7CjxVAaw2AFwI 0_GFv_Wryl42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1lx2IqxVAqx4xG 67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14v26r1q6r43MI IYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_JFI_Gr1lIxAIcVC0I7IYx2IY6xkF7I0E 14v26F4j6r4UJwCI42IY6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280aVAFwI0_Jr 0_Gr1lIxAIcVC2z280aVCY1x0267AKxVW8JVW8JrUvcSsGvfC2KfnxnUUI43ZEXa7IU0jN t3UUUUU== X-CM-SenderInfo: purev21wro2thvvxqx5xdzvxpfor3voofrz/1tbiAQAKBGcElPgLOAAAsz From: Roberto Sassu Use the READ_ONCE() and WRITE_ONCE() macros to mark concurrent read and write accesses to the portion of the inode security blob containing the iint pointer. Writers are serialized by the iint lock. Signed-off-by: Roberto Sassu --- security/integrity/ima/ima.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 6474a15b584a..3fe1651395ce 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -215,7 +215,7 @@ ima_inode_get_iint(const struct inode *inode) return NULL; iint_lock = ima_inode_security(inode->i_security); - return iint_lock->iint; + return READ_ONCE(iint_lock->iint); } static inline void ima_inode_set_iint(const struct inode *inode, @@ -227,7 +227,7 @@ static inline void ima_inode_set_iint(const struct inode *inode, return; iint_lock = ima_inode_security(inode->i_security); - iint_lock->iint = iint; + WRITE_ONCE(iint_lock->iint, iint); } struct ima_iint_cache *ima_iint_find(struct inode *inode);