The NFSv2/NFSv3 server does not handle zero length WRITE requests correctly

The NFSv2 and NFSv3 servers do not handle WRITE requests for 0 bytes correctly. The specifications indicate that the server should accept the request, but it should mostly turn into a no-op. Currently, the server will return an XDR decode error, which it should not. Attached is a patch which addresses this issue. It also adds some boundary checking to ensure that the request contains as much data as was requested to be written. It also correctly handles an NFSv3 request which requests to write more data than the server has stated that it is prepared to handle. Previously, there was some support which looked like it should work, but wasn't quite right. Signed-off-by: Peter Staubach <staubach@redhat.com> Acked-by: Neil Brown <neilb@suse.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

The NFSv2/NFSv3 server does not handle zero length WRITE requests correctly
The NFSv2 and NFSv3 servers do not handle WRITE requests for 0 bytes correctly. The specifications indicate that the server should accept the request, but it should mostly turn into a no-op. Currently, the server will return an XDR decode error, which it should not. Attached is a patch which addresses this issue. It also adds some boundary checking to ensure that the request contains as much data as was requested to be written. It also correctly handles an NFSv3 request which requests to write more data than the server has stated that it is prepared to handle. Previously, there was some support which looked like it should work, but wasn't quite right. Signed-off-by: Peter Staubach <staubach@redhat.com> Acked-by: Neil Brown <neilb@suse.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Peter Staubach · Linus Torvalds
1 parent 8842c9655b
Showing 2 changed files with 77 additions and 17 deletions Side-by-side Diff
fs/nfsd/nfs3xdr.c
fs/nfsd/nfsxdr.c
@@ -369,7 +369,7 @@
 nfs3svc_decode_writeargs(struct svc_rqst *rqstp, __be32 *p,
 					struct nfsd3_writeargs *args)
 {
-	unsigned int len, v, hdr;
+	unsigned int len, v, hdr, dlen;
 	u32 max_blocksize = svc_max_payload(rqstp);
  
 	if (!(p = decode_fh(p, &args->fh))
  
  
  
  
@@ -379,18 +379,47 @@
 	args->count = ntohl(*p++);
 	args->stable = ntohl(*p++);
 	len = args->len = ntohl(*p++);
+	/*
+	 * The count must equal the amount of data passed.
+	 */
+	if (args->count != args->len)
+		return 0;
  
+	/*
+	 * Check to make sure that we got the right number of
+	 * bytes.
+	 *
+	 * If more than one page was used, then compute the length
+	 * of the data in the request as the total size of the
+	 * request minus the transport protocol headers minus the
+	 * RPC protocol headers minus the NFS protocol fields
+	 * already consumed.  If the request fits into a single
+	 * page, then compete the length of the data as the size
+	 * of the NFS portion of the request minus the NFS
+	 * protocol fields already consumed.
+	 */
 	hdr = (void*)p - rqstp->rq_arg.head[0].iov_base;
-	if (rqstp->rq_arg.len < hdr ||
-	    rqstp->rq_arg.len - hdr < len)
+	if (rqstp->rq_respages != rqstp->rq_pages + 1) {
+		dlen = rqstp->rq_arg.len -
+			(PAGE_SIZE - rqstp->rq_arg.head[0].iov_len) - hdr;
+	} else {
+		dlen = rqstp->rq_arg.head[0].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.
+	 */
+	if (dlen != ((len + 3) & ~0x3))
 		return 0;
  
+	if (args->count > max_blocksize) {
+		args->count = max_blocksize;
+		len = args->len = max_blocksize;
+	}
 	rqstp->rq_vec[0].iov_base = (void*)p;
 	rqstp->rq_vec[0].iov_len = rqstp->rq_arg.head[0].iov_len - hdr;
-
-	if (len > max_blocksize)
-		len = max_blocksize;
-	v=  0;
+	v = 0;
 	while (len > rqstp->rq_vec[v].iov_len) {
 		len -= rqstp->rq_vec[v].iov_len;
 		v++;
@@ -398,9 +427,8 @@
 		rqstp->rq_vec[v].iov_len = PAGE_SIZE;
 	}
 	rqstp->rq_vec[v].iov_len = len;
-	args->vlen = v+1;
-
-	return args->count == args->len && rqstp->rq_vec[0].iov_len > 0;
+	args->vlen = v + 1;
+	return 1;
 }
  
 int
@@ -284,8 +284,9 @@
 nfssvc_decode_writeargs(struct svc_rqst *rqstp, __be32 *p,
 					struct nfsd_writeargs *args)
 {
-	unsigned int len;
+	unsigned int len, hdr, dlen;
 	int v;
+
 	if (!(p = decode_fh(p, &args->fh)))
 		return 0;
  
  
@@ -293,11 +294,42 @@
 	args->offset = ntohl(*p++);	/* offset */
 	p++;				/* totalcount */
 	len = args->len = ntohl(*p++);
-	rqstp->rq_vec[0].iov_base = (void*)p;
-	rqstp->rq_vec[0].iov_len = rqstp->rq_arg.head[0].iov_len -
-				(((void*)p) - rqstp->rq_arg.head[0].iov_base);
+	/*
+	 * The protocol specifies a maximum of 8192 bytes.
+	 */
 	if (len > NFSSVC_MAXBLKSIZE_V2)
-		len = NFSSVC_MAXBLKSIZE_V2;
+		return 0;
+
+	/*
+	 * Check to make sure that we got the right number of
+	 * bytes.
+	 *
+	 * If more than one page was used, then compute the length
+	 * of the data in the request as the total size of the
+	 * request minus the transport protocol headers minus the
+	 * RPC protocol headers minus the NFS protocol fields
+	 * already consumed.  If the request fits into a single
+	 * page, then compete the length of the data as the size
+	 * of the NFS portion of the request minus the NFS
+	 * protocol fields already consumed.
+	 */
+	hdr = (void*)p - rqstp->rq_arg.head[0].iov_base;
+	if (rqstp->rq_respages != rqstp->rq_pages + 1) {
+		dlen = rqstp->rq_arg.len -
+			(PAGE_SIZE - rqstp->rq_arg.head[0].iov_len) - hdr;
+	} else {
+		dlen = rqstp->rq_arg.head[0].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.
+	 */
+	if (dlen != ((len + 3) & ~0x3))
+		return 0;
+
+	rqstp->rq_vec[0].iov_base = (void*)p;
+	rqstp->rq_vec[0].iov_len = rqstp->rq_arg.head[0].iov_len - hdr;
 	v = 0;
 	while (len > rqstp->rq_vec[v].iov_len) {
 		len -= rqstp->rq_vec[v].iov_len;
@@ -306,8 +338,8 @@
 		rqstp->rq_vec[v].iov_len = PAGE_SIZE;
 	}
 	rqstp->rq_vec[v].iov_len = len;
-	args->vlen = v+1;
-	return rqstp->rq_vec[0].iov_len > 0;
+	args->vlen = v + 1;
+	return 1;
 }
  
 int
...	...	@@ -369,7 +369,7 @@
369	369	nfs3svc_decode_writeargs(struct svc_rqst rqstp, __be32 p,
370	370	struct nfsd3_writeargs *args)
371	371	{
372		- unsigned int len, v, hdr;
	372	+ unsigned int len, v, hdr, dlen;
373	373	u32 max_blocksize = svc_max_payload(rqstp);
374	374
375	375	if (!(p = decode_fh(p, &args->fh))
376	376
377	377
378	378
379	379
...	...	@@ -379,18 +379,47 @@
379	379	args->count = ntohl(*p++);
380	380	args->stable = ntohl(*p++);
381	381	len = args->len = ntohl(*p++);
	382	+ /*
	383	+ * The count must equal the amount of data passed.
	384	+ */
	385	+ if (args->count != args->len)
	386	+ return 0;
382	387
	388	+ /*
	389	+ * Check to make sure that we got the right number of
	390	+ * bytes.
	391	+ *
	392	+ * If more than one page was used, then compute the length
	393	+ * of the data in the request as the total size of the
	394	+ * request minus the transport protocol headers minus the
	395	+ * RPC protocol headers minus the NFS protocol fields
	396	+ * already consumed. If the request fits into a single
	397	+ * page, then compete the length of the data as the size
	398	+ * of the NFS portion of the request minus the NFS
	399	+ * protocol fields already consumed.
	400	+ */
383	401	hdr = (void*)p - rqstp->rq_arg.head[0].iov_base;
384		- if (rqstp->rq_arg.len < hdr \|\|
385		- rqstp->rq_arg.len - hdr < len)
	402	+ if (rqstp->rq_respages != rqstp->rq_pages + 1) {
	403	+ dlen = rqstp->rq_arg.len -
	404	+ (PAGE_SIZE - rqstp->rq_arg.head[0].iov_len) - hdr;
	405	+ } else {
	406	+ dlen = rqstp->rq_arg.head[0].iov_len - hdr;
	407	+ }
	408	+ /*
	409	+ * Round the length of the data which was specified up to
	410	+ * the next multiple of XDR units and then compare that
	411	+ * against the length which was actually received.
	412	+ */
	413	+ if (dlen != ((len + 3) & ~0x3))
386	414	return 0;
387	415
	416	+ if (args->count > max_blocksize) {
	417	+ args->count = max_blocksize;
	418	+ len = args->len = max_blocksize;
	419	+ }
388	420	rqstp->rq_vec[0].iov_base = (void*)p;
389	421	rqstp->rq_vec[0].iov_len = rqstp->rq_arg.head[0].iov_len - hdr;
390		-
391		- if (len > max_blocksize)
392		- len = max_blocksize;
393		- v= 0;
	422	+ v = 0;
394	423	while (len > rqstp->rq_vec[v].iov_len) {
395	424	len -= rqstp->rq_vec[v].iov_len;
396	425	v++;
...	...	@@ -398,9 +427,8 @@
398	427	rqstp->rq_vec[v].iov_len = PAGE_SIZE;
399	428	}
400	429	rqstp->rq_vec[v].iov_len = len;
401		- args->vlen = v+1;
402		-
403		- return args->count == args->len && rqstp->rq_vec[0].iov_len > 0;
	430	+ args->vlen = v + 1;
	431	+ return 1;
404	432	}
405	433
406	434	int
...	...	@@ -284,8 +284,9 @@
284	284	nfssvc_decode_writeargs(struct svc_rqst rqstp, __be32 p,
285	285	struct nfsd_writeargs *args)
286	286	{
287		- unsigned int len;
	287	+ unsigned int len, hdr, dlen;
288	288	int v;
	289	+
289	290	if (!(p = decode_fh(p, &args->fh)))
290	291	return 0;
291	292
292	293
...	...	@@ -293,11 +294,42 @@
293	294	args->offset = ntohl(p++); / offset */
294	295	p++; /* totalcount */
295	296	len = args->len = ntohl(*p++);
296		- rqstp->rq_vec[0].iov_base = (void*)p;
297		- rqstp->rq_vec[0].iov_len = rqstp->rq_arg.head[0].iov_len -
298		- (((void*)p) - rqstp->rq_arg.head[0].iov_base);
	297	+ /*
	298	+ * The protocol specifies a maximum of 8192 bytes.
	299	+ */
299	300	if (len > NFSSVC_MAXBLKSIZE_V2)
300		- len = NFSSVC_MAXBLKSIZE_V2;
	301	+ return 0;
	302	+
	303	+ /*
	304	+ * Check to make sure that we got the right number of
	305	+ * bytes.
	306	+ *
	307	+ * If more than one page was used, then compute the length
	308	+ * of the data in the request as the total size of the
	309	+ * request minus the transport protocol headers minus the
	310	+ * RPC protocol headers minus the NFS protocol fields
	311	+ * already consumed. If the request fits into a single
	312	+ * page, then compete the length of the data as the size
	313	+ * of the NFS portion of the request minus the NFS
	314	+ * protocol fields already consumed.
	315	+ */
	316	+ hdr = (void*)p - rqstp->rq_arg.head[0].iov_base;
	317	+ if (rqstp->rq_respages != rqstp->rq_pages + 1) {
	318	+ dlen = rqstp->rq_arg.len -
	319	+ (PAGE_SIZE - rqstp->rq_arg.head[0].iov_len) - hdr;
	320	+ } else {
	321	+ dlen = rqstp->rq_arg.head[0].iov_len - hdr;
	322	+ }
	323	+ /*
	324	+ * Round the length of the data which was specified up to
	325	+ * the next multiple of XDR units and then compare that
	326	+ * against the length which was actually received.
	327	+ */
	328	+ if (dlen != ((len + 3) & ~0x3))
	329	+ return 0;
	330	+
	331	+ rqstp->rq_vec[0].iov_base = (void*)p;
	332	+ rqstp->rq_vec[0].iov_len = rqstp->rq_arg.head[0].iov_len - hdr;
301	333	v = 0;
302	334	while (len > rqstp->rq_vec[v].iov_len) {
303	335	len -= rqstp->rq_vec[v].iov_len;
...	...	@@ -306,8 +338,8 @@
306	338	rqstp->rq_vec[v].iov_len = PAGE_SIZE;
307	339	}
308	340	rqstp->rq_vec[v].iov_len = len;
309		- args->vlen = v+1;
310		- return rqstp->rq_vec[0].iov_len > 0;
	341	+ args->vlen = v + 1;
	342	+ return 1;
311	343	}
312	344
313	345	int