[PATCH 10/12] new iov_iter flavour: pipe-backed

[Al Viro]

On Thu, Sep 29, 2016 at 10:53:55PM +0200, Miklos Szeredi wrote:

> The EFAULT logic seems to be missing across the board.  And callers
> don't expect a zero return value.  Most will loop indefinitely.

Nope.  copy_page_to_iter() *never* returns -EFAULT.  Including the iovec
one - check copy_page_to_iter_iovec().  Any caller that does not expect
a zero return value from that primitive is a bug, triggerable as soon as
you feed it an iovec with NULL ->iov_base.

> Again, unexpected zero return if this is the first page.  Should
> return -ENOMEM?  Some callers only expect -EFAULT, though.

For copy_to_iter() and zero_iter() it's definitely "return zero".  For
get_pages...  Hell knows; those probably ought to return -EFAULT, but
I'll need to look some more at the callers.  It should end up triggering
a short read as the end result (or, as usual, EFAULT on zero-length read).

> > +               /* take it relative to the beginning of buffer */
> > +               size += off - pipe->bufs[idx].offset;
> > +               while (1) {
> > +                       buf = &pipe->bufs[idx];
> > +                       if (size > buf->len) {
> > +                               size -= buf->len;
> > +                               idx = next_idx(idx, pipe);
> > +                               off = 0;
> 
> off is unused and reassigned before breaking out of the loop.

True.

> [...]
> 
> > +       if (unlikely(i->type & ITER_PIPE)) {
> > +               struct pipe_inode_info *pipe = i->pipe;
> > +               size_t off;
> > +               int idx;
> > +
> > +               if (!sanity(i))
> > +                       return 0;
> > +
> > +               data_start(i, &idx, &off);
> > +               /* some of this one + all after this one */
> > +               npages = ((pipe->curbuf - idx - 1) & (pipe->buffers - 1)) + 1;
> 
> It's supposed to take i->count into account, no?  And that calculation
> will result in really funny things if the pipe is full.  And we can't
> return -EFAULT here, since that's not expected by callers...

It should look at i->count, in principle.  OTOH, overestimating the amount
is not really a problem for possible users of such iov_iter.  I'll look
into that.