RFR (S): 8012015: Use PROT_NONE when reserving memory

[David Holmes]

Looks okay to me.

David

On 25/04/2013 7:43 AM, Mikael Vidstedt wrote:
>
> New webrev, now with a test based on the whitebox framework:
>
> http://cr.openjdk.java.net/~mikael/webrevs/8012015/webrev.02/webrev
>
> Cheers,
> Mikael
>
> On 4/18/2013 7:52 PM, David Holmes wrote:
>> Hi Mikael,
>>
>> On 19/04/2013 8:58 AM, Mikael Vidstedt wrote:
>>>
>>> Please review the below patch which changes the access rights when
>>> reserving memory on Linux and BSD from using read+write to none, which
>>> matches what's done on Solaris. Full background below.
>>
>> This seems quite reasonable.
>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~mikael/webrevs/8012015/webrev.00/webrev/
>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012015
>>>
>>> Passes JPRT testing, and I also verified manually that CDS (still) works
>>> on my Linux workstation.
>>>
>>> I'm also taking suggestion on how to implement a regression test for
>>> this.
>>
>> Add it to the whitebox testing code perhaps?
>>
>> David
>> -----
>>
>> One alternative would be to parse /proc/self/maps on Linux, find
>>> the corresponding range and verify that the protection flags are
>>> correct, do almost the same thing for OSX but using /proc/PID/task/vmmap
>>> instead etc, but that obvious is a lot of platform dependent scaffolding
>>> for a regression test. An alternative I'm leaning towards would be to
>>> just read and/or write to the page and assert that a SIGSEGV was raised.
>>> Other suggestions?
>>>
>>>
>>> Background (copied from the bug report for your convenience):
>>>
>>> Memory is reserved on the *nix platforms using mmap and passing in the
>>> MAP_NORESERVE. Before the memory can actually used it needs to be
>>> committed, and this is done by calling mmap without the MAP_NORESERVE
>>> flag. The commit call also specifies the requested access/protection
>>> bits for the address range.
>>>
>>> Currently Linux and BSD/OSX the protection used when reserving memory is
>>> PROT_READ|PROT_WRITE. This is done in the anon_mmap in the respective
>>> os_*.cpp files. This means that the memory range is actually readable
>>> and writable, but because the MAP_NORESERVE flag has been specified
>>> there is no guarantee that a read/write will succeed - if the system is
>>> low on memory and out of swap space for example the read/write may raise
>>> a signal.
>>>
>>> This is not normally a problem - before the memory is used it is
>>> typically committed. However, for subtle bugs where wild pointers are
>>> used etc it would be preferable to get a SEGSEGV and catch the bug early
>>> rather than have the use of the wild pointer silently succeed.
>>>
>>> In the Solaris implementation of anon_mmap there is a comment about
>>> exactly this:
>>>
>>>    // Map uncommitted pages PROT_NONE so we fail early if we touch an
>>>    // uncommitted page. Otherwise, the read/write might succeed if we
>>>    // have enough swap space to back the physical page.
>>>
>>>
>>> Also, on both Linux and BSD/OSX the respective pd_uncommit_memory
>>> functions both restore the memory to PROT_NONE, so newly reserved memory
>>> currently gets PROT_READ|PROT_WRITE, but memory which gets uncommitted
>>> gets PROT_NONE which does not appear to be very symmetrical.
>>>
>>> Cheers,
>>> Mikael
>