Re: kern/38336: NULL deref in nfs_lookup

[yamt%mwd.biglobe.ne.jp@localhost (YAMAMOTO Takashi)]

The following reply was made to PR kern/38336; it has been noted by GNATS.

From: yamt%mwd.biglobe.ne.jp@localhost (YAMAMOTO Takashi)
To: gnats-bugs%NetBSD.org@localhost
Cc: kern-bug-people%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, 
netbsd-bugs%netbsd.org@localhost,
        ad%netbsd.org@localhost
Subject: Re: kern/38336: NULL deref in nfs_lookup
Date: Mon, 31 Mar 2008 11:46:58 +0900 (JST)

 >  On Sun, Mar 30, 2008 at 02:00:05PM +0000, YAMAMOTO Takashi wrote:
 >  
 >  >  > nfs_vnops.c:
 >  >  > 
 >  >  >     925        nfsm_request(np, NFSPROC_LOOKUP, curlwp, cnp->cn_cred);
 >  >  >     926        if (error) {
 >  >  >     927                nfsm_postop_attr(dvp, attrflag, 0);
 >  >  >     928                m_freem(mrep);
 >  >  >     929                goto nfsmout;
 >  >  >     930        }
 >  >  > 
 >  >  > nfsm_request() fills 'error' and typically 'md'. In case of error,
 >  >  > 'md' is sometimes not filled and can contain junk from the stack,
 >  >  > but nfsm_postop_attr() assumes that 'md' is always filled. Here is 
 >  >  
 >  >  can you be specific about "sometimes"?
 >  >  unless NFSERR_RETERR is set, nfsm_request macro itself does "goto 
 > nfsmout"
 >  >  so "if (error)" in the above code is not executed.
 >  
 >  I had a broken sbwait() that returned ERESTART when it should have been
 >  ignoring signals. I haven't checked to see if the error can happen in
 >  -current, but it seems better to handle it. I think the return path for the
 >  error code was:
 >  
 >  sbwait -> soreceive -> nfs_receive -> nfs_reply -> nfs_request -> nfs_lookup
 >  
 >  Andrew
 
 my understanding is that, if nfs_reply returns an error,
 nfs_request doesn't set NFSERR_RETERR, thus the above nfsm_postop_attr
 is not executed.
 
 YAMAMOTO Takashi