From mboxrd@z Thu Jan 1 00:00:00 1970 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=-3.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,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 43114ECE560 for ; Sun, 23 Sep 2018 16:02:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CA4F82147C for ; Sun, 23 Sep 2018 16:02:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CA4F82147C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=i-love.sakura.ne.jp Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726309AbeIWWAr (ORCPT ); Sun, 23 Sep 2018 18:00:47 -0400 Received: from www262.sakura.ne.jp ([202.181.97.72]:63537 "EHLO www262.sakura.ne.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726146AbeIWWAq (ORCPT ); Sun, 23 Sep 2018 18:00:46 -0400 Received: from fsav302.sakura.ne.jp (fsav302.sakura.ne.jp [153.120.85.133]) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTP id w8NFxsmD006872; Mon, 24 Sep 2018 00:59:54 +0900 (JST) (envelope-from penguin-kernel@i-love.sakura.ne.jp) Received: from www262.sakura.ne.jp (202.181.97.72) by fsav302.sakura.ne.jp (F-Secure/fsigk_smtp/530/fsav302.sakura.ne.jp); Mon, 24 Sep 2018 00:59:54 +0900 (JST) X-Virus-Status: clean(F-Secure/fsigk_smtp/530/fsav302.sakura.ne.jp) Received: from [192.168.1.8] (softbank060157066051.bbtec.net [60.157.66.51]) (authenticated bits=0) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTPSA id w8NFxmfR006857 (version=TLSv1.2 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 24 Sep 2018 00:59:54 +0900 (JST) (envelope-from penguin-kernel@i-love.sakura.ne.jp) Subject: Re: [PATCH v4 00/19] LSM: Module stacking for SARA and Landlock To: Kees Cook , Casey Schaufler Cc: LSM , James Morris , SE Linux , LKLM , John Johansen , Paul Moore , Stephen Smalley , "linux-fsdevel@vger.kernel.org" , Alexey Dobriyan , =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= , Salvatore Mesoraca References: <680e6e16-0890-8304-0e8e-6c58966813b5@schaufler-ca.com> From: Tetsuo Handa Message-ID: <39457e79-3816-824b-6b4d-89d21b03f9ce@i-love.sakura.ne.jp> Date: Mon, 24 Sep 2018 00:59:47 +0900 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2018/09/23 11:43, Kees Cook wrote: >>> I'm excited about getting this landed! >> >> Soon. Real soon. I hope. I would very much like for >> someone from the SELinux camp to chime in, especially on >> the selinux_is_enabled() removal. > > Agreed. > This patchset from Casey lands before the patchset from Kees, doesn't it? OK, a few comments (if I didn't overlook something). lsm_early_cred()/lsm_early_task() are called from only __init functions. lsm_cred_alloc()/lsm_file_alloc() are called from only security/security.c . lsm_early_inode() should be avoided because it is not appropriate to call panic() when lsm_early_inode() is called after __init phase. Since all free hooks are called when one of init hooks failed, each free hook needs to check whether init hook was called. An example is inode_free_security() in security/selinux/hooks.c (but not addressed in this patch). This patchset might fatally prevent LKM-based LSM modules, for LKM-based LSMs cannot count on lsm_*_alloc() because size for lsm_*_alloc() cannot be updated upon loading LKM-based LSMs. If security_file_free() is called regardless of whether lsm_file_cache is defined, LKM-based LSMs can be loaded using current behavior (apart from the fact that legitimate interface for appending to security_hook_heads is currently missing). How do you plan to handle LKM-based LSMs? include/linux/lsm_hooks.h | 6 ++---- security/security.c | 31 ++++++------------------------- security/smack/smack_lsm.c | 8 +++++++- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 7e8b32f..8014614 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -2095,13 +2095,11 @@ static inline void __init yama_add_hooks(void) { } static inline void loadpin_add_hooks(void) { }; #endif -extern int lsm_cred_alloc(struct cred *cred, gfp_t gfp); extern int lsm_inode_alloc(struct inode *inode); #ifdef CONFIG_SECURITY -void lsm_early_cred(struct cred *cred); -void lsm_early_inode(struct inode *inode); -void lsm_early_task(struct task_struct *task); +void __init lsm_early_cred(struct cred *cred); +void __init lsm_early_task(struct task_struct *task); #endif #endif /* ! __LINUX_LSM_HOOKS_H */ diff --git a/security/security.c b/security/security.c index e7c85060..341e8df 100644 --- a/security/security.c +++ b/security/security.c @@ -267,7 +267,7 @@ int unregister_lsm_notifier(struct notifier_block *nb) * * Returns 0, or -ENOMEM if memory can't be allocated. */ -int lsm_cred_alloc(struct cred *cred, gfp_t gfp) +static int lsm_cred_alloc(struct cred *cred, gfp_t gfp) { if (blob_sizes.lbs_cred == 0) { cred->security = NULL; @@ -286,7 +286,7 @@ int lsm_cred_alloc(struct cred *cred, gfp_t gfp) * * Allocate the cred blob for all the modules if it's not already there */ -void lsm_early_cred(struct cred *cred) +void __init lsm_early_cred(struct cred *cred) { int rc; @@ -344,7 +344,7 @@ void __init security_add_blobs(struct lsm_blob_sizes *needed) * * Returns 0, or -ENOMEM if memory can't be allocated. */ -int lsm_file_alloc(struct file *file) +static int lsm_file_alloc(struct file *file) { if (!lsm_file_cache) { file->f_security = NULL; @@ -379,25 +379,6 @@ int lsm_inode_alloc(struct inode *inode) } /** - * lsm_early_inode - during initialization allocate a composite inode blob - * @inode: the inode that needs a blob - * - * Allocate the inode blob for all the modules if it's not already there - */ -void lsm_early_inode(struct inode *inode) -{ - int rc; - - if (inode == NULL) - panic("%s: NULL inode.\n", __func__); - if (inode->i_security != NULL) - return; - rc = lsm_inode_alloc(inode); - if (rc) - panic("%s: Early inode alloc failed.\n", __func__); -} - -/** * lsm_task_alloc - allocate a composite task blob * @task: the task that needs a blob * @@ -466,7 +447,7 @@ int lsm_msg_msg_alloc(struct msg_msg *mp) * * Allocate the task blob for all the modules if it's not already there */ -void lsm_early_task(struct task_struct *task) +void __init lsm_early_task(struct task_struct *task) { int rc; @@ -1202,11 +1183,11 @@ void security_file_free(struct file *file) { void *blob; + call_void_hook(file_free_security, file); + if (!lsm_file_cache) return; - call_void_hook(file_free_security, file); - blob = file->f_security; file->f_security = NULL; kmem_cache_free(lsm_file_cache, blob); diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 7843004..b0b4045 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -750,6 +750,13 @@ static int smack_set_mnt_opts(struct super_block *sb, if (sp->smk_flags & SMK_SB_INITIALIZED) return 0; + if (inode->i_security == NULL) { + int rc = lsm_inode_alloc(inode); + + if (rc) + return rc; + } + if (!smack_privileged(CAP_MAC_ADMIN)) { /* * Unprivileged mounts don't get to specify Smack values. @@ -818,7 +825,6 @@ static int smack_set_mnt_opts(struct super_block *sb, /* * Initialize the root inode. */ - lsm_early_inode(inode); init_inode_smack(inode, sp->smk_root); if (transmute) {