Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Mimi Zohar <zohar@linux.vnet.ibm.com>
To: "Luis R. Rodriguez" <mcgrof@suse.com>
Cc: Paul Moore <pmoore@redhat.com>,
	David Howells <dhowells@redhat.com>,
	John Johansen <john.johansen@canonical.com>,
	Kees Cook <keescook@chromium.org>,
	fsdevel@vger.kernel.org,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	kexec@lists.infradead.org, Joey Lee <jlee@suse.com>,
	linux-security-module <linux-security-module@vger.kernel.org>,
	Gary Lin <GLin@suse.com>,
	Casey Schaufler <casey@schaufler-ca.com>,
	David Woodhouse <dwmw2@infradead.org>,
	linux-modules@vger.kernel.org
Subject: Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version
Date: Thu, 21 Jan 2016 15:37:52 -0500	[thread overview]
Message-ID: <1453408672.9549.168.camel@linux.vnet.ibm.com> (raw)
In-Reply-To: <CAB=NE6VS1xptdnmfPLC4fGmFwdBSjAHxcmK9ThLP4haHa_j6fA@mail.gmail.com>

On Thu, 2016-01-21 at 08:56 -0800, Luis R. Rodriguez wrote:
> On Thu, Jan 21, 2016 at 5:12 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Thu, 2016-01-21 at 01:03 +0100, Luis R. Rodriguez wrote:
> >> On Mon, Jan 18, 2016 at 10:11:23AM -0500, Mimi Zohar wrote:
> >> > This patch replaces the module copy_module_from_fd() call with the VFS
> >> > common kernel_read_file_from_fd() function.  Instead of reading the
> >> > kernel module twice, once for measuring/appraising and then loading
> >> > the kernel module, the file is read once.
> >> >
> >> > This patch defines a new security hook named security_kernel_read_file(),
> >> > which is called before reading the file.  For now, call the module
> >> > security hook from security_kernel_read_file until the LSMs have been
> >> > converted to use the kernel_read_file hook.
> >> >
> >> > This patch retains the kernel_module_from_file hook, but removes the
> >> > security_kernel_module_from_file() function.
> >>
> >> I think it would help if your cover letter and this patch described
> >> a bit that some LSMs either prefer to read / check / appraise files
> >> prior to loading and some other prefer to do that later. You could
> >> explain the LSM hook preferences and what they do. Then here you
> >> can explain how this one prefers a hook early, but acknowledge that
> >> the other one still exists.
> >
> > Before this patch set, IMA measured/appraised/audited a file before
> > allowing it to be accessed, causing the file in some cases to be read
> > twice.   This patch set changes that.  Files are read into memory and
> > then measured/appraised/audited.
> 
> Sounds like this could help also with performance, has any preliminary
> benchmarking been done to see the effect ?

In general, IMA's pre-reading a file has negligible performance impact,
if any.   Dmitry's LinuxCon 2013 Europe talk "Integrity Protection
Solutions in Linux" had some performance statistics.  I'm not sure this
change will have much of a performance impact.

> > By defining
> > the pre and post security hooks in this patch set, it permits each of
> > the LSMs to migrate to the new hooks independently of each other.   Lets
> > ask the LSM maintainers what they think.
> 
> I see -- yeah making this a 2 step thing makes sense, so long as the
> maintainers can later expect / understand what would be done in a
> second patch set. Breaking this down in two patch sets makes sense. 

I'll defer adding the pre and post security hooks to the subsequent
patch set.

Mimi

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec