On Sun, Dec 20, 2015 at 12:11:04AM -0500, Mimi Zohar wrote:
> On Thu, 2015-12-17 at 23:03 +0100, Luis R. Rodriguez wrote:
> > On Tue, Dec 08, 2015 at 01:01:23PM -0500, Mimi Zohar wrote:
> > > diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> > > index 8a45576..4d149c9 100644
> > > --- a/security/integrity/iint.c
> > > +++ b/security/integrity/iint.c
> > > @@ -222,6 +223,11 @@ int integrity_read_file(const char *path, char
> > > **data)
> > > return rc;
> > > }
> > >
> > > + if (!S_ISREG(file_inode(file)->i_mode)) {
> > > + rc = -EACCES;
> > > + goto out;
> > > + }
> > > +
> > > size = i_size_read(file_inode(file));
> > > if (size <= 0)
> > > goto out;
> >
> > This hunk seems to be unrelated to this patch? If so can it be split out?
>
> Yes, sure. Up to now, 'cat' was used to load the IMA policy. A lot
> of the problems related to opening and reading a file were hidden. So
> besides making sure that only a regular file is opened, what other
> things should we be checking? For example, do we permit the kernel to
> read NFS mounted files? Should the kernel be limited to opening only
> local files? Answering these questions becomes important as we move to
> a single kernel file read function.
Answering this properly should include effort to study and consolidate other
kernel read routines. From the little that I've so far reviewed these we don't
have much differences in requirements even between this IMA one and the sound
one
you just pointed out, the small changes for correctness however are important to
capture for all. Because of this we should be able to still provide a generic
read routine that takes all considerations into account, enables flexibility
but more importantly shares the best practices for correctness.
I can understand you might want to not wait for that yet, and I think that's
fine, but lets work in parallel to make that happen.
Luis
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
