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

[Mimi Zohar]

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 at 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