Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > @@ -408,6 +417,10 @@ int netfs_write_begin(struct netfs_inode *ctx,
> > ractl._nr_pages = folio_nr_pages(folio);
> > netfs_rreq_expand(rreq, &ractl);
> >
> > + /* Set up the output buffer */
> > + iov_iter_xarray(&rreq->iter, ITER_DEST, &mapping->i_pages,
> > + rreq->start, rreq->len);
>
> Should the above be ITER_SOURCE ?
No - we're in ->write_begin() and are prefetching. If you look in the code,
there's a netfs_begin_read() call a few lines below. The output buffer for
the read is the page we're going to write into.
Note that netfs_write_begin() should be considered deprecated as the whole
perform_write thing will get replaced.
> > @@ -88,6 +78,11 @@ static void netfs_read_from_server(struct netfs_io_request *rreq,
> > struct netfs_io_subrequest *subreq)
> > {
> > netfs_stat(&netfs_n_rh_download);
> > + if (iov_iter_count(&subreq->io_iter) != subreq->len - subreq->transferred)
> > + pr_warn("R=%08x[%u] ITER PRE-MISMATCH %zx != %zx-%zx %lx\n",
> > + rreq->debug_id, subreq->debug_index,
> > + iov_iter_count(&subreq->io_iter), subreq->len,
> > + subreq->transferred, subreq->flags);
>
> pr_warn is a bit alarmist, esp given the cryptic message. Maybe demote
> this to INFO or DEBUG?
>
> Does this indicate a bug in the client or that the server is sending us
> malformed frames?
Good question. The network filesystem updated subreq->transferred to indicate
it had transferred X amount of data, but the iterator had been updated to
indicate Y amount of data was transferred. They really ought to match as it
may otherwise indicate an underrun (and potential leakage of old data).
Overruns are less of a problem since the iterator would have to 'go negative'
as it were.
However, it might be better just to leave io_iter unchecked since we end up
resetting it anyway each time we reinvoke the ->issue_read() op. It's always
possible that it will get copied and a different iterator get passed to the
network layer or cache fs - and so the change to the iterator then has to be
manually propagated just to avoid the warning.
David
