[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: (XS): JDK-8068004: [Findbugs]sun.jvm.hotspot.debugger may expose internal representation
From: Sundararajan Athijegannathan <sundararajan.athijegannathan () oracle ! com>
Date: 2016-07-29 16:28:11
Message-ID: 1c3ecf29-aaf0-43f4-a83b-baacc803f643 () oracle ! com
[Download RAW message or body]
Agreed that it could be considered as a defense-in-depth fix. But, in
this case Page data could be huge. I think it was not cloned in first
place to avoid copying many big byte[] instances around.
-Sundar
On 7/29/2016 9:36 PM, Daniel D. Daugherty wrote:
> Two points:
>
> 1) if Findbugs reports the same issue on JDK9 code, then we want to
> address such that we reduce any Findbugs noise
>
> 2) Fixing it could be considered to be a defense-in-depth change.
>
> Dan
>
>
> On 7/29/16 7:19 AM, Sundararajan Athijegannathan wrote:
>> Well, we can't code for that kind of overrides - Findbugs or any such
>> tool is about normal mode of execution. With that argument, people
>> can override classes using -Xpatch option as well!
>>
>> -Sundar
>>
>> On 7/29/2016 6:05 PM, Jini Susan George wrote:
>>>
>>> Thank you, JB and Sundar. Sundar, would that hold true even if
>>> –XaddExports is used ?
>>>
>>>
>>>
>>> Regards,
>>>
>>> Jini.
>>>
>>>
>>>
>>> *From:*Sundararajan Athijegannathan
>>> *Sent:* Friday, July 29, 2016 5:11 PM
>>> *To:* serviceability-dev@openjdk.java.net
>>> *Subject:* Re: RFR: (XS): JDK-8068004:
>>> [Findbugs]sun.jvm.hotspot.debugger may expose internal representation
>>>
>>>
>>>
>>> If cloning is done to avoid exposing byte[] outside SA, this fix is
>>> not needed in jdk9. In jdk9, none of the SA packages are exposed and
>>> code outside SA cannot access this. Besides, Page data may be very
>>> big - cloning that ever constructor and getter may be too costly.
>>>
>>> -Sundar
>>>
>>>
>>>
>>> On 7/29/2016 5:07 PM, Jaroslav Bachorik wrote:
>>>
>>> Hi Jini,
>>>
>>>
>>>
>>> 'null' seems to be a valid value for 'data' field in both of the
>>> places you are using 'data.clone()' - you should guard for null
>>> and call 'clone()' only if the passed in value is non-null.
>>>
>>>
>>>
>>> Cheers,
>>>
>>>
>>>
>>> -JB-
>>>
>>>
>>>
>>> On Fri, Jul 29, 2016 at 11:29 AM, Jini Susan George
>>> <jini.george@oracle.com <mailto:jini.george@oracle.com>> wrote:
>>>
>>> Hi all,
>>>
>>> Please review the fix for the following SA defect (to avoid
>>> exposing internal representations by storing or returning
>>> externally mutable objects directly).
>>>
>>> Bug ID: https://bugs.openjdk.java.net/browse/JDK-8068004
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sballal/sponsorship/8068004/webrev.00/
>>> <http://cr.openjdk.java.net/%7Esballal/sponsorship/8068004/webrev.00/>
>>>
>>> Thanks,
>>>
>>> - Jini Susan George
>>>
>>>
>>>
>>>
>>>
>>
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p>Agreed that it could be considered as a defense-in-depth fix.
But, in this case Page data could be huge. I think it was not
cloned in first place to avoid copying many big byte[] instances
around. <br>
</p>
-Sundar
<div class="moz-cite-prefix"><br>
On 7/29/2016 9:36 PM, Daniel D. Daugherty wrote:<br>
</div>
<blockquote
cite="mid:4e26134e-bba9-a153-8f71-7871ce21c746@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<tt>Two points:<br>
<br>
1) if Findbugs reports the same issue on JDK9 code, then we want
to<br>
address such that we reduce any Findbugs noise<br>
<br>
2) Fixing it could be considered to be a defense-in-depth
change.<br>
<br>
Dan<br>
<br>
</tt><br>
<div class="moz-cite-prefix">On 7/29/16 7:19 AM, Sundararajan
Athijegannathan wrote:<br>
</div>
<blockquote
cite="mid:9acdebc0-2056-cacd-bf8d-f502ffa35761@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Well, we can't code for that kind of overrides - Findbugs or any
such tool is about normal mode of execution. With that argument,
people can override classes using -Xpatch option as well!<br>
<br>
-Sundar<br>
<br>
<div class="moz-cite-prefix">On 7/29/2016 6:05 PM, Jini Susan
George wrote:<br>
</div>
<blockquote
cite="mid:94cb73fd-d8c0-4521-8a93-4f5a3e60b140@default"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<meta name="Generator" content="Microsoft Word 12 (filtered
medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";
color:black;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p
{mso-style-priority:99;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:12.0pt;
font-family:"Times New Roman","serif";
color:black;}
p.MsoNoSpacing, li.MsoNoSpacing, div.MsoNoSpacing
{mso-style-priority:1;
margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";
color:black;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
<div class="WordSection1">
<p class="MsoNoSpacing">Thank you, JB and Sundar. Sundar,
would that hold true even if –XaddExports is used ? <o:p></o:p></p>
<p class="MsoNoSpacing"><o:p> </o:p></p>
<p class="MsoNoSpacing">Regards,<o:p></o:p></p>
<p class="MsoNoSpacing">Jini.<o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> \
</o:p></span></p> <div style="border:none;border-left:solid blue
1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF
1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span
style="font-size:10.0pt;font-family:"Tahoma","sans-serif";color:windowtext">From:</span></b><span
style="font-size:10.0pt;font-family:"Tahoma","sans-serif";color:windowtext">
Sundararajan Athijegannathan <br>
<b>Sent:</b> Friday, July 29, 2016 5:11 PM<br>
<b>To:</b> <a moz-do-not-send="true"
class="moz-txt-link-abbreviated"
\
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a><br>
<b>Subject:</b> Re: RFR: (XS): JDK-8068004:
[Findbugs]sun.jvm.hotspot.debugger may expose
internal representation<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p>If cloning is done to avoid exposing byte[] outside SA,
this fix is not needed in jdk9. In jdk9, none of the SA
packages are exposed and code outside SA cannot access
this. Besides, Page data may be very big - cloning that
ever constructor and getter may be too costly.<o:p></o:p></p>
<p>-Sundar<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On 7/29/2016 5:07 PM, Jaroslav
Bachorik wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">Hi Jini, <o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">'null' seems to be a valid
value for 'data' field in both of the places you
are using 'data.clone()' - you should guard for
null and call 'clone()' only if the passed in
value is non-null.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Cheers,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">-JB-<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Fri, Jul 29, 2016 at 11:29
AM, Jini Susan George <<a
moz-do-not-send="true"
href="mailto:jini.george@oracle.com"
target="_blank">jini.george@oracle.com</a>>
wrote:<o:p></o:p></p>
<div>
<p><span
\
style="font-family:"Calibri","sans-serif"">Hi all, \
</span><o:p></o:p></p> <p><span
\
style="font-family:"Calibri","sans-serif"">Please review the fix \
for the following</span> <span \
style="font-family:"Calibri","sans-serif"">SA</span> <span
\
style="font-family:"Calibri","sans-serif"">defect (to avoid \
exposing internal representations by storing or returning externally mutable
objects directly).</span><o:p></o:p></p>
<p><span
\
style="font-family:"Calibri","sans-serif"">Bug ID:</span> <a \
moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8068004"
target="_blank"><span
\
style="font-family:"Calibri","sans-serif"">https://bugs.openjdk.java.net/browse/JDK-8068004</span></a><o:p></o:p></p>
<p><span
\
style="font-family:"Calibri","sans-serif"">Webrev:</span> <a \
moz-do-not-send="true" \
href="http://cr.openjdk.java.net/%7Esballal/sponsorship/8068004/webrev.00/" \
target="_blank"><span \
style="font-family:"Calibri","sans-serif"">http://cr.openjdk.java.net/~sballal/sponsorship/8068004/webrev.00/</span></a><o:p></o:p></p>
<p><span
\
style="font-family:"Calibri","sans-serif"">Thanks,</span><o:p></o:p></p>
<p><span
\
style="font-family:"Calibri","sans-serif"">- Jini Susan \
George</span><o:p></o:p></p> </div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic