Received-SPF: pass (google.com: domain of linux-kernel+bounces-5400-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
	Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
	Kingdom.
	Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
In-Reply-To: <488523.1702996313@warthog.procyon.org.uk>
References: <488523.1702996313@warthog.procyon.org.uk> <367107fa03540f7ddd2e8de51c751348bd7eb42c.camel@kernel.org> <20231213152350.431591-1-dhowells@redhat.com> <20231213152350.431591-13-dhowells@redhat.com>
Cc: dhowells@redhat.com, Jeff Layton <jlayton@kernel.org>,
    Steve French <smfrench@gmail.com>,
    Matthew Wilcox <willy@infradead.org>,
    Marc Dionne <marc.dionne@auristor.com>,
    Paulo Alcantara <pc@manguebit.com>,
    Shyam Prasad N <sprasad@microsoft.com>, Tom Talpey <tom@talpey.com>,
    Dominique Martinet <asmadeus@codewreck.org>,
    Eric Van Hensbergen <ericvh@kernel.org>,
    Ilya Dryomov <idryomov@gmail.com>,
    Christian Brauner <christian@brauner.io>, linux-cachefs@redhat.com,
    linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org,
    linux-nfs@vger.kernel.org, ceph-devel@vger.kernel.org,
    v9fs@lists.linux.dev, linux-fsdevel@vger.kernel.org,
    linux-mm@kvack.org, netdev@vger.kernel.org,
    linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 12/39] netfs: Add iov_iters to (sub)requests to describe various buffers
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <488793.1702996856.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date: Tue, 19 Dec 2023 14:40:56 +0000
Message-ID: <488794.1702996856@warthog.procyon.org.uk>

David Howells <dhowells@redhat.com> wrote:

> > > @@ -88,6 +78,11 @@ static void netfs_read_from_server(struct netfs_i=
o_request *rreq,
> > >  				   struct netfs_io_subrequest *subreq)
> > >  {
> > >  	netfs_stat(&netfs_n_rh_download);
> > > +	if (iov_iter_count(&subreq->io_iter) !=3D subreq->len - subreq->tr=
ansferred)
> > > +		pr_warn("R=3D%08x[%u] ITER PRE-MISMATCH %zx !=3D %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 demot=
e
> > this to INFO or DEBUG?
> > =

> > Does this indicate a bug in the client or that the server is sending u=
s
> > malformed frames?
> =

> Good question.  The network filesystem updated subreq->transferred to in=
dicate
> it had transferred X amount of data, but the iterator had been updated t=
o
> indicate Y amount of data was transferred.  They really ought to match a=
s 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 nega=
tive'
> 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 a=
lways
> 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 t=
o be
> manually propagated just to avoid the warning.

Actually, it's more complicated than that.  It's an assertion that netfsli=
b is
doing the right prep.  This assertion is checked both when we initially ma=
ke a
request (in which case it definitely shouldn't fire) and when we perform a
resubmission on partial/complete read failure when we need to carefully
revalidate the numbers to make sure we don't end up with holes or wrinkles=
 in
the buffer.

Anyway, it shouldn't happen - but if it does, it probably presages data
corruption.

David