From 7bcc2cfc86b51d4b2df8177a3af0a695433631f4 Mon Sep 17 00:00:00 2001 From: Andriy Gapon Date: Tue, 21 Nov 2017 18:28:14 +0000 Subject: [PATCH] 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(). Reported by: mav Analyzed by: mav Reviewed by: mav Pointyhat to: avg (myself) MFC after: 1 week X-MFC after: r326067 X-Erratum: wanted --- .../opensolaris/uts/common/fs/zfs/zfs_vnops.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c index fc8bdb3103df..cf4ec5c34084 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c @@ -1037,31 +1037,18 @@ zfs_write(vnode_t *vp, uio_t *uio, int ioflag, cred_t *cr, caller_context_t *ct) * 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 } /* @@ -1139,10 +1126,8 @@ zfs_write(vnode_t *vp, uio_t *uio, int ioflag, cred_t *cr, caller_context_t *ct) 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,