Author: mjg
Date: Mon Feb 3 14:28:31 2020
New Revision: 357447
URL: https://svnweb.freebsd.org/changeset/base/357447
Log:
fd: fix f_count acquire in fget_unlocked
The code was using a hand-rolled fcmpset loop, while in other places the same
count is manipulated with the refcount API.
This transferred from a stylistic issue into a bug after the API got extended
to support flags. As a result the hand-rolled loop could bump the count high
enough to set the bit flag. Another bump + refcount_release would then free
the file prematurely.
The bug is only present in -CURRENT.
Modified:
head/sys/kern/kern_descrip.c
Modified: head/sys/kern/kern_descrip.c
==============================================================================
--- head/sys/kern/kern_descrip.c Mon Feb 3 14:25:32 2020
(r357446)
+++ head/sys/kern/kern_descrip.c Mon Feb 3 14:28:31 2020
(r357447)
@@ -2693,7 +2693,6 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights
#endif
const struct fdescenttbl *fdt;
struct file *fp;
- u_int count;
#ifdef CAPABILITIES
seqc_t seq;
cap_rights_t haverights;
@@ -2729,27 +2728,27 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights
if (error != 0)
return (error);
#endif
- count = fp->f_count;
- retry:
- if (count == 0) {
+ if
(__predict_false(!refcount_acquire_if_not_zero(&fp->f_count))) {
/*
+ * The count was found either saturated or zero.
+ * This re-read is not any more racy than using the
+ * return value from fcmpset.
+ */
+ if (fp->f_count != 0)
+ return (EBADF);
+ /*
* Force a reload. Other thread could reallocate the
- * table before this fd was closed, so it possible that
- * there is a stale fp pointer in cached version.
+ * table before this fd was closed, so it is possible
+ * that there is a stale fp pointer in cached version.
*/
fdt = (struct fdescenttbl
*)atomic_load_ptr(&fdp->fd_files);
continue;
}
- if (__predict_false(count + 1 < count))
- return (EBADF);
-
/*
* Use an acquire barrier to force re-reading of fdt so it is
* refreshed for verification.
*/
- if (__predict_false(atomic_fcmpset_acq_int(&fp->f_count,
- &count, count + 1) == 0))
- goto retry;
+ atomic_thread_fence_acq();
fdt = fdp->fd_files;
#ifdef CAPABILITIES
if (seqc_consistent_nomb(fd_seqc(fdt, fd), seq))
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
