d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

zfs_write: fix problem with writes appearing to succeed when over quota

Browse Source 
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
This commit is contained in:
Andriy Gapon 2017-11-21 18:28:14 +00:00
parent 01dd1497ec
commit 7bcc2cfc86
1 changed files with 0 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
View File

@ -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,