From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from e23smtp06.au.ibm.com ([202.81.31.148]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aHdge-0001vo-Pg for kexec@lists.infradead.org; Fri, 08 Jan 2016 20:31:05 +0000 Received: from localhost by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sat, 9 Jan 2016 06:30:41 +1000 Received: from d23relay06.au.ibm.com (d23relay06.au.ibm.com [9.185.63.219]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id 17B8F2CE8054 for ; Sat, 9 Jan 2016 07:30:39 +1100 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay06.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u08KULID29229238 for ; Sat, 9 Jan 2016 07:30:29 +1100 Received: from d23av03.au.ibm.com (localhost [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u08KU6Xv002851 for ; Sat, 9 Jan 2016 07:30:06 +1100 Message-ID: <1452284986.2651.18.camel@linux.vnet.ibm.com> Subject: Re: [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel From: Mimi Zohar Date: Fri, 08 Jan 2016 15:29:46 -0500 In-Reply-To: References: <1452280924-28774-1-git-send-email-zohar@linux.vnet.ibm.com> <1452280924-28774-2-git-send-email-zohar@linux.vnet.ibm.com> Mime-Version: 1.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Kees Cook Cc: "linux-fsdevel@vger.kernel.org" , "Luis R. Rodriguez" , Dmitry Torokhov , Kexec Mailing List , David Howells , linux-security-module , David Woodhouse , linux-modules@vger.kernel.org On Fri, 2016-01-08 at 12:24 -0800, Kees Cook wrote: > On Fri, Jan 8, 2016 at 11:22 AM, Mimi Zohar wrote: > > In order to measure and appraise files being read by the kernel, > > new module and kexec syscalls were defined which include a file > > descriptor. Other places in the kernel (eg. firmware, IMA, > > sound) also read files. > > > > This patch introduces a common function for reading files from > > the kernel with the corresponding security post-read hook and > > function. > > > > Changelog: > > - Add missing > > > > Signed-off-by: Mimi Zohar > > --- > > fs/exec.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/fs.h | 1 + > > include/linux/lsm_hooks.h | 11 ++++++++++ > > include/linux/security.h | 9 ++++++++ > > security/security.c | 16 ++++++++++++++ > > 5 files changed, 93 insertions(+) > > > > diff --git a/fs/exec.c b/fs/exec.c > > index b06623a..3c48a19 100644 > > --- a/fs/exec.c > > +++ b/fs/exec.c > > @@ -56,6 +56,7 @@ > > #include > > #include > > #include > > +#include > > > > #include > > #include > > @@ -831,6 +832,61 @@ int kernel_read(struct file *file, loff_t offset, > > > > EXPORT_SYMBOL(kernel_read); > > > > +int kernel_read_file(struct file *file, void **buf, loff_t *size, > > + loff_t max_size, int policy_id) > > +{ > > + loff_t i_size, pos; > > + ssize_t bytes = 0; > > + int ret; > > + > > + if (!S_ISREG(file_inode(file)->i_mode)) > > + return -EINVAL; > > + > > + i_size = i_size_read(file_inode(file)); > > + if (max_size > 0 && i_size > max_size) > > + return -EFBIG; > > + if (i_size == 0) > > + return -EINVAL; > > + > > + *buf = vmalloc(i_size); > > This could get very large -- what risks do we have to system stability > here? Having userspace able to trigger such a massive allocation could > be a problem. The firmware loader was limited to MAX_INT... The different callers allowed different sizes. Instead of hard coding the max size for all callers, the third parameter of kernel_file_read is the caller max_size. Mimi _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e23smtp01.au.ibm.com ([202.81.31.143]:37827 "EHLO e23smtp01.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750860AbcAHUap (ORCPT ); Fri, 8 Jan 2016 15:30:45 -0500 Received: from localhost by e23smtp01.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sat, 9 Jan 2016 06:30:42 +1000 Message-ID: <1452284986.2651.18.camel@linux.vnet.ibm.com> Subject: Re: [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel From: Mimi Zohar To: Kees Cook Cc: linux-security-module , "Luis R. Rodriguez" , Kexec Mailing List , linux-modules@vger.kernel.org, "linux-fsdevel@vger.kernel.org" , David Howells , David Woodhouse , Dmitry Torokhov Date: Fri, 08 Jan 2016 15:29:46 -0500 In-Reply-To: References: <1452280924-28774-1-git-send-email-zohar@linux.vnet.ibm.com> <1452280924-28774-2-git-send-email-zohar@linux.vnet.ibm.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Sender: owner-linux-modules@vger.kernel.org List-ID: On Fri, 2016-01-08 at 12:24 -0800, Kees Cook wrote: > On Fri, Jan 8, 2016 at 11:22 AM, Mimi Zohar wrote: > > In order to measure and appraise files being read by the kernel, > > new module and kexec syscalls were defined which include a file > > descriptor. Other places in the kernel (eg. firmware, IMA, > > sound) also read files. > > > > This patch introduces a common function for reading files from > > the kernel with the corresponding security post-read hook and > > function. > > > > Changelog: > > - Add missing > > > > Signed-off-by: Mimi Zohar > > --- > > fs/exec.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/fs.h | 1 + > > include/linux/lsm_hooks.h | 11 ++++++++++ > > include/linux/security.h | 9 ++++++++ > > security/security.c | 16 ++++++++++++++ > > 5 files changed, 93 insertions(+) > > > > diff --git a/fs/exec.c b/fs/exec.c > > index b06623a..3c48a19 100644 > > --- a/fs/exec.c > > +++ b/fs/exec.c > > @@ -56,6 +56,7 @@ > > #include > > #include > > #include > > +#include > > > > #include > > #include > > @@ -831,6 +832,61 @@ int kernel_read(struct file *file, loff_t offset, > > > > EXPORT_SYMBOL(kernel_read); > > > > +int kernel_read_file(struct file *file, void **buf, loff_t *size, > > + loff_t max_size, int policy_id) > > +{ > > + loff_t i_size, pos; > > + ssize_t bytes = 0; > > + int ret; > > + > > + if (!S_ISREG(file_inode(file)->i_mode)) > > + return -EINVAL; > > + > > + i_size = i_size_read(file_inode(file)); > > + if (max_size > 0 && i_size > max_size) > > + return -EFBIG; > > + if (i_size == 0) > > + return -EINVAL; > > + > > + *buf = vmalloc(i_size); > > This could get very large -- what risks do we have to system stability > here? Having userspace able to trigger such a massive allocation could > be a problem. The firmware loader was limited to MAX_INT... The different callers allowed different sizes. Instead of hard coding the max size for all callers, the third parameter of kernel_file_read is the caller max_size. Mimi