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