From patchwork Sun Sep 10 08:10:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Hellwig X-Patchwork-Id: 9946223 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 A228360348 for ; Sun, 10 Sep 2017 08:11:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 91D0128A86 for ; Sun, 10 Sep 2017 08:11:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8641328A87; Sun, 10 Sep 2017 08:11:45 +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.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=unavailable 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 6F13C28A88 for ; Sun, 10 Sep 2017 08:11:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751150AbdIJIKx (ORCPT ); Sun, 10 Sep 2017 04:10:53 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:54515 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751138AbdIJIKv (ORCPT ); Sun, 10 Sep 2017 04:10:51 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=OI2+5hViPIzIaUc4uNO6wiueGDIfD5bjXgkYgJCY77A=; b=joO3XMGFBoaAWOYd1fDHZ08dS gpGgpoMUZf2S2pti4DH3dtS6zTk1aMCHjwTNKMfzWqArdKTRtNLnRFxGIWCbUiXFBr0CSgdZviUty c2XuZIdaROJT+QIsB0jt4YS3LvbQLvJpbJRDZiSQMyuON7AqwECPetIm23BM4KSNRlhJqZmKNz/H7 /0sFEpRlEFqwkflZwzFiOTedNKymx3lqQCFidPC+X+QXdS4goHoCTGMS3gsbn39SOrrM8xFv5oDu3 77hJkPd1azBWhNiwobuO+p8NZL8OBjFGfdnEDsykzlXpRhR/dw8dc4XSxw9RaJ5c7E+oyJoK0lu+c tdcTcRtZA==; Received: from hch by bombadil.infradead.org with local (Exim 4.87 #1 (Red Hat Linux)) id 1dqxKK-00006f-37; Sun, 10 Sep 2017 08:10:48 +0000 Date: Sun, 10 Sep 2017 01:10:48 -0700 From: Christoph Hellwig To: Linus Torvalds Cc: Christoph Hellwig , James Morris , Linux Kernel Mailing List , LSM List , Mimi Zohar , Dmitry Kasatkin , 5ac7eace2d00eab5ae0e9fdee63e38aee6001f7c@infradead.org Subject: Re: [GIT PULL] Security subsystem updates for 4.14 Message-ID: <20170910081047.GA19533@infradead.org> References: <20170908070943.GA26549@infradead.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.3 (2017-05-23) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Sep 08, 2017 at 10:25:53AM -0700, Linus Torvalds wrote: > I don't think anybody actually tests linux-next kernels in any big > way, and the automated tests that do get run probably don't run with > any integrity checking enabled. Well, for the atual IMA deadlock issue I asked Mimi to produce automated tests and we get started on it. I was pretty pissed about the assumptions IMA made on the fs, whch weren't documented or automatically tested - coming from the XFS background where we want all our features to run through automated tests that was just not how I'd expect thing to work. But now as part of that I messed up the other caller of it, so I shouldn't complain too loud.. That being said - I really hink the certificate loading should not even go thorught this whole call path, but use our common helper to load a file into a buffer. Something like the patch below, I'm just not sure if the last policy argument is what people want or if we'd need to add a new policy type for certificates. --- From 3092ca95dfeff0dbd4e03ae62a51762facb15592 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sun, 10 Sep 2017 09:49:45 +0200 Subject: security/digisig: read file using kernel_read_file_from_path This avoid using the new integrity_read file operation which requires i_rwsem already to be held, and avoids a lot of code duplication and call the proper LSM hooks. [also constifies the path argument to kernel_read_file_from_path, as the callers needs it. Should probably be split into a separate patch] Signed-off-by: Christoph Hellwig --- fs/exec.c | 2 +- include/linux/fs.h | 2 +- security/integrity/digsig.c | 8 ++++--- security/integrity/iint.c | 49 ------------------------------------------ security/integrity/integrity.h | 2 -- 5 files changed, 7 insertions(+), 56 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 01a9fb9d8ac3..957a8ce294af 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -974,7 +974,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, } EXPORT_SYMBOL_GPL(kernel_read_file); -int kernel_read_file_from_path(char *path, void **buf, loff_t *size, +int kernel_read_file_from_path(const char *path, void **buf, loff_t *size, loff_t max_size, enum kernel_read_file_id id) { struct file *file; diff --git a/include/linux/fs.h b/include/linux/fs.h index 2d0e6748e46e..58855daba1eb 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2791,7 +2791,7 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id) extern int kernel_read(struct file *, loff_t, char *, unsigned long); extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, enum kernel_read_file_id); -extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t, +extern int kernel_read_file_from_path(const char *, void **, loff_t *, loff_t, enum kernel_read_file_id); extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, enum kernel_read_file_id); diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 06554c448dce..8112cdeeee3c 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -112,13 +112,15 @@ int __init integrity_init_keyring(const unsigned int id) int __init integrity_load_x509(const unsigned int id, const char *path) { key_ref_t key; - char *data; + void *data = NULL; + loff_t size; int rc; if (!keyring[id]) return -EINVAL; - rc = integrity_read_file(path, &data); + rc = kernel_read_file_from_path(path, data, &size, + 0, READING_POLICY); if (rc < 0) return rc; @@ -139,6 +141,6 @@ int __init integrity_load_x509(const unsigned int id, const char *path) key_ref_to_ptr(key)->description, path); key_ref_put(key); } - kfree(data); + vfree(data); return 0; } diff --git a/security/integrity/iint.c b/security/integrity/iint.c index 6fc888ca468e..c84e05866052 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -200,55 +200,6 @@ int integrity_kernel_read(struct file *file, loff_t offset, } /* - * integrity_read_file - read entire file content into the buffer - * - * This is function opens a file, allocates the buffer of required - * size, read entire file content to the buffer and closes the file - * - * It is used only by init code. - * - */ -int __init integrity_read_file(const char *path, char **data) -{ - struct file *file; - loff_t size; - char *buf; - int rc = -EINVAL; - - if (!path || !*path) - return -EINVAL; - - file = filp_open(path, O_RDONLY, 0); - if (IS_ERR(file)) { - rc = PTR_ERR(file); - pr_err("Unable to open file: %s (%d)", path, rc); - return rc; - } - - size = i_size_read(file_inode(file)); - if (size <= 0) - goto out; - - buf = kmalloc(size, GFP_KERNEL); - if (!buf) { - rc = -ENOMEM; - goto out; - } - - rc = integrity_kernel_read(file, 0, buf, size); - if (rc == size) { - *data = buf; - } else { - kfree(buf); - if (rc >= 0) - rc = -EIO; - } -out: - fput(file); - return rc; -} - -/* * integrity_load_keys - load integrity keys hook * * Hooks is called from init/main.c:kernel_init_freeable() diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index a53e7e4ab06c..e1bf040fb110 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -120,8 +120,6 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode); int integrity_kernel_read(struct file *file, loff_t offset, void *addr, unsigned long count); -int __init integrity_read_file(const char *path, char **data); - #define INTEGRITY_KEYRING_EVM 0 #define INTEGRITY_KEYRING_IMA 1 #define INTEGRITY_KEYRING_MODULE 2