Subject: Re: kern/35728: repeated kernel panics: free: duplicated free (NFS-related)
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Antti Kantee <pooka@cs.hut.fi>
List: netbsd-bugs
Date: 02/20/2007 14:05:03
The following reply was made to PR kern/35728; it has been noted by GNATS.

From: Antti Kantee <pooka@cs.hut.fi>
To: Arto Selonen <arto+dated+1171977722.42cefd5529de6cc7@selonen.org>
Cc: gnats-bugs@NetBSD.org, christos@netbsd.org
Subject: Re: kern/35728: repeated kernel panics: free: duplicated free (NFS-related)
Date: Tue, 20 Feb 2007 16:00:40 +0200

 On Tue Feb 20 2007 at 15:20:59 +0200, Arto Selonen wrote:
 > >diff -u -r1.123 nfs_serv.c
 > >--- nfs_serv.c	4 Feb 2007 14:48:51 -0000	1.123
 > >+++ nfs_serv.c	20 Feb 2007 12:07:16 -0000
 > >@@ -2706,6 +2706,7 @@
 > >		toff = off;
 > >		siz = fullsiz;
 > >		free(cookies, M_TEMP);
 > >+		cookies = NULL;
 > >		goto again;
 > >	}
 > >
 > >@@ -2975,6 +2976,7 @@
 > >		toff = off;
 > >		siz = fullsiz;
 > >		free(cookies, M_TEMP);
 > >+		cookies = NULL;
 > >		goto again;
 > >	}
 > >
 > 
 > Did not make any practical difference that I could tell. Debugger trace
 > seemed the same, too.
 
 Hmm.... could you provide the source code line it crashes on, if you
 have a dump with symbols?  It would help a bit in trying to figure out
 which free() is to blame.
 
 Ok, my next guess is that ufs_readdir() sets cookies, but frees it because
 of an error.  I am unsure what the exact semantics are supposed to be,
 but returning an error and cookies pointing to garbage can't be a good
 idea.
 (I'm assuming, of course, that you are serving off of ffs)
 
 Index: ufs_vnops.c
 ===================================================================
 RCS file: /cvsroot/src/sys/ufs/ufs/ufs_vnops.c,v
 retrieving revision 1.149
 diff -u -r1.149 ufs_vnops.c
 --- ufs_vnops.c	29 Jan 2007 15:42:50 -0000	1.149
 +++ ufs_vnops.c	20 Feb 2007 13:58:53 -0000
 @@ -1787,10 +1787,12 @@
  	error = uiomove(ndbuf, count, uio);
  out:
  	if (ap->a_cookies) {
 -		if (error)
 +		if (error) {
  			free(*(ap->a_cookies), M_TEMP);
 -		else
 +			*(ap->a_cookies) = NULL;
 +		} else {
  			*ap->a_ncookies = ccp - *(ap->a_cookies);
 +		}
  	}
  	uio->uio_offset = off;
  	free(ndbuf, M_TEMP);
 
 -- 
 Antti Kantee <pooka@iki.fi>                     Of course he runs NetBSD
 http://www.iki.fi/pooka/                          http://www.NetBSD.org/
     "la qualité la plus indispensable du cuisinier est l'exactitude"