[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Code Review 7110017: is_headless_jre should be updated to reflect the new,location of awt toolkit li
From:       Dmitry.Samersoff () oracle ! com (Dmitry Samersoff)
Date:       2011-11-16 9:07:27
Message-ID: 4EC37D4F.3040806 () oracle ! com
[Download RAW message or body]

David,

OK. Looks fine for me.

-Dmitry


On 2011-11-16 04:01, David Holmes wrote:
> Hi Dmitry,
> 
> On 15/11/2011 10:17 PM, Dmitry Samersoff wrote:
>> 1.  What is the goal of renaming xawtstr to new_xawtstr?
> 
> It isn't renamed. We added new_xawtstr to refer to the new library name,
> while keeping xawtstr to refer to the old library name - this code has
> to work for JDK 8 and JDK 7 and the library renaming is only in 8. We
> removed the (as of JDK7) unused motifstr.
> 
>> 2.  (Not to your code but as far as you touch it) "libawt_xawt" is
>> longer than "libjvm" but we are copying it to static buffer without a
>> boundary check - it can cause buffer overflow in some marginal case and
>> clearly exploitable.
>>
>> I would prefer to add extra 5 bytes to libmawtpath
>> ( sizeof("libawt_xawt.so") - sizeof("libjvm.so") )
>>
>>   char libmawtpath[MAXPATHLEN + 5];
> 
> Note that we are replacing either
> 
> client/libjvm.so  (16 chars)
> 
> or
> 
> server/libjvm.so (16 chars)
> 
> with either:
> 
> xawt/libmawt.so (15 chars)
> 
> or
> 
> libawt_xawt.so  (14 chars)
> 
> so there is no danger of overflow.
> 
>> 3. I don't see Windows changes - is it OK ?
> 
> Yes. is_headless_jre() is always false on Windows as there is no
> headless JRE there.
> 
> Thanks,
> David
> -----
> 
>>
>>
>>
>> On 2011-11-15 06:05, David Holmes wrote:
>>> Pinging runtime! One more Reviewer please.
>>>
>>> Thanks,
>>> David
>>>
>>> On 11/11/2011 9:11 PM, David Holmes wrote:
>>>> Chris,
>>>>
>>>> Thanks for contributing the changes. I've applied them and tested them
>>>> so it is a Thumbs Up from me.
>>>>
>>>> We need one further Reviewer from runtime.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 11/11/2011 9:12 PM, Chris Hegarty wrote:
>>>>>
>>>>> With the changes proposed by CR 7110002: "Rename xawt/libmawt.so and
>>>>> headless/libmawt.so so they can be colocated with libawt",
>>>>> os::is_headless_jre() will have to be updated to look for the renamed
>>>>> awt toolkit libraries. See the discussion on awt-dev for further
>>>>> information:
>>>>> http://mail.openjdk.java.net/pipermail/awt-dev/2011-November/002026.html
>>>>>
>>>>>
>>>>> The check for the motif toolkit can also be removed as the motif based
>>>>> toolkit has been removed in JDK7.
>>>>>
>>>>> Since is_headless_jre is checking is for headless and short circuits
>>>>> and returns false ( headed ) if it finds any headed library we can
>>>>> just check for the existence of either the new or old library, without
>>>>> JDK version specific code. If either exists then it is a headed
>>>>> environment.
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~dholmes/7110017/webrev/
>>>>>
>>>>> -Chris.
>>>>>
>>>>> P.S. Thanks to David Holmes for helping drive this on the vm side.
>>
>>


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...

[prev in list] [next in list] [prev in thread] [next in thread]