[systemd-devel] [PATCH] Partially revert "ma-setup: simplify"

[Zbigniew Jędrzejewski-Szmek]

On Mon, Jun 08, 2015 at 12:29:51PM +0200, Lennart Poettering wrote:
> On Thu, 04.06.15 13:24, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
> 
> > On Wed, Jun 03, 2015 at 11:23:51AM -0400, Mimi Zohar wrote:
> > > On Wed, 2015-06-03 at 06:50 +0200, Lennart Poettering wrote:
> > > > On Tue, 02.06.15 11:55, Mimi Zohar (zohar at linux.vnet.ibm.com) wrote:
> > > > 
> > > > > > We could add another parameter to copy_bytes(), but in this case it's
> > > > > > cleaner to call fstat() and loop_write().
> > > > > 
> > > > > Right.  copy_bytes has no concept of rules/records.  So either "another
> > > > > parameter" is added to copy_bytes to indicate skip try_sendfile and
> > > > > write the entire policy, or [partially] revert the patch to calll
> > > > > loop_write() to write the entire policy directly.
> > > > 
> > > > In which way does sendfile() fail here? I mean, the code currently
> > > > understands ENOSYS and EINVAL as indications that sendfile() is not
> > > > supported on an fd. What does sendfile() on the IMA device return?
> > > > Most likely we can just check for that error code, and then try the
> > > > loop as fallback.
> > > 
> > > After the sendfile failure, in addition to resetting the file position
> > > to the beginning of the file,  the file would also need to be closed and
> > > re-opened.   Otherwise, IMA assumes the policy was malformed and fails
> > > the policy update.
> > OK, this seems just now worth the complication. I pushed this patch as is.
> 
> I'd really prefer if we'd understand the issue first, before we push
> patches about things. Can you explain why the file position would have
> been altered by sendfile()?
Hm, after looking at the kernel code, I'm confused. My latest patch
reverted the code to the previous behaviour, which Mimi reported to work,
but unless I'm reading the kernel code wrong, it is not good enough to
work in the general case:

ima_write_policy() limits the data to PAGE_SIZE. So if the policy is
longer than PAGE_SIZE, and the writer does not split the policy into
chunks, the policy will either be silently truncated on write, or fail
to parse, depending on where the PAGE_SIZE cutoff falls. So the only
reasonable way to write the policy seems to be to separate it into
lines on the writer side. ??

Zbyszek