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