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

[Lennart Poettering]

On Mon, 08.06.15 12:29, Lennart Poettering (lennart at poettering.net) wrote:

> On Wed, 03.06.15 11:23, Mimi Zohar (zohar at linux.vnet.ibm.com) 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.
> 
> Why would the file position need to be reset? I mean, either the
> sendfile() works or it doesn't. If it doesn't, then it should not
> modify the fd's file position in any way.
> 
> Are you saying that sendfile() is really broken about this?
> 
> I really don't see why first trying sendfile(), then falling back to
> read()/write() would not work. 
> 
> Can you elaborate?

BTW, I have now created this gihub issue for this:

https://github.com/systemd/systemd/issues/105

I am very much against shipping systemd with a patch applied that
tapes over issues when we don#t even understand the problem at hand.

Mimi, can you hence please elaborate on the issue, and why
specifically you think the file position would have to be reset if we
continue to use copy_bytes() for this?

Otherwise I will revert the work-around patch again, since I don't
want to carry this if we don#t understand the issue, and this will pop
up again the next time somebdoy wants to unify how we shovel bytes
between fds...

Lennart

-- 
Lennart Poettering, Red Hat