svn commit: r352564 - head/sys/fs/msdosfs

[Kyle Evans]

Author: kevans
Date: Fri Sep 20 20:47:10 2019
New Revision: 352564
URL: https://svnweb.freebsd.org/changeset/base/352564

Log:
  msdosfs: do not deget unlinked denodes
  
  When a file is unlinked, the denode is not reclaimed until the last
  reference is dropped, but the directory entry is immediately up for reuse.
  This is a problem later when createde goes to grab a denode for the newly
  created entry -- we search the hash and find a dead denode, then return that
  without even bumping the reference count and the data later gets truncated
  when the the last reference to the unlinked file is dropped.
  
  This manifested itself as a broken in-place strip(1) on msdosfs. elfcopy
  will do a sequence incredibly roughly like this:
  
  open("/mnt/foo", ...) => fd 3
  mmap()
  unlink("/mnt/foo")
  open("/mnt/foo", ...) => fd 4
  write(4, ...)
  close(4)
  close(3)
  
  and the resulting file would be truncated, but the write succeeded, as long
  as a reference to the unlinked file had not been closed.
  
  Some archaeology indicates that this bug has likely existed since msdosfs
  was converted to use vfs_hash instead of a home rolled hash implementation
  in r143570. Prior to that point, the hashget implementation would do a
  refcnt check while searching and explicitly only return a denode with
  de_refcnt != 0. vfs_hash did not yet have the callback that it does today,
  so this slipped away and did not come back when it later grew that
  functionality.
  
  The comment indicating that we want to skip these denodes has been updated
  to reflect where this is actually done. My repo-diving session seems to
  indicate that the refcnt check was likely never actually below the comment,
  to be pedantic, but instead a detail wrapped up in the hashget
  implementation since the beginning of its inclusion into FreeBSD.
  
  This bug was the cause behind the issue addressed in r352557.
  
  Reported by:	jhibbits
  Reviewed by:	kib
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D21731

Modified:
  head/sys/fs/msdosfs/msdosfs_denode.c

Modified: head/sys/fs/msdosfs/msdosfs_denode.c
==============================================================================
--- head/sys/fs/msdosfs/msdosfs_denode.c	Fri Sep 20 19:56:55 2019	(r352563)
+++ head/sys/fs/msdosfs/msdosfs_denode.c	Fri Sep 20 20:47:10 2019	(r352564)
@@ -79,7 +79,7 @@ de_vncmpf(struct vnode *vp, void *arg)
 
 	a = arg;
 	de = VTODE(vp);
-	return (de->de_inode != *a);
+	return (de->de_inode != *a) || (de->de_refcnt <= 0);
 }
 
 /*
@@ -124,8 +124,9 @@ deget(struct msdosfsmount *pmp, u_long dirclust, u_lon
 	 * address of "." entry. For root dir (if not FAT32) use cluster
 	 * MSDOSFSROOT, offset MSDOSFSROOT_OFS
 	 *
-	 * NOTE: The check for de_refcnt > 0 below insures the denode being
-	 * examined does not represent an unlinked but still open file.
+	 * NOTE: de_vncmpf will explicitly skip any denodes that do not have
+	 * a de_refcnt > 0.  This insures that that we do not attempt to use
+	 * a denode that represents an unlinked but still open file.
 	 * These files are not to be accessible even when the directory
 	 * entry that represented the file happens to be reused while the
 	 * deleted file is still open.