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;
From:   Chuck Lever <chuck.lever@oracle.com>
To:     Bruce Fields <bfields@fieldses.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v1 07/42] NFSD: Update WRITE3arg decoder to use struct
 xdr_stream
Thread-Topic: [PATCH v1 07/42] NFSD: Update WRITE3arg decoder to use struct
 xdr_stream
Thread-Index: AQHW8O8XiOIeY47PXUyIyG1mP1wKvaoz/mcA
Date:   Fri, 22 Jan 2021 18:55:23 +0000
Message-ID: <C7B44F6C-0EE8-4920-AB1C-C2EF6A807861@oracle.com>
References: <160986050640.5532.16498408936966394862.stgit@klimt.1015granger.net>
 <160986061812.5532.3122782251888690881.stgit@klimt.1015granger.net>
 <20210122184754.GC18583@fieldses.org>
In-Reply-To: <20210122184754.GC18583@fieldses.org>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <472DBBE70DE4C74D8C4743A04152C809@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 3b02671d-ef91-4d85-c875-08d8bf074608
X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jan 2021 18:55:23.2765
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Pgnt051jTLNz35RIZUdA50dnk5v5ytSJwZb67gyrkPv9vhdDQTT9wW6jsngIxf8pfqiaVbqQwwDquB/Op34FLA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4260
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9872 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 spamscore=0
 suspectscore=0 phishscore=0 adultscore=0 mlxlogscore=999 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2101220096
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9872 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 mlxscore=0
 suspectscore=0 lowpriorityscore=0 bulkscore=0 adultscore=0 spamscore=0
 phishscore=0 priorityscore=1501 impostorscore=0 malwarescore=0
 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2009150000 definitions=main-2101220097
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jan 22, 2021, at 1:47 PM, J. Bruce Fields <bfields@fieldses.org> wrote=
:
>=20
> On Tue, Jan 05, 2021 at 10:30:18AM -0500, Chuck Lever wrote:
>> As part of the update, open code that sanity-checks the size of the
>> data payload against the length of the RPC Call message has to be
>> re-implemented to use xdr_stream infrastructure.
>=20
> I'm having a little trouble parsing that.  Did you mean "write code"?

The WRITE payload size sanity-checking code has to be re-implemented.


> --b.
>=20
>>=20
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> fs/nfsd/nfs3xdr.c |   51 ++++++++++++++++++++---------------------------=
----
>> 1 file changed, 20 insertions(+), 31 deletions(-)
>>=20
>> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
>> index ff98eae5db81..0aafb096de91 100644
>> --- a/fs/nfsd/nfs3xdr.c
>> +++ b/fs/nfsd/nfs3xdr.c
>> @@ -405,52 +405,41 @@ nfs3svc_decode_readargs(struct svc_rqst *rqstp, __=
be32 *p)
>> int
>> nfs3svc_decode_writeargs(struct svc_rqst *rqstp, __be32 *p)
>> {
>> +	struct xdr_stream *xdr =3D &rqstp->rq_arg_stream;
>> 	struct nfsd3_writeargs *args =3D rqstp->rq_argp;
>> -	unsigned int len, hdr, dlen;
>> 	u32 max_blocksize =3D svc_max_payload(rqstp);
>> 	struct kvec *head =3D rqstp->rq_arg.head;
>> 	struct kvec *tail =3D rqstp->rq_arg.tail;
>> +	size_t remaining;
>>=20
>> -	p =3D decode_fh(p, &args->fh);
>> -	if (!p)
>> +	if (!svcxdr_decode_nfs_fh3(xdr, &args->fh))
>> 		return 0;
>> -	p =3D xdr_decode_hyper(p, &args->offset);
>> -
>> -	args->count =3D ntohl(*p++);
>> -	args->stable =3D ntohl(*p++);
>> -	len =3D args->len =3D ntohl(*p++);
>> -	if ((void *)p > head->iov_base + head->iov_len)
>> +	if (xdr_stream_decode_u64(xdr, &args->offset) < 0)
>> 		return 0;
>> -	/*
>> -	 * The count must equal the amount of data passed.
>> -	 */
>> -	if (args->count !=3D args->len)
>> +	if (xdr_stream_decode_u32(xdr, &args->count) < 0)
>> +		return 0;
>> +	if (xdr_stream_decode_u32(xdr, &args->stable) < 0)
>> 		return 0;
>>=20
>> -	/*
>> -	 * Check to make sure that we got the right number of
>> -	 * bytes.
>> -	 */
>> -	hdr =3D (void*)p - head->iov_base;
>> -	dlen =3D head->iov_len + rqstp->rq_arg.page_len + tail->iov_len - hdr;
>> -	/*
>> -	 * Round the length of the data which was specified up to
>> -	 * the next multiple of XDR units and then compare that
>> -	 * against the length which was actually received.
>> -	 * Note that when RPCSEC/GSS (for example) is used, the
>> -	 * data buffer can be padded so dlen might be larger
>> -	 * than required.  It must never be smaller.
>> -	 */
>> -	if (dlen < XDR_QUADLEN(len)*4)
>> +	/* opaque data */
>> +	if (xdr_stream_decode_u32(xdr, &args->len) < 0)
>> 		return 0;
>>=20
>> +	/* request sanity */
>> +	if (args->count !=3D args->len)
>> +		return 0;
>> +	remaining =3D head->iov_len + rqstp->rq_arg.page_len + tail->iov_len;
>> +	remaining -=3D xdr_stream_pos(xdr);
>> +	if (remaining < xdr_align_size(args->len))
>> +		return 0;
>> 	if (args->count > max_blocksize) {
>> 		args->count =3D max_blocksize;
>> -		len =3D args->len =3D max_blocksize;
>> +		args->len =3D max_blocksize;
>> 	}
>>=20
>> -	args->first.iov_base =3D (void *)p;
>> -	args->first.iov_len =3D head->iov_len - hdr;
>> +	args->first.iov_base =3D xdr->p;
>> +	args->first.iov_len =3D head->iov_len - xdr_stream_pos(xdr);
>> +
>> 	return 1;
>> }
>>=20
>>=20

--
Chuck Lever