RFR 9: 8166148 Fix for JDK-8165936 broke Solaris builds

[Thomas Stüfe]

Hi Roger,

Thank you for taking care of this! Fix looks fine.

Kind Regards, Thomas

On Thursday, 15 September 2016, Roger Riggs <Roger.Riggs at oracle.com> wrote:

> Please review a fix for build breakage on Solaris.
> The NAME_MAX value does not seem to add much value in this case and is not
> defined on all supported platforms.
>
> Webrev:
>    http://cr.openjdk.java.net/~rriggs/webrev-max-8166148/
> Issue:
>    https://bugs.openjdk.java.net/browse/JDK-8166148
>
> Thanks, Roger
>
>
> On 9/15/2016 1:39 PM, Roger Riggs wrote:
>
>> Hi Thomas,
>>
>> It looks like NAME_MAX is not defined on Solaris and breaks the build on
>> Solaris. [1]
>>
>> An  alternative would be to conditionally use PATH_MAX.
>>
>> But I think it would be reasonable to just remove the use of NAME_MAX,
>> since the following
>> code increases the value to at least 1024, which should be safe.
>>
>> If you don't have time to address this, I can propose a fix.
>>
>> Roger
>>
>> [1] 8166148  fix for JDK-8165936 broke solaris builds <
>> https://bugs.openjdk.java.net/browse/JDK-8166148>
>>
>>
>> On 9/14/2016 2:34 AM, Thomas Stüfe wrote:
>>
>>> Hi all,
>>>
>>> thanks for the reviews. Here is version two:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8165936-Potential
>>> -Heap-buffer-overflow-when-seaching-timezone-info-files/
>>> webrev.01/webrev/ <http://cr.openjdk.java.net/%7
>>> Estuefe/webrevs/8165936-Potential-Heap-buffer-overflow-when-
>>> seaching-timezone-info-files/webrev.01/webrev/>
>>>
>>> Only cosmetic changes:
>>> - made code pre-c99 compatible
>>> - consistently use dirent64
>>> - fix indentation in ifs
>>> - removed black between malloc and cast
>>>
>>> Kind Regards, Thomas
>>>
>>>
>>>
>>> On Tue, Sep 13, 2016 at 5:25 PM, Masayoshi Okutsu <
>>> masayoshi.okutsu at oracle.com <mailto:masayoshi.okutsu at oracle.com>> wrote:
>>>
>>>     Looks good to me. Thank you for fixing this bug!
>>>
>>>     Masayoshi
>>>
>>>
>>>
>>>     On 9/13/2016 11:49 PM, Thomas Stüfe wrote:
>>>
>>>         Hi Christoph, thanks for your review! Yes, I can remove the
>>> blank.
>>>
>>>         Kind Regards, Thomas
>>>
>>>         On Tue, Sep 13, 2016 at 2:35 PM, Langer, Christoph
>>>         <christoph.langer at sap.com <mailto:christoph.langer at sap.com>
>>>
>>>             wrote:
>>>             Hi Thomas,
>>>
>>>             your change looks good. I'm also forwarding this to
>>>             i18n-dev as issues in
>>>             TimeZone implementation are mostly handled there.
>>>
>>>             One remark: Can you take the opportunity to also remove
>>>             the blank between
>>>             the cast and malloc in line 150: "(struct dirent64 *)
>>>             malloc..."?
>>>
>>>             Unfortunately I'm no reviewer, so you still need an
>>>             official review.
>>>
>>>             Best regards
>>>             Christoph
>>>
>>>                 -----Original Message-----
>>>                 From: core-libs-dev
>>>                 [mailto:core-libs-dev-bounces at openjdk.java.net
>>> <mailto:core-libs-dev-bounces at openjdk.java.net>] On
>>>
>>>             Behalf
>>>
>>>                 Of Thomas Stüfe
>>>                 Sent: Dienstag, 13. September 2016 12:54
>>>                 To: Java Core Libs <core-libs-dev at openjdk.java.net
>>> <mailto:core-libs-dev at openjdk.java.net>>
>>>                 Subject: RFR(xs): 8165936: Potential Heap buffer
>>>                 overflow when seaching
>>>                 timezone info files
>>>
>>>                 Dear all,
>>>
>>>                 please take a look at this small change:
>>>
>>>                 Bug: https://bugs.openjdk.java.net/browse/JDK-8165936
>>> <https://bugs.openjdk.java.net/browse/JDK-8165936>
>>>                 Webrev:
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8165936-
>>> <http://cr.openjdk.java.net/%7Estuefe/webrevs/8165936->
>>>
>>>             Potential-Heap-buffer-
>>>
>>> overflow-when-seaching-timezone-info-files/webrev.00/webrev/
>>>
>>>                 readdir_r is used to iterate over the content of a
>>>                 system directory, but
>>>                 the buffer passed to it is too small: Its size should
>>>                 include the size of
>>>                 the dirent structure itself (minus the d_name member).
>>>
>>>                 The fix also now checks the return code of pathconf(),
>>>                 and if pathconf()
>>>                 returns an error, falls back to the NAME_MAX compile
>>>                 time constant.
>>>                 Finally, it imposes a minimum size for the buffer,
>>>                 because on older
>>>
>>>             System
>>>
>>>                 V systems NAME_MAX may be surprisingly small and
>>>                 readdir_r will not check
>>>                 the output buffer size. I think it is better to err on
>>>                 the safe side
>>>
>>>             here.
>>>
>>>                 Kind Regards, Thomas
>>>
>>>
>>>
>>>
>>
>