Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 20 Jul 2013 20:33:35 -0700
From:      Alan Cox <alc@rice.edu>
To:        Jeremie Le Hen <jlh@FreeBSD.org>
Cc:        Alan Cox <alc@FreeBSD.org>, Konstantin Belousov <kib@freebsd.org>, freebsd-current@FreeBSD.org, trasz@FreeBSD.org
Subject:   Re: Fix for sys_munlock(2) with racct
Message-ID:  <115EDC04-4B95-4FB5-9092-515188D8ADA7@rice.edu>
In-Reply-To: <20130720112218.GD13628@caravan.chchile.org>
References:  <20130720112218.GD13628@caravan.chchile.org>

next in thread | previous in thread | raw e-mail | index | archive | help

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

> Hi Edward, Alan,
>=20
> I plan to commit the following patch:
> http://people.freebsd.org/~jlh/racct_munlock.diff
>=20
> This solves the following panic:
>=20
> panic: racct_sub: freeing 301989888 of resource 5, which is more than =
allocated 73728 for pwsafe (pid 4177)
>=20
> 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.
>=20
> 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 =3D vm_map_wire(map, start, end,=20
            VM_MAP_WIRE_USER | VM_MAP_WIRE_NOHOLES);
#ifdef RACCT
        if (error !=3D 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.
=20
Alan




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?115EDC04-4B95-4FB5-9092-515188D8ADA7>