diff options
author | attilio <attilio@FreeBSD.org> | 2013-09-25 13:37:52 +0000 |
---|---|---|
committer | attilio <attilio@FreeBSD.org> | 2013-09-25 13:37:52 +0000 |
commit | 29d161240ea3c6786da8eaac6f6e04b60a3b2316 (patch) | |
tree | d6fc6244a4f5fc88d9e3927b1471630ed94a8754 | |
parent | 244d5cfa888ce4ebbf71aa49a4dfe0ca293be27c (diff) | |
download | FreeBSD-src-29d161240ea3c6786da8eaac6f6e04b60a3b2316.zip FreeBSD-src-29d161240ea3c6786da8eaac6f6e04b60a3b2316.tar.gz |
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
-rw-r--r-- | sys/kern/kern_descrip.c | 16 |
1 files changed, 14 insertions, 2 deletions
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c index 9e9010f..0c1c6b8 100644 --- a/sys/kern/kern_descrip.c +++ b/sys/kern/kern_descrip.c @@ -1512,12 +1512,20 @@ 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 * which will be processed when the struct filedesc is released. @@ -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 |