Subject: kern/3170: NFS server can crash the system
To: None <>
From: None <>
List: netbsd-bugs
Date: 01/30/1997 19:25:50
>Number:         3170
>Category:       kern
>Synopsis:       NFS server can crash due to incorrect loop test
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jan 30 18:35:03 1997
>Originator:     Michael L. Hitch
Montana State University
>Release:        1.2_BETA
System: NetBSD 1.2B NetBSD 1.2B (ZEUS) #970120X-0: Mon Jan 20 17:56:34 MST 1997 amiga

The NFS server can crash in nfs_readdir() under the right condition.  A
buffer is allocated for reading the requested directory.  If the buffer
happens to end on a page boundary and the following page is invalid,
nfs_readdir() may attempt to access the invalid page at the end of the
buffer.  This occurs because a reference through a pointer to the buffer
is done before doing a test for the end of the buffer.

Do the right NFS directory read at the right time :-).  I do not know how
to duplicate the problem;  I was doing a find on an nfs-mounted file system
when the nfs server crashed.  The server continued to crash after rebooting
until I rebooted the client.  After locating where the crash occurred and
examining the code, I was able to fix the problem and added a check to
print a message when the original code would have referenced the invalid
page.  I am able to reliably hit the right conditions which would have
caused the crash.

Change the code sequence in the while loop to test for the end of the loop
before accessing the directory data:

--- /usr/src/sys/nfs/nfs_serv.c	Wed Dec 11 05:21:15 1996
+++ ./nfs_serv.c	Thu Jan 30 18:56:40 1997
@@ -2804,8 +2804,8 @@
 	dp = (struct dirent *)cpos;
 	cookiep = cookies;
-	while ((dp->d_fileno == 0 || dp->d_type == DT_WHT)
-	       && cpos < cend && ncookies > 0) {
+	while (cpos < cend && ncookies > 0
+	       && (dp->d_fileno == 0 || dp->d_type == DT_WHT)) {
 		cpos += dp->d_reclen;
 		dp = (struct dirent *)cpos;