Date: Tue, 7 Aug 2007 01:27:13 +0200 From: Tijl Coosemans <tijl@ulyssis.org> To: freebsd-current@freebsd.org, wine-freebsd@hub.org Subject: Re: mmap(2) MAP_FIXED isn't thread-safe (+testcase) Message-ID: <200708070127.15951.tijl@ulyssis.org> In-Reply-To: <200708051656.50168.tijl@ulyssis.org> References: <200708051656.50168.tijl@ulyssis.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_T56tGsgPgDAKRJI
Content-Type: text/plain;
charset="iso-8859-15"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
On Sunday 05 August 2007 16:56:46 Tijl Coosemans wrote:
> The problem is in sys/vm/vm_mmap.c:vm_mmap(). In case of MAP_FIXED
> first vm_map_remove() is called and then later vm_map_find(). This
> would need some locking, but I don't know which lock or how to
> approach this, so can somebody have a look at this?
I had another go at it today. I've attached a patch. It uses
vm_map_lock(), but to do that I made the lock recursive.
--Boundary-00=_T56tGsgPgDAKRJI
Content-Type: text/plain; charset="iso-8859-15"; name="patch-current-mmap"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="patch-current-mmap"
--- sys/vm/vm_mmap.c.orig 2007-07-30 23:58:11.000000000 +0200
+++ sys/vm/vm_mmap.c 2007-08-07 00:33:52.000000000 +0200
@@ -1341,7 +1341,6 @@ vm_mmap(vm_map_t map, vm_offset_t *addr,
if (*addr != trunc_page(*addr))
return (EINVAL);
fitit = FALSE;
- (void) vm_map_remove(map, *addr, *addr + size);
}
/*
* Lookup/allocate object.
@@ -1394,8 +1393,11 @@ vm_mmap(vm_map_t map, vm_offset_t *addr,
maxprot |= VM_PROT_EXECUTE;
#endif
+ vm_map_lock(map);
if (fitit)
*addr = pmap_addr_hint(object, *addr, size);
+ else
+ (void) vm_map_remove(map, *addr, *addr + size);
if (flags & MAP_STACK)
rv = vm_map_stack(map, *addr, size, prot, maxprot,
@@ -1403,6 +1405,7 @@ vm_mmap(vm_map_t map, vm_offset_t *addr,
else
rv = vm_map_find(map, object, foff, addr, size, fitit,
prot, maxprot, docow);
+ vm_map_unlock(map);
if (rv != KERN_SUCCESS) {
/*
--- sys/vm/vm_map.c.orig 2007-08-07 00:49:58.000000000 +0200
+++ sys/vm/vm_map.c 2007-08-07 00:44:47.000000000 +0200
@@ -215,8 +215,8 @@ vm_map_zinit(void *mem, int size, int fl
map = (vm_map_t)mem;
map->nentries = 0;
map->size = 0;
- mtx_init(&map->system_mtx, "system map", NULL, MTX_DEF | MTX_DUPOK);
- sx_init(&map->lock, "user map");
+ mtx_init(&map->system_mtx, "system map", NULL, MTX_DEF | MTX_DUPOK | MTX_RECURSE);
+ sx_init_flags(&map->lock, "user map", SX_RECURSE);
return (0);
}
@@ -590,8 +590,8 @@ void
vm_map_init(vm_map_t map, vm_offset_t min, vm_offset_t max)
{
_vm_map_init(map, min, max);
- mtx_init(&map->system_mtx, "system map", NULL, MTX_DEF | MTX_DUPOK);
- sx_init(&map->lock, "user map");
+ mtx_init(&map->system_mtx, "system map", NULL, MTX_DEF | MTX_DUPOK | MTX_RECURSE);
+ sx_init_flags(&map->lock, "user map", SX_RECURSE);
}
/*
--Boundary-00=_T56tGsgPgDAKRJI--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200708070127.15951.tijl>