>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
>Last-Modified:
>Originator:     Michael L. Hitch
>Organization:
Montana State University
>Release:        1.2_BETA
>Environment:
System: NetBSD amiga2.oscs.montana.edu 1.2B NetBSD 1.2B (ZEUS) #970120X-0: Mon Jan 20 17:56:34 MST 1997 mhitch@amiga2.oscs.montana.edu:/work/tmp/src/sys/arch/amiga/compile/ZEUS amiga


>Description:
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.

>How-To-Repeat:
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.

>Fix:
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;
 		cookiep++;
>Audit-Trail:
>Unformatted: