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/%7Estuefe/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.oku...@oracle.com <mailto:masayoshi.oku...@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.lan...@sap.com <mailto:christoph.lan...@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-boun...@openjdk.java.net
<mailto:core-libs-dev-boun...@openjdk.java.net>] On
Behalf
Of Thomas Stüfe
Sent: Dienstag, 13. September 2016 12:54
To: Java Core Libs <core-libs-...@openjdk.java.net
<mailto:core-libs-...@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
