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

Peter Levart peter.levart at gmail.com
Wed Oct 18 08:17:29 UTC 2017



On 10/18/2017 10:14 AM, Peter Levart wrote:
> Hi Thomas,
>
> Shouldn't the following line:
>
>   47     return len2 + pagesize - 1 / pagesize;
>
> ..be written as:
>
>            return (len2 + pagesize - 1) / pagesize;

...or better yet, as:

                return numPages;


(you already calculate it correctly in previous line, but then return an 
expression, which is wrong).

Regards, Peter

>
>
> 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
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20171018/318562c4/attachment-0001.html>


More information about the nio-dev mailing list