On Thu, 2020-11-19 at 09:31 -0500, Chuck Lever wrote:
>
>
> > On Nov 19, 2020, at 9:30 AM, Trond Myklebust <
> > trondmy@xxxxxxxxxxxxxxx> wrote:
> >
> > On Thu, 2020-11-19 at 09:17 -0500, Chuck Lever wrote:
> > >
> > >
> > > > On Nov 18, 2020, at 5:19 PM, trondmy@xxxxxxxxxx wrote:
> > > >
> > > > From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> > > >
> > > > When doing a read() into a page, we also don't care if the nul
> > > > padding
> > > > stays in that last page when the data length is not 32-bit
> > > > aligned.
> > >
> > > What if the READ payload lands in the middle of a file? The
> > > pad on the end will overwrite file content just past where
> > > the READ payload lands.
> >
> > If the size > buf->page_len, then it gets truncated in
> > xdr_align_pages() afaik.
>
> I will need to check how RPC/RDMA behaves. It might build a
> chunk that includes the pad in this case, which would break
> things.
That would be a bug in the existing code too, then. It shouldn't be
writing beyond the buffer size we set in the NFS layer.
> > > > Signed-off-by: Trond Myklebust
> > > > <trond.myklebust@xxxxxxxxxxxxxxx>
> > > > ---
> > > > fs/nfs/nfs2xdr.c | 2 +-
> > > > fs/nfs/nfs3xdr.c | 2 +-
> > > > fs/nfs/nfs4xdr.c | 2 +-
> > > > 3 files changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
> > > > index db9c265ad9e1..468bfbfe44d7 100644
> > > > --- a/fs/nfs/nfs2xdr.c
> > > > +++ b/fs/nfs/nfs2xdr.c
> > > > @@ -102,7 +102,7 @@ static int decode_nfsdata(struct xdr_stream
> > > > *xdr, struct nfs_pgio_res *result)
> > > > if (unlikely(!p))
> > > > return -EIO;
> > > > count = be32_to_cpup(p);
> > > > - recvd = xdr_read_pages(xdr, count);
> > > > + recvd = xdr_read_pages(xdr, xdr_align_size(count));
> > > > if (unlikely(count > recvd))
> > > > goto out_cheating;
> > > > out:
> > > > diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
> > > > index d3e1726d538b..8ef7c961d3e2 100644
> > > > --- a/fs/nfs/nfs3xdr.c
> > > > +++ b/fs/nfs/nfs3xdr.c
> > > > @@ -1611,7 +1611,7 @@ static int decode_read3resok(struct
> > > > xdr_stream *xdr,
> > > > ocount = be32_to_cpup(p++);
> > > > if (unlikely(ocount != count))
> > > > goto out_mismatch;
> > > > - recvd = xdr_read_pages(xdr, count);
> > > > + recvd = xdr_read_pages(xdr, xdr_align_size(count));
> > > > if (unlikely(count > recvd))
> > > > goto out_cheating;
> > > > out:
> > > > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> > > > index 755b556e85c3..5baa767106dc 100644
> > > > --- a/fs/nfs/nfs4xdr.c
> > > > +++ b/fs/nfs/nfs4xdr.c
> > > > @@ -5202,7 +5202,7 @@ static int decode_read(struct xdr_stream
> > > > *xdr, struct rpc_rqst *req,
> > > > return -EIO;
> > > > eof = be32_to_cpup(p++);
> > > > count = be32_to_cpup(p);
> > > > - recvd = xdr_read_pages(xdr, count);
> > > > + recvd = xdr_read_pages(xdr, xdr_align_size(count));
> > > > if (count > recvd) {
> > > > dprintk("NFS: server cheating in read reply: "
> > > > "count %u > recvd %u\n", count,
> > > > recvd);
> > > > --
> > > > 2.28.0
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
