Re: Fix for sys_munlock(2) with racct

On Jul 20, 2013, at 4:22 AM, Jeremie Le Hen wrote:

> Hi Edward, Alan,
> 
> I plan to commit the following patch:
> http://people.freebsd.org/~jlh/racct_munlock.diff
> 
> This solves the following panic:
> 
> panic: racct_sub: freeing 301989888 of resource 5, which is more than allocated 73728 for pwsafe (pid 4177)
> 
> The problem is that the racct code in sys_munlock() trusts too much the
> user's input.  vm_map_unwire_count() now returns how much memory has
> really been unwired.
> 
> Any objection?


Can you elaborate on what you mean by "sys_munlock() trusts too much the user's input."   munlock(2) is supposed to return ENOMEM if any addresses within the specified range are not backed by valid mappings.  (Valid mappings with PROT_NONE access are something of a gray area here.)  However, it is not error for a to call munlock() on a range that isn't locked, or to call it a second, third, etc. time on the same range.  Is that what is provoking your panic?

By the way, sys_mlock() uses a simpler approach to deal with a similar situation:

        error = vm_map_wire(map, start, end, 
            VM_MAP_WIRE_USER | VM_MAP_WIRE_NOHOLES);
#ifdef RACCT
        if (error != KERN_SUCCESS) {
                PROC_LOCK(proc);
                racct_set(proc, RACCT_MEMLOCK,
                    ptoa(pmap_wired_count(map->pmap)));
                PROC_UNLOCK(proc);
        }
#endif

However, the code in sys_mlock() for maintaining RACCT_MEMLOCK, including the above snippet, is race-y.  Two simultaneous callers to sys_mlock() will produce incorrect results.  (I haven't looked at sys_mlockall() or vm_map_growstack().)

Also, a wired mapping can be destroyed by calling munmap(2) without first calling munlock(2), in which case, RACCT_MEMLOCK will be incorrect.
 
Alan