Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Subject: Re: [PATCH 3/3] NFS: Avoid copy of xdr padding in read()
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <20201118221939.20715-3-trondmy@kernel.org>
Date:   Thu, 19 Nov 2020 09:17:44 -0500
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <42FFB4EC-5E31-4002-92FC-7CA329479D78@oracle.com>
References: <20201118221939.20715-1-trondmy@kernel.org>
 <20201118221939.20715-2-trondmy@kernel.org>
 <20201118221939.20715-3-trondmy@kernel.org>
To:     trondmy@kernel.org
Precedence: bulk


> On Nov 18, 2020, at 5:19 PM, trondmy@kernel.org wrote:
>=20
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>=20
> 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.


> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> fs/nfs/nfs2xdr.c | 2 +-
> fs/nfs/nfs3xdr.c | 2 +-
> fs/nfs/nfs4xdr.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>=20
> 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 =3D be32_to_cpup(p);
> -	recvd =3D xdr_read_pages(xdr, count);
> +	recvd =3D 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 =3D be32_to_cpup(p++);
> 	if (unlikely(ocount !=3D count))
> 		goto out_mismatch;
> -	recvd =3D xdr_read_pages(xdr, count);
> +	recvd =3D 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 =3D be32_to_cpup(p++);
> 	count =3D be32_to_cpup(p);
> -	recvd =3D xdr_read_pages(xdr, count);
> +	recvd =3D 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);
> --=20
> 2.28.0
>=20

--
Chuck Lever