Re: [vfs_cache] Re: CVS commit: src/sys/kern

[Andrew Doran <ad%netbsd.org@localhost>]

On Sun, Mar 29, 2020 at 11:41:23AM +0200, Maxime Villard wrote:
> Le 23/03/2020 ? 21:02, Andrew Doran a ?crit?:
> > Module Name:	src
> > Committed By:	ad
> > Date:		Mon Mar 23 20:02:14 UTC 2020
> > 
> > Modified Files:
> > 	src/sys/kern: vfs_cache.c
> > 
> > Log Message:
> > cache_remove(): remove from the vnode list first, so cache_revlookup()
> > doesn't try to re-activate an entry no longer on the LRU list.
> > 
> > 
> > To generate a diff of this commit:
> > cvs rdiff -u -r1.133 -r1.134 src/sys/kern/vfs_cache.c
> > 
> > Please note that diffs are not public domain; they are subject to the
> > copyright notices on the relevant files.
> 
> It appears that your recent changes in vfs_cache.c have introduced a
> use-after-free. Booting KASAN gives:
> 
> 	ASan: Unauthorized Access In 0x...: Addr 0x... [1 byte, read, PoolUseAfterFree]
> 
> It seems that the problem is here:
> 
> 557 		if (nameiop == CREATE && (cnflags & ISLASTCN) != 0) {
> 558 			/*
> 559 			 * Last component and we are preparing to create
> 560 			 * the named object, so flush the negative cache
> 561 			 * entry.
> 562 			 */
> 563 			COUNT(ncs_badhits);
> 564 			cache_remove(ncp, true);    <-------- HERE
> 565 			hit = false;
> 566 		} else {
> 567 			COUNT(ncs_neghits);
> 568 			SDT_PROBE(vfs, namecache, lookup, hit, dvp, name,
> 569 			    namelen, 0, 0);
> 570 			/* found neg entry; vn is already null from above */
> 571 			hit = true;
> 572 		}
> 573 		if (iswht_ret != NULL) {
> 574 			/*
> 575 			 * Restore the ISWHITEOUT flag saved earlier.
> 576 			 */
> 577 			*iswht_ret = ncp->nc_whiteout;  <------ ouch
> 578 		} else {
> 579 			KASSERT(!ncp->nc_whiteout);  <------ ouch
> 580 		}
> 
> cache_remove() frees 'ncp', and then 'ncp->nc_whiteout' is read.

Fixed with vfs_cache.c 1.136.  Thank you for bringing it to my attention.

Andrew