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

[Lennart Poettering]

On Tue, 02.06.15 15:28, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:

> On Tue, Jun 02, 2015 at 04:17:04PM +0200, Lennart Poettering wrote:
> > On Mon, 01.06.15 12:36, Mimi Zohar (zohar at linux.vnet.ibm.com) wrote:
> > 
> > > On Mon, 2015-06-01 at 11:57 -0400, Mimi Zohar wrote:
> > > > On Mon, 2015-06-01 at 10:36 -0400, Zbigniew Jędrzejewski-Szmek wrote:
> > > > > ---
> > > > > OK, that's strange. Because the error message comes from copy_bytes() failing,
> > > > > and in copy_bytes() EINVAL could only originate in loop_write(), unless
> > > > > I'm missing something. Can you check the following patch, which essentially
> > > > > reverts to the old copy method?
> > > > > 
> > > > > Zbyszek
> > > > 
> > > > With minor "include" changes to have it apply cleanly, the IMA policy is
> > > > loaded properly.
> > > 
> > > FYI, by commenting out try_sendfile() in copy_bytes, the policy is also
> > > loaded properly.
> > 
> > How precisely does it fail? What's the error code you see?
> That's a good question. I'm not clear how that happens. One explanation
> would be if sendfile moved the input file offset on error... but that seems
> unlikely.
>  
> > Generally I'd prefer using the same routine for copying bytes between
> > fds wherever that's necessary, rather than introduce individual loops
> > at various places...
> > 
> > Or in other words: I'd prefer to make copy_bytes() useful for this
> > case too, rather then not use it.
> copy_bytes() tries to do the write in chunks, but ima kernel code
> needs every rule to be written in one write. Writing the whole file at
> once avoids the issue, and this is indendent of the issue with sendfile.
> We could add another parameter to copy_bytes(), but in this case it's
> cleaner to call fstat() and loop_write().

I figure we should change copy_bytes() to take a flags param and then
define COPY_FORCE_ONE_WRITE or so, that uses fstat() to size the copy
buffer, and avoids looping.

And the try_reflink thing should then become another flag for this.

Lennart

-- 
Lennart Poettering, Red Hat