From owner-svn-src-all@freebsd.org Fri Sep 20 20:47:11 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 6A2BE126BF6; Fri, 20 Sep 2019 20:47:11 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46Zm3z284Zz4cR7; Fri, 20 Sep 2019 20:47:11 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 282C134D2; Fri, 20 Sep 2019 20:47:11 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x8KKlBGV086361; Fri, 20 Sep 2019 20:47:11 GMT (envelope-from kevans@FreeBSD.org) Received: (from kevans@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x8KKlAX2086360; Fri, 20 Sep 2019 20:47:10 GMT (envelope-from kevans@FreeBSD.org) Message-Id: <201909202047.x8KKlAX2086360@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kevans set sender to kevans@FreeBSD.org using -f From: Kyle Evans Date: Fri, 20 Sep 2019 20:47:10 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r352564 - head/sys/fs/msdosfs X-SVN-Group: head X-SVN-Commit-Author: kevans X-SVN-Commit-Paths: head/sys/fs/msdosfs X-SVN-Commit-Revision: 352564 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Sep 2019 20:47:11 -0000 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.