Avoid memory accesses reordering which can result in fget_unlocked()

seeing a stale fd_ofiles table once fd_nfiles is already updated,
resulting in OOB accesses.

Approved by:	re (kib)
Sponsored by:	EMC / Isilon storage division
Reported and tested by:	pho
Reviewed by:	benno
This commit is contained in:
Attilio Rao 2013-09-25 13:37:52 +00:00
parent c9b24e38e8
commit 57a9eeb4ed
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=255868

View File

@ -1512,11 +1512,19 @@ fdgrowtable(struct filedesc *fdp, int nfd)
memcpy(nmap, omap, NDSLOTS(onfiles) * sizeof(*omap));
/* update the pointers and counters */
fdp->fd_nfiles = nnfiles;
memcpy(ntable, otable, onfiles * sizeof(ntable[0]));
fdp->fd_ofiles = ntable;
fdp->fd_map = nmap;
/*
* In order to have a valid pattern for fget_unlocked()
* fdp->fd_nfiles might be the last member to be updated, otherwise
* fget_unlocked() consumers may reference a new, higher value for
* fdp->fd_nfiles before to access the fdp->fd_ofiles array,
* resulting in OOB accesses.
*/
atomic_store_rel_int(&fdp->fd_nfiles, nnfiles);
/*
* Do not free the old file table, as some threads may still
* reference entries within it. Instead, place it on a freelist
@ -2308,7 +2316,11 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
int error;
#endif
if (fd < 0 || fd >= fdp->fd_nfiles)
/*
* Avoid reads reordering and then a first access to the
* fdp->fd_ofiles table which could result in OOB operation.
*/
if (fd < 0 || fd >= atomic_load_acq_int(&fdp->fd_nfiles))
return (EBADF);
/*
* Fetch the descriptor locklessly. We avoid fdrop() races by