RFR(xs): (aix but affects shared code too) 8186665: buffer overflow in Java_java_nio_MappedByteBuffer_isLoaded0

[Peter Levart]

Hi Thomas,

Shouldn't the following line:

   47     return len2 + pagesize - 1 / pagesize;

..be written as:

            return (len2 + pagesize - 1) / pagesize;


Regards, Peter

On 10/18/2017 09:44 AM, Thomas Stüfe wrote:
> Hi all,
>
> I am wrapping up fixes which did not make it into the repo before the 
> consolidation. Alan already reviewed the last webrev, but I need a 
> second reviewer.
>
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8186665 
> <https://bugs.openjdk.java.net/browse/JDK-8186665>
> Last Webrev: 
> http://cr.openjdk.java.net/~stuefe/webrevs/8186665-buffer-overflow-in-mincore/webrev.01/webrev 
> <http://cr.openjdk.java.net/%7Estuefe/webrevs/8186665-buffer-overflow-in-mincore/webrev.01/webrev/>
>
> Issue text for your convenience:
>
> --
> In Java_java_nio_MappedByteBuffer_isLoaded0, we call mincore(2) to 
> retrieve the paging status of an address range.
>
> The size of the output buffer for mincore(2) depends on the number of 
> pages in *system page size* in the given memory range (this is spelled 
> out more or less explicitly on AIX and Linux, nothing is said on 
> BSD/OSX, but I assume the same). The number of pages in the memory 
> range is calculated by MappedByteBuffer.isLoaded() and handed down to 
> Java_java_nio_MappedByteBuffer_isLoaded0() together with the memory 
> range to test.
>
> MappedByteBuffer.isLoaded() calculates this number of pages based on 
> jjdk.internal.misc.Unsafe.pagesize(), which ultimately comes down to 
> os::vm_page_size().
>
> For AIX, os::vm_page_size() may return a page size larger than the 
> system page size of 4K. The reason for this is that on AIX, memory can 
> be backed by different page sizes, usually either 4K or 64K - e.g. 
> posix thread stacks may have 4K pages, java heap (system V shared 
> memory) with 64K pages, but mmap memory is always 4K page backed...
>
> But as the OpenJDK code base generally assumes one homogeneous page 
> size for everything - which is usually synonymous with 
> os::vm_page_size() - a decision had to be made which page size to 
> assume as a global system page size, and this may be a larger page 
> size than the 4K system page size mincore(2) assumes.
>
> This usually is no problem, but with mincore(2) it is: as the size of 
> the output buffer depends on the number of pages, calculating with a 
> too-large page size causes the output buffer to be too small and hence 
> the buffer overflows. The solution must be to base the size of the 
> mincore output buffer on the system page size.
>
> --
>
> Thanks and Kind Regards, Thomas
>
>
> On Thu, Aug 31, 2017 at 9:39 PM, Alan Bateman <Alan.Bateman at oracle.com 
> <mailto:Alan.Bateman at oracle.com>> wrote:
>
>     On 31/08/2017 19:01, Thomas Stüfe wrote:
>
>         Hi Alan,
>
>         thank you for your review!
>
>         Updated webrev:
>         http://cr.openjdk.java.net/~stuefe/webrevs/8186665-buffer-overflow-in-mincore/webrev.01/webrev/
>         <http://cr.openjdk.java.net/%7Estuefe/webrevs/8186665-buffer-overflow-in-mincore/webrev.01/webrev/>
>         <http://cr.openjdk.java.net/%7Estuefe/webrevs/8186665-buffer-overflow-in-mincore/webrev.01/webrev/
>         <http://cr.openjdk.java.net/%7Estuefe/webrevs/8186665-buffer-overflow-in-mincore/webrev.01/webrev/>>
>
>         I fixed the indention and embellished the comments around the
>         sentinel at the end of the buffer somewhat.
>
>     This looks okay to me.
>
>     -Alan
>
>