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

fd: fix f_count acquire in fget_unlocked

Browse Source 
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.
This commit is contained in:
Mateusz Guzik 2020-02-03 14:28:31 +00:00
parent f1fa1ba3d0
commit fee204544e
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=357447
1 changed files with 11 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
sys/kern/kern_descrip.c
View File

@ -2693,7 +2693,6 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
#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_t *needrightsp,
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))