Date: Thu, 8 Aug 2019 19:53:07 +0000 (UTC) From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r350776 - head/sys/kern Message-ID: <201908081953.x78Jr7XW062674@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rmacklem Date: Thu Aug 8 19:53:07 2019 New Revision: 350776 URL: https://svnweb.freebsd.org/changeset/base/350776 Log: Fix copy_file_range(2) for an unlikely race during hole finding. Since the VOP_IOCTL(FIOSEEKDATA/FIOSEEKHOLE) calls are done with the vnode unlocked, it is possible for another thread to do: - truncate(), lseek(), write() between the two calls and create a hole where FIOSEEKDATA returned the start of data. For this case, VOP_IOCTL(FIOSEEKHOLE) will return the same offset for the hole location. This could result in an infinite loop in the copy code, since copylen is set to 0 and the copy doesn't advance. Usually, this race is avoided because of the use of rangelocks, but the NFS server does not do range locking and could do a sequence like the above to create the hole. This patch checks for this case and makes the hole search fail, to avoid the infinite loop. At this time, it is an open question as to whether or not the NFS server should do range locking to avoid this race. Modified: head/sys/kern/vfs_vnops.c Modified: head/sys/kern/vfs_vnops.c ============================================================================== --- head/sys/kern/vfs_vnops.c Thu Aug 8 18:52:53 2019 (r350775) +++ head/sys/kern/vfs_vnops.c Thu Aug 8 19:53:07 2019 (r350776) @@ -2895,6 +2895,17 @@ vn_generic_copy_file_range(struct vnode *invp, off_t * endoff = startoff; error = VOP_IOCTL(invp, FIOSEEKHOLE, &endoff, 0, incred, curthread); + /* + * Since invp is unlocked, it may be possible for + * another thread to do a truncate(), lseek(), write() + * creating a hole at startoff between the above + * VOP_IOCTL() calls, if the other thread does not do + * rangelocking. + * If that happens, startoff == endoff and finding + * the hole has failed, so set an error. + */ + if (error == 0 && startoff == endoff) + error = EINVAL; /* Any error. Reset to 0. */ } if (error == 0) { if (startoff > *inoffp) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201908081953.x78Jr7XW062674>