From patchwork Wed Jan 4 11:00:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tetsuo Handa X-Patchwork-Id: 9496465 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id EF5B2606DD for ; Wed, 4 Jan 2017 11:01:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E7CB127DF9 for ; Wed, 4 Jan 2017 11:01:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DC42727E5A; Wed, 4 Jan 2017 11:01:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EE43727E33 for ; Wed, 4 Jan 2017 11:01:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935638AbdADLBF (ORCPT ); Wed, 4 Jan 2017 06:01:05 -0500 Received: from www262.sakura.ne.jp ([202.181.97.72]:19802 "EHLO www262.sakura.ne.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758371AbdADLBC (ORCPT ); Wed, 4 Jan 2017 06:01:02 -0500 Received: from fsav102.sakura.ne.jp (fsav102.sakura.ne.jp [27.133.134.229]) by www262.sakura.ne.jp (8.14.5/8.14.5) with ESMTP id v04B0xWt014431; Wed, 4 Jan 2017 20:00:59 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Received: from www262.sakura.ne.jp (202.181.97.72) by fsav102.sakura.ne.jp (F-Secure/fsigk_smtp/530/fsav102.sakura.ne.jp); Wed, 04 Jan 2017 20:00:59 +0900 (JST) X-Virus-Status: clean(F-Secure/fsigk_smtp/530/fsav102.sakura.ne.jp) Received: from ccsecurity.localdomain (softbank126227147111.bbtec.net [126.227.147.111]) (authenticated bits=0) by www262.sakura.ne.jp (8.14.5/8.14.5) with ESMTP id v04B0jVD014355 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 4 Jan 2017 20:00:59 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) From: Tetsuo Handa To: linux-security-module@vger.kernel.org Cc: Tetsuo Handa , John Johansen , Paul Moore , Stephen Smalley , Eric Paris , Casey Schaufler , Kees Cook , James Morris , "Serge E. Hallyn" , =?UTF-8?q?Jos=C3=A9=20Bollo?= Subject: [PATCH] LSM: Revive security_task_alloc() hook. Date: Wed, 4 Jan 2017 20:00:32 +0900 Message-Id: <1483527632-4523-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> X-Mailer: git-send-email 1.8.3.1 MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP ---------- John Johansen wrote at http://lkml.kernel.org/r/c83a7281-367f-2459-3bcd-f15b64d950a6@canonical.com : > On 12/20/2016 12:03 PM, Casey Schaufler wrote: > > Whether or not José moves forward with PTAGS we have identified > > an issue with the current cred based hooks for AppArmor, TOMOYO > > and at least one other proposed module. Regardless of the issues > > of /proc/pid there is work to be done. > > > I can take a stab at it in a few weeks. While not critical for apparmor > it would certainly cleanup apparmor's cred handling. John might be still busy or offline now. So, here is my implementation. ---------- We switched from "struct task_struct"->security to "struct cred"->security in Linux 2.6.29. But not all LSM modules were happy with that change. TOMOYO LSM module is an example which want to use per "struct task_struct" security blob, for TOMOYO's security context is defined based on "struct task_struct" rather than "struct cred". AppArmor LSM module is another example which want to use it, for AppArmor is currently abusing the cred a little bit to store the change_hat and setexeccon info. Although security_task_free() hook was revived in Linux 3.4 because Yama LSM module wanted to release per "struct task_struct" security blob, security_task_alloc() hook and "struct task_struct"->security field were not revived. Nowadays, we are getting proposals of lightweight LSM modules which want to use per "struct task_struct" security blob. PTAGS LSM module and CaitSith LSM module which are currently under proposal for inclusion also want to use it. Therefore, it will be time to revive security_task_alloc() hook. We are already allowing multiple concurrent LSM modules (up to one fully armored module which uses "struct cred"->security field or exclusive hooks like security_xfrm_state_pol_flow_match(), plus unlimited number of lightweight modules which do not use "struct cred"->security nor exclusive hooks) as long as they are built into the kernel. Since multiple LSM modules might want to use "struct task_struct"->security field, we need to somehow calculate and allocate enough size for that field. Casey is trying to calculate and allocate enough size for "struct cred"->security field, but that approach is not applicable for LKM based LSM modules. On the other hand, lightweight LSM modules (e.g. Yama) do not always allocate "struct task_struct"->security field. If we tolerate managing per "struct task_struct" security blobs outside of "struct task_struct", we don't need to calculate and allocate enough size for "struct task_struct"->security field, we can save memory by using address of "struct task_struct" as a hash key for searching corresponding security blobs, and as a result we can also allow LKM based LSM modules. Therefore, this patch does not revive "struct task_struct"->security field. It would be possible to remember location in security_hook_heads.task_alloc list and undo up to the corresponding security_hook_heads.task_free list when task_alloc failed. But security_task_alloc() unlikely fails, Yama is safe to call task_free even if security blob was not allocated, and LKM based LSM modules will anyway have to be prepared for possibility of calling task_free without corresponding task_alloc call. Therefore, this patch calls security_task_free() even if security_task_alloc() failed. Signed-off-by: Tetsuo Handa Cc: John Johansen Cc: Paul Moore Cc: Stephen Smalley Cc: Eric Paris Cc: Casey Schaufler Cc: Kees Cook Cc: James Morris Cc: Serge E. Hallyn Cc: José Bollo --- include/linux/lsm_hooks.h | 14 +++++++++++++- include/linux/security.h | 6 ++++++ kernel/fork.c | 4 ++++ security/security.c | 15 +++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 0dde959..efa1354d 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -534,8 +534,18 @@ * manual page for definitions of the @clone_flags. * @clone_flags contains the flags indicating what should be shared. * Return 0 if permission is granted. + * @task_alloc: + * @task task being allocated. + * Handle allocation of task-related resources. Note that task_free is + * called even if task_alloc failed. This means that all task_free users + * have to be prepared for task_free being called without corresponding + * task_alloc call. Since the address of @task is guaranteed to remain + * unchanged between task_alloc call and task_free call, task_free users + * can use the address of @task for checking whether task_free is called + * without corresponding task_alloc call. + * Returns a zero on success, negative values on failure. * @task_free: - * @task task being freed + * @task task about to be freed. * Handle release of task-related resources. (Note that this can be called * from interrupt context.) * @cred_alloc_blank: @@ -1479,6 +1489,7 @@ int (*file_open)(struct file *file, const struct cred *cred); int (*task_create)(unsigned long clone_flags); + int (*task_alloc)(struct task_struct *task); void (*task_free)(struct task_struct *task); int (*cred_alloc_blank)(struct cred *cred, gfp_t gfp); void (*cred_free)(struct cred *cred); @@ -1744,6 +1755,7 @@ struct security_hook_heads { struct list_head file_receive; struct list_head file_open; struct list_head task_create; + struct list_head task_alloc; struct list_head task_free; struct list_head cred_alloc_blank; struct list_head cred_free; diff --git a/include/linux/security.h b/include/linux/security.h index f4ebac1..d7cb449 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -305,6 +305,7 @@ int security_file_send_sigiotask(struct task_struct *tsk, int security_file_receive(struct file *file); int security_file_open(struct file *file, const struct cred *cred); int security_task_create(unsigned long clone_flags); +int security_task_alloc(struct task_struct *task); void security_task_free(struct task_struct *task); int security_cred_alloc_blank(struct cred *cred, gfp_t gfp); void security_cred_free(struct cred *cred); @@ -857,6 +858,11 @@ static inline int security_task_create(unsigned long clone_flags) return 0; } +static inline int security_task_alloc(struct task_struct *task) +{ + return 0; +} + static inline void security_task_free(struct task_struct *task) { } diff --git a/kernel/fork.c b/kernel/fork.c index 11c5c8a..9094a48 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1643,6 +1643,9 @@ static __latent_entropy struct task_struct *copy_process( goto bad_fork_cleanup_perf; /* copy all the process information */ shm_init_task(p); + retval = security_task_alloc(p); + if (retval) + goto bad_fork_cleanup_audit; retval = copy_semundo(clone_flags, p); if (retval) goto bad_fork_cleanup_audit; @@ -1862,6 +1865,7 @@ static __latent_entropy struct task_struct *copy_process( exit_sem(p); bad_fork_cleanup_audit: audit_free(p); + security_task_free(p); bad_fork_cleanup_perf: perf_event_free_task(p); bad_fork_cleanup_policy: diff --git a/security/security.c b/security/security.c index 32052f5..b746f85 100644 --- a/security/security.c +++ b/security/security.c @@ -892,6 +892,20 @@ int security_task_create(unsigned long clone_flags) return call_int_hook(task_create, 0, clone_flags); } +int security_task_alloc(struct task_struct *task) +{ + /* + * Since multiple LSMs might supply this call, we might end up partial + * allocation. Fortunately, we can unconditionally call + * security_task_free() if security_task_alloc() failed because Yama + * (the only user of task_free call as of revival of task_alloc call) + * is prepared for being called unconditionally and we can as well make + * new users of task_free call be prepared for being called + * unconditionally. + */ + return call_int_hook(task_alloc, 0, task); +} + void security_task_free(struct task_struct *task) { call_void_hook(task_free, task); @@ -1731,6 +1745,7 @@ struct security_hook_heads security_hook_heads = { .file_receive = LIST_HEAD_INIT(security_hook_heads.file_receive), .file_open = LIST_HEAD_INIT(security_hook_heads.file_open), .task_create = LIST_HEAD_INIT(security_hook_heads.task_create), + .task_alloc = LIST_HEAD_INIT(security_hook_heads.task_alloc), .task_free = LIST_HEAD_INIT(security_hook_heads.task_free), .cred_alloc_blank = LIST_HEAD_INIT(security_hook_heads.cred_alloc_blank),