Hi Roger, Thank you for taking care of this! Fix looks fine.
Kind Regards, Thomas On Thursday, 15 September 2016, Roger Riggs <roger.ri...@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.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 >>> >>> >>> >>> >> >
