From owner-freebsd-current@FreeBSD.ORG Mon Aug 6 23:27:19 2007 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1D2DB16A417 for ; Mon, 6 Aug 2007 23:27:19 +0000 (UTC) (envelope-from tijl@ulyssis.org) Received: from mailrelay004.isp.belgacom.be (mailrelay004.isp.belgacom.be [195.238.6.170]) by mx1.freebsd.org (Postfix) with ESMTP id B0D1A13C46E for ; Mon, 6 Aug 2007 23:27:18 +0000 (UTC) (envelope-from tijl@ulyssis.org) Received: from 77.206-245-81.adsl-dyn.isp.belgacom.be (HELO kalimero.kotnet.org) ([81.245.206.77]) by mailrelay004.isp.belgacom.be with ESMTP; 07 Aug 2007 01:27:17 +0200 Received: from localhost (localhost [127.0.0.1]) by kalimero.kotnet.org (8.14.1/8.14.1) with ESMTP id l76NRGVC001214; Tue, 7 Aug 2007 01:27:16 +0200 (CEST) (envelope-from tijl@ulyssis.org) From: Tijl Coosemans To: freebsd-current@freebsd.org, wine-freebsd@hub.org Date: Tue, 7 Aug 2007 01:27:13 +0200 User-Agent: KMail/1.9.7 References: <200708051656.50168.tijl@ulyssis.org> In-Reply-To: <200708051656.50168.tijl@ulyssis.org> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_T56tGsgPgDAKRJI" Message-Id: <200708070127.15951.tijl@ulyssis.org> Cc: Subject: Re: mmap(2) MAP_FIXED isn't thread-safe (+testcase) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Aug 2007 23:27:19 -0000 --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--