From owner-freebsd-hackers@FreeBSD.ORG Sat Jun 28 16:10:00 2014 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id E7969E63; Sat, 28 Jun 2014 16:09:59 +0000 (UTC) Received: from mail-qa0-x22f.google.com (mail-qa0-x22f.google.com [IPv6:2607:f8b0:400d:c00::22f]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8E41C2233; Sat, 28 Jun 2014 16:09:59 +0000 (UTC) Received: by mail-qa0-f47.google.com with SMTP id hw13so5097093qab.34 for ; Sat, 28 Jun 2014 09:09:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=Y72H81e1jIUJPvjhjkIsCD2fTunjKoJK+GjCHI80Iqs=; b=v5qO13pm6MHquoLOpHY+ujnHwl2NY0DJ765lRd0pTHzYNsdZ/VSXtr0F3cfDu6hd4B 6+gR1qYfJBndeN2vQfOv0lNWjoeWVEoOMkl1D74KWEl1jWYHPREicijbrg4PIAVECUc7 diUHwlskdpbluQ6/f8/Hd58551l7pIwgvdsC6VUWDHQbB2VZg7FPwddeV36hNlG13ZYa d1HWJ4zJW8lDYd3W2+X4llPEn/bKeEuCzfCMIp+T0r04UjmyyCWk3WHRVvI88uavlqK8 dyvmkTmSXby9GEi+zdnGt8/W3mJ605BqCSStVXt1eFgSr2iRql2eEDfpB2ktrV2wvUut 8JVg== X-Received: by 10.229.204.200 with SMTP id fn8mr43326120qcb.14.1403971798695; Sat, 28 Jun 2014 09:09:58 -0700 (PDT) Received: from pwnie.vrt.sourcefire.com (moist.vrt.sourcefire.com. [198.148.79.134]) by mx.google.com with ESMTPSA id g18sm17371886qaa.33.2014.06.28.09.09.57 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 28 Jun 2014 09:09:58 -0700 (PDT) Date: Sat, 28 Jun 2014 12:09:56 -0400 From: Shawn Webb To: Alan Cox Subject: Re: Help With ASLR Message-ID: <20140628160956.GB4365@pwnie.vrt.sourcefire.com> References: <20140626232727.GB1825@pwnie.vrt.sourcefire.com> <53ACE5B4.8070700@rice.edu> <20140627132810.GA8396@pwnie.vrt.sourcefire.com> <53AD976A.2000503@rice.edu> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="WhfpMioaduB5tiZL" Content-Disposition: inline In-Reply-To: <53AD976A.2000503@rice.edu> X-PGP-Key: http://pgp.mit.edu/pks/lookup?op=vindex&search=0x6A84658F52456EEE User-Agent: Mutt/1.5.23 (2014-03-12) Cc: freebsd-hackers@freebsd.org, kib@freebsd.org, Oliver Pinter X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 28 Jun 2014 16:10:00 -0000 --WhfpMioaduB5tiZL Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Jun 27, 2014 11:10 AM -0500, Alan Cox wrote: > On 06/27/2014 08:28, Shawn Webb wrote: > > On Jun 26, 2014 10:32 PM -0500, Alan Cox wrote: > >> On 06/26/2014 18:27, Shawn Webb wrote: > >>> Hey All, > >>> > >>> I've exchanged a few helpful emails with kib@. He has glanced over the > >>> ASLR patch we have against 11-CURRENT (which is also attached to this > >>> email as a reference point). He has suggested some changes to the VM > >>> subsystem that I'm having trouble getting my head wrapped around. > >>> > >>> Essentially, he suggests that instead of doing the randomization in > >>> various places like the ELF loader and sys_mmap, we should modify > >>> vm_map_find() in sys/vm/vm_map.c to apply the randomization there. > >>> > >>> Here's his suggestion in full (I hope he doesn't mind me directly > >>> quoting him): > >>> > >>> The mapping address randomization can only be performed in the > >>> vm_map_find(). This is the place where usermode mapping flags are > >>> already parsed, and the real decision for the placement is done, with > >>> all the contextual information collected, the fixed requests are not > >>> passed there. Doing the 'randomization' in vm_mmap() means that the > >>> mmap command must be parsed twice, which presented patch fails to do > >>> in any sane way. Only mappings in the usermode maps should be > >>> randomized. > >>> > >>> Current patch does not randomize the mapping location at all, it only > >>> randomizes the hint address passed from the userspace, which is > >>> completely useless. > >> > >> Kostik, I'm not sure that I understand what you're saying here. The > >> randomly generated offset is added to the variable "addr" after this > >> block of code has been executed: > >> > >> } else { > >> /* > >> * XXX for non-fixed mappings where no hint is > provided or > >> * the hint would fall in the potential heap space, > >> * place it after the end of the largest possible heap. > >> * > >> * There should really be a pmap call to determine a > >> reasonable > >> * location. > >> */ > >> PROC_LOCK(td->td_proc); > >> if (addr =3D=3D 0 || > >> (addr >=3D round_page((vm_offset_t)vms->vm_taddr) = && > >> addr < round_page((vm_offset_t)vms->vm_daddr + > >> lim_max(td->td_proc, RLIMIT_DATA)))) > >> addr =3D round_page((vm_offset_t)vms->vm_daddr= + > >> lim_max(td->td_proc, RLIMIT_DATA)); > >> PROC_UNLOCK(td->td_proc); > >> } > >> > >> So, the point at which the eventual call to vm_map_findspace() starts > >> looking for free space is determined by the randomization, and thus the > >> chosen address will be influenced by the randomization. The first > >> mmap() that I do in one execution of the program will be unlikely to > >> have the same address in the next execution. > >> > >> That said, I think that this block of code would be a better place for > >> the pax_aslr_mmap() call than were it currently resides. Moving it he= re > >> would address the problem with MAP_32BIT mentioned below. > >> > >> > >>> ... It also fails to correct the address to honour > >>> the placement flags like MAP_32BIT or MAP_ALIGNMENT_MASK. > >> > >> > >> I think that MAP_32BIT is broken, but not MAP_ALIGNMENT_MASK. Whatever > >> address the patch causes to be passed into vm_map_find(), we will round > >> it up as necessary to achieve the requested alignment. > >> > >> In some sense, the real effect of the map alignment directives, > >> including automatic alignment for superpages, is going to be to chop o= ff > >> address bits that Shawn has worked to randomize. More on this below. > >> See [*]. > >> > >>> ... What must > >>> be done is vm_map_find() requesting vm_map_findspace() for the address > >>> hole of the size of the requested mapping + (number of address bits to > >>> randomize << PAGE_SHIFT). Then, the rng value should be obtained and > >>> final location for the mapping calculated as return value + (rng << > >>> PAGE_SHIFT). > >> > >> > >> This seems to be trying to implement a different and more complex sche= me > >> than what Shawn is trying to implement. Specifically, suppose that we > >> have a program that creates 5 mappings with mmap(2). Call them M1, M2, > >> M3, M4, and M5, respectively. Shawn is perfectly happy for M1 to alwa= ys > >> come before M2 in the address space, M2 to always come before M3, and = so > >> on. He's not trying to permute their order in the address space from > >> one execution of the program to the next. He's only trying to change > >> their addresses from one execution to the next. The impression that I > >> got during the BSDCan presentation was that this is what other operati= ng > >> systems were doing, and it was considered strike a reasonable balance > >> between run-time overhead and the security benefits. > >> > >> > >>> > >>> If no address space hole exists which can satisfy the enlarged > >>> request, either a direct fallback to the initial length should be > >>> performed (I prefer this), or exponential decrease of the length up to > >>> the initial size done, and findspace procedure repeated. > >>> > >>> Also, the vm_map_find() knows about the superpages hint for the > >>> mapping being performed, which allows to not break superpages. When > >>> superpage-aligned mapping is requested, SUPERPAGE_SHIFT (available > >>> from pagesizes[1]) should be used instead of PAGE_SHIFT in the formula > >>> above, and probably, a different amount of address bits in the page > >>> table page level 2 to randomize, selected. > >> > >> > >> [*] I agree with this last observation. I mentioned this possibility > >> to Shawn after his talk. On machines where pagesizes[1] is defined, > >> i.e., non-zero, it makes little sense to randomize the low-order addre= ss > >> bits that will be "rounded away" by aligning to pagesizes[1] boundary. > >> > >> > >>> > >>> =3D=3D=3D end of kib@ suggestions =3D=3D=3D > >>> > >>> I have a few concerns, though: > >>> > >>> 1) vm_map_find is also used when loading certain bits of data in the > >>> kernel (like KLDs, for example); > >>> 2) It's not possible to tell in vm_map_find if we're creating a mappi= ng > >>> for the stack, the execbase, or any other suitable mmap call. We apply > >>> ASLR differently based on those three aspects; > >>> 3) vm_map_find is also used in initializing the VM system upon boot. > >>> What impacts would this cause? > >>> > >>> I would really appreciate any feedback. Thank you in advance for any > >>> help or guidance. > >> > >> Shawn, while I'm here, I'll mention that what you were doing with stac= ks > >> in pax_aslr_mmap() seemed odd. Can you explain? > >>=20 > >> > > > > Thank you for your input, Alan. From what it sounds like, you're > > suggesting we don't implement the ASLR functionality in vm_map_find() > > and we stick with what we have. Please correct me if I'm wrong. >=20 >=20 > Yes. I interpreted Kostik's email as implicitly proposing a different > specification for how mmap()-created mappings should be handled than you > started from. In other words, if you're not trying to permute the order > in which mappings appear in the address space from execution to > execution, but only shift the entire group of mappings around a bit, > then you don't need to modify vm_map_find(). You can stick with > changing sys_mmap(). >=20 > That said, I'm inferring how you intended mmap()-created mappings to be > handled from looking at the code and statements that you made during > your presentation. Are my two emails correctly describing what you > intended to do with mmap()-created mappings? >=20 > I do, however, think that you're calling pax_aslr_mmap() at an > inopportune point in sys_mmap(). Hopefully, that was clear in my > previous email. >=20 >=20 > > > > Did our changes break MAP_32BIT or was MAP_32BIT broken all along? If > > our ASLR patch breaks it, what would be the best way to fix it? >=20 >=20 > I suspect but I'm not certain that your patch breaks calls to > mmap(MAP_32BIT). But, if you move the call to pax_aslr_mmap() to the > place that I mentioned before, then pax_alsr_mmap() simply won't be > called when MAP_32BIT or MAP_FIXED are passed to mmap(). >=20 >=20 > > > > I'm' a little unsure of your question regarding stacks in > > pax_aslr_mmap(). I don't believe we're doing anything with stacks in > > that function. We do have the function pax_aslr_stack(), but we're > > working on providing a more robust stack randomization (which isn't in > > the patchset I provided). >=20 > mmap(MAP_STACK) is used to create thread stacks. The following code > didn't make sense to me: >=20 > + if (!(td->td_proc->p_vmspace->vm_map.flags & > MAP_ENTRY_GROWS_DOWN)) > + *addr +=3D td->td_proc->p_vmspace->vm_aslr_delta_= mmap; > + else > + *addr -=3D td->td_proc->p_vmspace->vm_aslr_delta_= mmap; >=20 > MAP_ENTRY_GROWS_DOWN is not a valid vm map flag. Moreover, the > subtraction case doesn't make sense to me. >=20 >=20 >=20 I made the change you suggested, which did indeed fix MAP_32BIT. However, ASLR isn't applied to mappings when MAP_32BIT is specified. I'll cook up a better fix so that we can apply ASLR in this case, too. Thanks for the suggestion. I've removed the MAP_ENTRY_GROWS_DOWN conditional. As you (and another person told me off-list), that conditional is useless. We're working on a better way to handle stack randomization. Thanks for the help. Once we have stack randomization finished, I'll send another CFT patch and have a few specific people look at it. Thanks, Shawn --WhfpMioaduB5tiZL Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJTrujTAAoJEGqEZY9SRW7urO0P/jdlR4gMOL+jheh3B284M6e+ KoVJMnySfuEodUZUAxGvN+rS7m3WfFj3/C1Y5hN1HWYHBcofdlT0Dae8MhHPJeCK W+qtV8T5c4PJ0IEPTzSWFn5U/JQo+IZ9ste03qdSXwgbCM+oCHbaU91EDBZIY8Kx ITaX/3tQ+58Xw22z9wT9KOj/6+p1pS0SeBOd7rXkM2DDA+qR1Y6/XnCKB5TKK2BS qhu8E8K1wn7+mFuIuAWVb3SXeX/Jmyh2zmKBjO+AG18p6olmI85LSxdK13fEczaN x9jTN7vEExWPFo1UJDFW0/zJ25UZg6Oy0TKbv5czEteMmoCx4uICGFwP8a99aIKx 89pdBva0A2tRZJgzcIJU4ujjXbtO27HmSMY0gfsmqkT+OfJAb0sPIPyRbRyD1KfY kbgp8FGbHOIoIMFwVPpiVRWrlYkwFEPSevWEANNyxEyxKXR9RkVk4EkWRPgAPra7 ylIQEdluZOkaL8/AXOqlm6xGS37Slj8ol712risP0xBu69H8KVaP9lW4Ifbr3yDG OAnE9GwLsen7cBnr2mjSO6LngNjssmoZKFRFgw7FML6Hpf8C/Qupr6XDUWwxO++k FeIGWYU/Cypc1x77q27TYl5dwN+a2MGeiRKzz/e9jd5noiBi9Eh2g9bfpx/9Ryra mPEgzHjcEAL9n4Uhy3L1 =HKQ9 -----END PGP SIGNATURE----- --WhfpMioaduB5tiZL--