Author: kib
Date: Sun Mar 19 16:01:44 2017
New Revision: 315563
URL: https://svnweb.freebsd.org/changeset/base/315563

Log:
  MFC r313690:
  Consistently handle negative or wrapping offsets in the mmap(2) syscalls.
  
  MFC r315158:
  Fix two missed places where vm_object offset to index calculation
  should use unsigned shift.

Modified:
  stable/11/lib/libc/sys/mmap.2
  stable/11/sys/kern/uipc_shm.c
  stable/11/sys/kern/vfs_vnops.c
  stable/11/sys/vm/device_pager.c
  stable/11/sys/vm/sg_pager.c
  stable/11/sys/vm/vm_map.c
  stable/11/sys/vm/vm_object.h
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/lib/libc/sys/mmap.2
==============================================================================
--- stable/11/lib/libc/sys/mmap.2       Sun Mar 19 15:56:06 2017        
(r315562)
+++ stable/11/lib/libc/sys/mmap.2       Sun Mar 19 16:01:44 2017        
(r315563)
@@ -28,7 +28,7 @@
 .\"    @(#)mmap.2      8.4 (Berkeley) 5/11/95
 .\" $FreeBSD$
 .\"
-.Dd November 25, 2016
+.Dd February 4, 2017
 .Dt MMAP 2
 .Os
 .Sh NAME
@@ -53,11 +53,37 @@ starting at byte offset
 .Fa offset .
 If
 .Fa len
-is not a multiple of the pagesize, the mapped region may extend past the
+is not a multiple of the page size, the mapped region may extend past the
 specified range.
 Any such extension beyond the end of the mapped object will be zero-filled.
 .Pp
 If
+.Fa fd
+references a regular file or a shared memory object, the range of
+bytes starting at
+.Fa offset
+and continuing for
+.Fa len
+bytes must be legitimate for the possible (not necessarily
+current) offsets in the object.
+In particular, the
+.Fa offset
+value cannot be negative.
+If the object is truncated and the process later accesses a pages that
+is wholly within the truncated region, the access is aborted and a
+.Dv SIGBUS
+signal is delivered to the process.
+.Pp
+If
+.Fa fd
+references a device file, the interpretation of the
+.Fa offset
+value is device specific and defined by the device driver.
+The virtual memory subsystem does not impose any restrictitions on the
+.Fa offset
+value in this case, passing it unchanged to the driver.
+.Pp
+If
 .Fa addr
 is non-zero, it is used as a hint to the system.
 (As a convenience to the system, the actual address of the region may differ
@@ -157,7 +183,7 @@ If
 .Dv MAP_FIXED
 is specified,
 .Fa addr
-must be a multiple of the pagesize.
+must be a multiple of the page size.
 If
 .Dv MAP_EXCL
 is not specified, a successful
@@ -361,6 +387,12 @@ The
 argument
 is not a valid open file descriptor.
 .It Bq Er EINVAL
+An invalid (negative) value was passed in the
+.Fa offset
+argument, when
+.Fa fd
+referenced a regular file or shared memory.
+.It Bq Er EINVAL
 An invalid value was passed in the
 .Fa prot
 argument.

Modified: stable/11/sys/kern/uipc_shm.c
==============================================================================
--- stable/11/sys/kern/uipc_shm.c       Sun Mar 19 15:56:06 2017        
(r315562)
+++ stable/11/sys/kern/uipc_shm.c       Sun Mar 19 16:01:44 2017        
(r315563)
@@ -886,20 +886,20 @@ shm_mmap(struct file *fp, vm_map_t map, 
                return (EACCES);
        maxprot &= cap_maxprot;
 
+       /* See comment in vn_mmap(). */
+       if (
+#ifdef _LP64
+           objsize > OFF_MAX ||
+#endif
+           foff < 0 || foff > OFF_MAX - objsize)
+               return (EINVAL);
+
 #ifdef MAC
        error = mac_posixshm_check_mmap(td->td_ucred, shmfd, prot, flags);
        if (error != 0)
                return (error);
 #endif
        
-       /*
-        * XXXRW: This validation is probably insufficient, and subject to
-        * sign errors.  It should be fixed.
-        */
-       if (foff >= shmfd->shm_size ||
-           foff + objsize > round_page(shmfd->shm_size))
-               return (EINVAL);
-
        mtx_lock(&shm_timestamp_lock);
        vfs_timestamp(&shmfd->shm_atime);
        mtx_unlock(&shm_timestamp_lock);

Modified: stable/11/sys/kern/vfs_vnops.c
==============================================================================
--- stable/11/sys/kern/vfs_vnops.c      Sun Mar 19 15:56:06 2017        
(r315562)
+++ stable/11/sys/kern/vfs_vnops.c      Sun Mar 19 16:01:44 2017        
(r315563)
@@ -2452,6 +2452,24 @@ vn_mmap(struct file *fp, vm_map_t map, v
        }
        maxprot &= cap_maxprot;
 
+       /*
+        * For regular files and shared memory, POSIX requires that
+        * the value of foff be a legitimate offset within the data
+        * object.  In particular, negative offsets are invalid.
+        * Blocking negative offsets and overflows here avoids
+        * possible wraparound or user-level access into reserved
+        * ranges of the data object later.  In contrast, POSIX does
+        * not dictate how offsets are used by device drivers, so in
+        * the case of a device mapping a negative offset is passed
+        * on.
+        */
+       if (
+#ifdef _LP64
+           size > OFF_MAX ||
+#endif
+           foff < 0 || foff > OFF_MAX - size)
+               return (EINVAL);
+
        writecounted = FALSE;
        error = vm_mmap_vnode(td, size, prot, &maxprot, &flags, vp,
            &foff, &object, &writecounted);

Modified: stable/11/sys/vm/device_pager.c
==============================================================================
--- stable/11/sys/vm/device_pager.c     Sun Mar 19 15:56:06 2017        
(r315562)
+++ stable/11/sys/vm/device_pager.c     Sun Mar 19 16:01:44 2017        
(r315563)
@@ -139,8 +139,18 @@ cdev_pager_allocate(void *handle, enum o
        if (foff & PAGE_MASK)
                return (NULL);
 
+       /*
+        * Treat the mmap(2) file offset as an unsigned value for a
+        * device mapping.  This, in effect, allows a user to pass all
+        * possible off_t values as the mapping cookie to the driver.  At
+        * this point, we know that both foff and size are a multiple
+        * of the page size.  Do a check to avoid wrap.
+        */
        size = round_page(size);
-       pindex = OFF_TO_IDX(foff + size);
+       pindex = UOFF_TO_IDX(foff) + UOFF_TO_IDX(size);
+       if (pindex > OBJ_MAX_SIZE || pindex < UOFF_TO_IDX(foff) ||
+           pindex < UOFF_TO_IDX(size))
+               return (NULL);
 
        if (ops->cdev_pg_ctor(handle, size, prot, foff, cred, &color) != 0)
                return (NULL);

Modified: stable/11/sys/vm/sg_pager.c
==============================================================================
--- stable/11/sys/vm/sg_pager.c Sun Mar 19 15:56:06 2017        (r315562)
+++ stable/11/sys/vm/sg_pager.c Sun Mar 19 16:01:44 2017        (r315563)
@@ -96,8 +96,9 @@ sg_pager_alloc(void *handle, vm_ooffset_
         * to map beyond that.
         */
        size = round_page(size);
-       pindex = OFF_TO_IDX(foff + size);
-       if (pindex > npages)
+       pindex = UOFF_TO_IDX(foff) + UOFF_TO_IDX(size);
+       if (pindex > npages || pindex < UOFF_TO_IDX(foff) ||
+           pindex < UOFF_TO_IDX(size))
                return (NULL);
 
        /*

Modified: stable/11/sys/vm/vm_map.c
==============================================================================
--- stable/11/sys/vm/vm_map.c   Sun Mar 19 15:56:06 2017        (r315562)
+++ stable/11/sys/vm/vm_map.c   Sun Mar 19 16:01:44 2017        (r315563)
@@ -4124,7 +4124,7 @@ RetryLookup:;
         * Return the object/offset from this entry.  If the entry was
         * copy-on-write or empty, it has been fixed up.
         */
-       *pindex = OFF_TO_IDX((vaddr - entry->start) + entry->offset);
+       *pindex = UOFF_TO_IDX((vaddr - entry->start) + entry->offset);
        *object = entry->object.vm_object;
 
        *out_prot = prot;
@@ -4205,7 +4205,7 @@ vm_map_lookup_locked(vm_map_t *var_map,   
         * Return the object/offset from this entry.  If the entry was
         * copy-on-write or empty, it has been fixed up.
         */
-       *pindex = OFF_TO_IDX((vaddr - entry->start) + entry->offset);
+       *pindex = UOFF_TO_IDX((vaddr - entry->start) + entry->offset);
        *object = entry->object.vm_object;
 
        *out_prot = prot;

Modified: stable/11/sys/vm/vm_object.h
==============================================================================
--- stable/11/sys/vm/vm_object.h        Sun Mar 19 15:56:06 2017        
(r315562)
+++ stable/11/sys/vm/vm_object.h        Sun Mar 19 16:01:44 2017        
(r315563)
@@ -195,8 +195,23 @@ struct vm_object {
 #define        OBJ_DISCONNECTWNT 0x4000        /* disconnect from vnode wanted 
*/
 #define        OBJ_TMPFS       0x8000          /* has tmpfs vnode allocated */
 
+/*
+ * Helpers to perform conversion between vm_object page indexes and offsets.
+ * IDX_TO_OFF() converts an index into an offset.
+ * OFF_TO_IDX() converts an offset into an index.  Since offsets are signed
+ *   by default, the sign propagation in OFF_TO_IDX(), when applied to
+ *   negative offsets, is intentional and returns a vm_object page index
+ *   that cannot be created by a userspace mapping.
+ * UOFF_TO_IDX() treats the offset as an unsigned value and converts it
+ *   into an index accordingly.  Use it only when the full range of offset
+ *   values are allowed.  Currently, this only applies to device mappings.
+ * OBJ_MAX_SIZE specifies the maximum page index corresponding to the
+ *   maximum unsigned offset.
+ */
 #define        IDX_TO_OFF(idx) (((vm_ooffset_t)(idx)) << PAGE_SHIFT)
 #define        OFF_TO_IDX(off) ((vm_pindex_t)(((vm_ooffset_t)(off)) >> 
PAGE_SHIFT))
+#define        UOFF_TO_IDX(off) (((vm_pindex_t)(off)) >> PAGE_SHIFT)
+#define        OBJ_MAX_SIZE    (UOFF_TO_IDX(UINT64_MAX) + 1)
 
 #ifdef _KERNEL
 
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to