From owner-svn-src-all@freebsd.org Fri Dec 1 11:14:16 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 214DBDFC65B; Fri, 1 Dec 2017 11:14:16 +0000 (UTC) (envelope-from avg@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 mx1.freebsd.org (Postfix) with ESMTPS id 850E07C5D3; Fri, 1 Dec 2017 11:14:15 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id vB1BEDc3095018; Fri, 1 Dec 2017 11:14:13 GMT (envelope-from avg@FreeBSD.org) Received: (from avg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id vB1BEDhI095017; Fri, 1 Dec 2017 11:14:13 GMT (envelope-from avg@FreeBSD.org) Message-Id: <201712011114.vB1BEDhI095017@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avg set sender to avg@FreeBSD.org using -f From: Andriy Gapon Date: Fri, 1 Dec 2017 11:14:13 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r326428 - stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs X-SVN-Group: stable-10 X-SVN-Commit-Author: avg X-SVN-Commit-Paths: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs X-SVN-Commit-Revision: 326428 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.25 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, 01 Dec 2017 11:14:16 -0000 Author: avg Date: Fri Dec 1 11:14:13 2017 New Revision: 326428 URL: https://svnweb.freebsd.org/changeset/base/326428 Log: MFC r326070: zfs_write: fix problem with writes appearing to succeed when over quota The problem happens when the writes have offsets and sizes aligned with a filesystem's recordsize (maximum block size). In this scenario dmu_tx_assign() would fail because of being over the quota, but the uio would already be modified in the code path where we copy data from the uio into a borrowed ARC buffer. That makes an appearance of a partial write, so zfs_write() would return success and the uio would be modified consistently with writing a single block. That bug can result in a data loss because the writes over the quota would appear to succeed while the actual data is being discarded. This commit fixes the bug by ensuring that the uio is not changed until after all error checks are done. To achieve that the code now uses uiocopy() + uioskip() as in the original illumos design. We can do that now that uiocopy() has been updated in r326067 to use vn_io_fault_uiomove(). Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c ============================================================================== --- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Fri Dec 1 11:13:58 2017 (r326427) +++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Fri Dec 1 11:14:13 2017 (r326428) @@ -1036,31 +1036,18 @@ zfs_write(vnode_t *vp, uio_t *uio, int ioflag, cred_t * holding up the transaction if the data copy hangs * up on a pagefault (e.g., from an NFS server mapping). */ -#ifdef illumos size_t cbytes; -#endif abuf = dmu_request_arcbuf(sa_get_db(zp->z_sa_hdl), max_blksz); ASSERT(abuf != NULL); ASSERT(arc_buf_size(abuf) == max_blksz); -#ifdef illumos if (error = uiocopy(abuf->b_data, max_blksz, UIO_WRITE, uio, &cbytes)) { dmu_return_arcbuf(abuf); break; } ASSERT(cbytes == max_blksz); -#else - ssize_t resid = uio->uio_resid; - error = vn_io_fault_uiomove(abuf->b_data, max_blksz, uio); - if (error != 0) { - uio->uio_offset -= resid - uio->uio_resid; - uio->uio_resid = resid; - dmu_return_arcbuf(abuf); - break; - } -#endif } /* @@ -1138,10 +1125,8 @@ zfs_write(vnode_t *vp, uio_t *uio, int ioflag, cred_t dmu_assign_arcbuf(sa_get_db(zp->z_sa_hdl), woff, abuf, tx); } -#ifdef illumos ASSERT(tx_bytes <= uio->uio_resid); uioskip(uio, tx_bytes); -#endif } if (tx_bytes && vn_has_cached_data(vp)) { update_pages(vp, woff, tx_bytes, zfsvfs->z_os,