From owner-freebsd-current@FreeBSD.ORG Mon Aug 6 11:38:46 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 93D9C16A417 for ; Mon, 6 Aug 2007 11:38:46 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from relay01.kiev.sovam.com (relay01.kiev.sovam.com [62.64.120.200]) by mx1.freebsd.org (Postfix) with ESMTP id 2B56313C459 for ; Mon, 6 Aug 2007 11:38:46 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from [89.162.146.170] (helo=skuns.kiev.zoral.com.ua) by relay01.kiev.sovam.com with esmtps (TLSv1:AES256-SHA:256) (Exim 4.67) (envelope-from ) id 1II0v5-000E2e-2f for freebsd-current@freebsd.org; Mon, 06 Aug 2007 14:38:44 +0300 Received: from deviant.kiev.zoral.com.ua (root@[10.1.1.148]) by skuns.kiev.zoral.com.ua (8.14.1/8.14.1) with ESMTP id l76BcNga054330 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 6 Aug 2007 14:38:23 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.1/8.14.1) with ESMTP id l76BcNnH068873; Mon, 6 Aug 2007 14:38:23 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.1/8.14.1/Submit) id l76BcLf6068872; Mon, 6 Aug 2007 14:38:21 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 6 Aug 2007 14:38:21 +0300 From: Kostik Belousov To: Tijl Coosemans Message-ID: <20070806113821.GB2738@deviant.kiev.zoral.com.ua> References: <200708051656.50168.tijl@ulyssis.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="2bJ57vwr75KGnr5s" Content-Disposition: inline In-Reply-To: <200708051656.50168.tijl@ulyssis.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: ClamAV version 0.91.1, clamav-milter version 0.91.1 on skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-1.4 required=5.0 tests=ALL_TRUSTED autolearn=failed version=3.2.1 X-Spam-Checker-Version: SpamAssassin 3.2.1 (2007-05-02) on skuns.kiev.zoral.com.ua X-Scanner-Signature: ad39b63ab710533149c2a2019fd816be X-DrWeb-checked: yes X-SpamTest-Envelope-From: kostikbel@gmail.com X-SpamTest-Group-ID: 00000000 X-SpamTest-Header: Not Detected X-SpamTest-Info: Profiles 1336 [August 3 2007] X-SpamTest-Info: helo_type=3 X-SpamTest-Method: none X-SpamTest-Rate: 0 X-SpamTest-Status: Not detected X-SpamTest-Status-Extended: not_detected X-SpamTest-Version: SMTP-Filter Version 3.0.0 [0255], KAS30/Release Cc: wine-freebsd@hub.org, freebsd-current@freebsd.org 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 11:38:46 -0000 --2bJ57vwr75KGnr5s Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable [I removed personal addresses from Cc:] On Sun, Aug 05, 2007 at 04:56:46PM +0200, Tijl Coosemans wrote: > Hi all, >=20 > While investigating ports/115092 and other reports of seemingly random > page faults when running Wine, I think I've found the cause to be mmap > not being thread-safe when MAP_FIXED is used. It causes mmap(MAP_FIXED) > to return -1(ENOMEM) sometimes when it shouldn't, but also to return an > address with wrong protections, hence the protection faults occuring. >=20 > Attached is a test program that shows this. It runs two threads. The > first mmap()'s a region, starts a second thread and then goes in a loop > calling mmap(PROT_WRITE,MAP_FIXED) on that region, essentially > replacing that mapping. This is basically what rtld does to map an ELF > object for instance when dlopen(3) is called. The second thread tries > to steal the mapping from the first by calling mmap(PROT_NONE) in a > loop. After a while the program segfaults when the first thread tries > to write to the mapped region. >=20 > Some lines are commented out. If you remove the commenting, I hit on > the case where mmap(MAP_FIXED) returns -1. >=20 > 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? > #include > #include > #include > #include >=20 > #define MAPSIZE ( 16 * 4096 ) >=20 > void *second_thr( void *addr ) { > int i; > void *addr2; > for( i =3D 0; ; i++ ) { > addr2 =3D mmap( NULL, MAPSIZE, PROT_NONE, MAP_ANON | MAP_PRIVATE, -1, 0= ); > /* > if( addr2 !=3D ( addr + MAPSIZE )) > break; > */ > munmap( addr2, MAPSIZE ); > } > printf( "thread2: addr(%p), addr2(%p), i(%d)\n", addr, addr2, i ); > return NULL; > } >=20 > int main( int argc, char **argv ) { > int i; > void *addr; > volatile char *addr_fix; > pthread_t thr; >=20 > addr =3D mmap( NULL, MAPSIZE, PROT_NONE, MAP_ANON | MAP_PRIVATE, -1, 0 ); > pthread_create( &thr, NULL, &second_thr, addr ); >=20 > for( i =3D 0; ; i++ ) { > addr_fix =3D mmap( addr, MAPSIZE, PROT_WRITE, MAP_ANON | MAP_PRIVATE | = MAP_FIXED, -1, 0 ); > if( addr_fix =3D=3D addr ) > *addr_fix =3D i; /* should be writable */ > /* > else > break; > */ > } > printf( "thread1: addr(%p), addr_fix(%p), errno(%d), i(%d)\n", addr, add= r_fix, errno, i ); >=20 > pthread_join( thr, NULL ); > return 0; > } Try the patch below. It changes behaviour for MAP_STACK|MAP_FIXED case, but I am not sure whether it worth fixing. diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c index 8799cc5..b9dabe8 100644 --- a/sys/vm/vm_map.c +++ b/sys/vm/vm_map.c @@ -155,6 +155,22 @@ static void vmspace_zdtor(void *mem, int size, void *a= rg); #define PROC_VMSPACE_LOCK(p) do { } while (0) #define PROC_VMSPACE_UNLOCK(p) do { } while (0) =20 +/* + * VM_MAP_RANGE_CHECK: [ internal use only ] + * + * Asserts that the starting and ending region + * addresses fall within the valid range of the map. + */ +#define VM_MAP_RANGE_CHECK(map, start, end) \ + { \ + if (start < vm_map_min(map)) \ + start =3D vm_map_min(map); \ + if (end > vm_map_max(map)) \ + end =3D vm_map_max(map); \ + if (start > end) \ + start =3D end; \ + } + void vm_map_startup(void) { @@ -1160,7 +1176,7 @@ vm_map_find(vm_map_t map, vm_object_t object, vm_ooff= set_t offset, vm_size_t length, boolean_t find_space, vm_prot_t prot, vm_prot_t max, int cow) { - vm_offset_t start; + vm_offset_t start, end; int result; =20 start =3D *addr; @@ -1171,9 +1187,14 @@ vm_map_find(vm_map_t map, vm_object_t object, vm_oof= fset_t offset, return (KERN_NO_SPACE); } start =3D *addr; + end =3D start + length; + } else { + end =3D start + length; + VM_MAP_RANGE_CHECK(map, start, end); + (void) vm_map_delete(map, start, end); } result =3D vm_map_insert(map, object, offset, - start, start + length, prot, max, cow); + start, end, prot, max, cow); vm_map_unlock(map); return (result); } @@ -1355,22 +1376,6 @@ _vm_map_clip_end(vm_map_t map, vm_map_entry_t entry,= vm_offset_t end) } =20 /* - * VM_MAP_RANGE_CHECK: [ internal use only ] - * - * Asserts that the starting and ending region - * addresses fall within the valid range of the map. - */ -#define VM_MAP_RANGE_CHECK(map, start, end) \ - { \ - if (start < vm_map_min(map)) \ - start =3D vm_map_min(map); \ - if (end > vm_map_max(map)) \ - end =3D vm_map_max(map); \ - if (start > end) \ - start =3D end; \ - } - -/* * vm_map_submap: [ kernel use only ] * * Mark the given range as handled by a subordinate map. diff --git a/sys/vm/vm_mmap.c b/sys/vm/vm_mmap.c index 9522fec..a48fd77 100644 --- a/sys/vm/vm_mmap.c +++ b/sys/vm/vm_mmap.c @@ -1341,7 +1341,6 @@ vm_mmap(vm_map_t map, vm_offset_t *addr, vm_size_t si= ze, vm_prot_t prot, if (*addr !=3D trunc_page(*addr)) return (EINVAL); fitit =3D FALSE; - (void) vm_map_remove(map, *addr, *addr + size); } /* * Lookup/allocate object. --2bJ57vwr75KGnr5s Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.7 (FreeBSD) iD8DBQFGtwgtC3+MBN1Mb4gRAqanAKCgdwEDv2eKL8hZLyck6V7XwhG+dwCgrWi6 rtOjbPxmRr7XfVOlXDTd4eQ= =9J1w -----END PGP SIGNATURE----- --2bJ57vwr75KGnr5s--