Author: attilio
Date: Wed Sep 25 13:37:52 2013
New Revision: 255868
URL: http://svnweb.freebsd.org/changeset/base/255868

Log:
  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

Modified:
  head/sys/kern/kern_descrip.c

Modified: head/sys/kern/kern_descrip.c
==============================================================================
--- head/sys/kern/kern_descrip.c        Wed Sep 25 02:49:18 2013        
(r255867)
+++ head/sys/kern/kern_descrip.c        Wed Sep 25 13:37:52 2013        
(r255868)
@@ -1512,12 +1512,20 @@ fdgrowtable(struct filedesc *fdp, int nf
        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 
        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
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to