Remove Test8050807.sh?

Tue Feb 27 20:54:12 UTC 2018

Yes, I meant a normal RFR to hotspot rt confidential.

I haven't asked any of the security folks about it.  I'm not sure who 
else would review it other than possibly David Holmes.   Or Karen, if I 
asked her to.

Harold

On 2/27/2018 3:50 PM, Daniel D. Daugherty wrote:
> On 2/27/18 3:22 PM, harold seigel wrote:
>>
>> Thanks Dan and Jerry!   This is most helpful.
>>
>> I tried "/manual" but that did not prevent the test from running when 
>> doing "jtreg ... hotspot/runtime".  I also didn't see how "/manual" 
>> would protect against other Java processes for the same user that 
>> were run outside of the test's JTReg run.
>>
>> I think the test should PASS for the (hopefully unusual) process 
>> interference case.  Then, no-one will need to look at the result.  
>> That isn't great but better than the current case where the test is 
>> excluded and not run at all.
>>
>> Should I go out for public review with test2 deleted and using Dan's 
>> code for test1 ?
>>
>
> By "public" you don't mean OpenJDK right? This is a closed security
> test so this one has to be internal...
>
> I would be okay with a wider internal review. Have you asked any
> of the security devs what should be done with a test that is so
> very picky?
>
> Dan
>
>
>> Thanks, Harold
>>
>>
>> On 2/27/2018 3:15 PM, Gerald Thornbrugh wrote:
>>> Hi Dan,
>>>
>>>> On Feb 27, 2018, at 12:32 PM, Daniel D. Daugherty 
>>>> <daniel.daugherty at oracle.com <mailto:daniel.daugherty at oracle.com>> 
>>>> wrote:
>>>>
>>>> I caught up with the review thread for 8067033 so now I see why that
>>>> second block is removed. I'm not sure about what 8067033 is claiming.
>>>> The second test case doesn't create a non-hardlinked regular file in
>>>> the target directory because the VM is allowed to remove that kind
>>>> of file. I didn't think it was allowed to remove a hardlinked file
>>>> nor a symlinked file. However, it has been a long time since I've
>>>> thought about the original fix.
>>>>
>>>> Jerry should probably chime in here about the removal of the second
>>>> test case.
>>> The second test case is ok to remove because just before putback I 
>>> change the code to remove
>>> symlinks and hardlinks because it was ok to do so.
>>>>
>>>> As for replacing:
>>>>
>>>>     rmdir ${HSPERFUSERPATH}
>>>>
>>>> with:
>>>>
>>>>     rm -rf ${HSPERFUSERPATH}
>>>>
>>>> If you do that, then this test can interfere with another running
>>>> VM that happens to have a file in ${HSPERFUSERPATH}. The other VM
>>>> can potentially crash when its JVM stat files are removed out from
>>>> under it.
>>> This is correct, I specifically used “rmdir” because of interaction 
>>> with another test.
>>>>
>>>> Basically, you want something like this:
>>>>
>>>>     rmdir ${HSPERFUSERPATH}
>>>>     ln -s ${DIRTARGETPATH} ${HSPERFUSERPATH}
>>>>     if [ ! -h ${HSPERFUSERPATH} ]; then
>>>>         # Was not able to create HSPERFUSERPATH as a symlink
>>>>         # so we cannot execute this part of the test.
>>>>         echo "Warning: Existing ${HSPERFUSERPATH} is interfering 
>>>> with this test."
>>>>         echo "This test is PASSING spuriously."
>>>>         # Cleanup what 'ln -s' created:
>>>>         rm ${HSPERFUSERPATH}/${DIRTARGETPATH}
>>>>         # Fall thru to rest of the cleanup code.
>>>>     else
>>>>         # The "test 1" stuff goes here.
>>>>     fi
>>>>
>>>> Here's what you get out of this version:
>>>>
>>>> - The "rmdir" line will only remove an empty directory so this test
>>>>   won't mess with another running VM.
>>>> - The symbolic link creation will either win the race with a VM
>>>>   trying to create the ${HSPERFUSERPATH} directory or it will
>>>>   lose the race.
>>>> - If ${HSPERFUSERPATH} is a symlink, then the "test 1" logic can
>>>>   execute.
>>>>
>>>>
>>>> Maybe a spurious PASS is not what we want. Maybe we want an
>>>> ERROR result instead of a spurious PASS or a FAIL. Dunno.
>>> I am not sure either.  This test is difficult because of the need to 
>>> run it in parallel with other tests.
>>>
>>> Did you ever determine if the “manual” option would do what we want?
>>>
>>> Thanks!
>>>
>>> Jerry
>>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>>
>>>> On 2/27/18 1:56 PM, Daniel D. Daugherty wrote:
>>>>> Why is the second test scenario removed from the test?
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>> On 2/27/18 1:32 PM, harold seigel wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I looked into the possibility of detecting when the test was 
>>>>>> going to fail because of interference from another Java process, 
>>>>>> and, in that case, then ignoring the result.  I came up with the 
>>>>>> following:
>>>>>>
>>>>>>     http://javaweb.us.oracle.com/~hseigel/webrev/bug_8067033.Feb27/webrev/index.html
>>>>>>
>>>>>> I think the test's "other Java process failure" only occurs if 
>>>>>> another process creates the "/tmp/hsperfdata_<user>" after the 
>>>>>> test deletes it but before the test re-creates it as a symbolic 
>>>>>> link at line 89.
>>>>>>
>>>>>> So, I added the "if [ -h ${HSPERFUSERPATH} ]" check at line 112 
>>>>>> in order to ignore the cases where another process created 
>>>>>> "/tmp/hsperfdata_<user>" as a regular directory before the test 
>>>>>> could create the symbolic link.
>>>>>>
>>>>>> I tested this by temporarily adding "mkdir ${HSPERFUSERPATH}" 
>>>>>> just before line 89.
>>>>>>
>>>>>> Does this seem okay?
>>>>>>
>>>>>> Thanks, Harold
>>>>>>
>>>>>> On 2/23/2018 3:52 PM, Gerald Thornbrugh wrote:
>>>>>>> Hi Coleen,
>>>>>>>
>>>>>>>>
>>>>>>>> If we run it and it fails when we run with jtreg on the command 
>>>>>>>> line or through make, and it's not on the ProblemList.txt, it 
>>>>>>>> is going to cost everybody time to figure out the failure isn't 
>>>>>>>> theirs.
>>>>>>>>
>>>>>>>> One of the uses of these tests is for POC verification.  If we 
>>>>>>>> can put the test in the original BugDB bug, or it's already 
>>>>>>>> there, that might meet the requirements of having this test.
>>>>>>>>
>>>>>>>> I wonder why this is a security bug at all.
>>>>>>> It allowed a non privileged user to overwrite and remove the 
>>>>>>> files associated with the
>>>>>>> owner of the /tmp/hsperfdata_<user> directory. If that user was 
>>>>>>> "root", system files
>>>>>>> could be overwritten or removed.
>>>>>>>
>>>>>>> Jerry
>>>>>>>>
>>>>>>>> Coleen
>>>>>>>>
>>>>>>>> On 2/23/18 3:33 PM, harold seigel wrote:
>>>>>>>>> Thanks Dan.
>>>>>>>>>
>>>>>>>>> I'll try it and see what happens.
>>>>>>>>>
>>>>>>>>> Harold
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2/23/2018 3:32 PM, Daniel D. Daugherty wrote:
>>>>>>>>>> I don't think that '/manual' tests are executed when we 
>>>>>>>>>> specify these
>>>>>>>>>> "test groups". I could very well be wrong...
>>>>>>>>>>
>>>>>>>>>> When I said this:
>>>>>>>>>>
>>>>>>>>>>> I believe if you just run jtreg on a target directory, it runs
>>>>>>>>>>> everything in that hierarchy including the manual tests. 
>>>>>>>>>>
>>>>>>>>>> what I meant was you manually ran "jtreg 
>>>>>>>>>> closed/test/hotspot/jtreg/compiler/c2"
>>>>>>>>>> and it proceeds to run _everything_ in the "c2" hierarchy 
>>>>>>>>>> including the manual
>>>>>>>>>> tests because you ran it from the cmd line, i.e., have a 
>>>>>>>>>> controlling tty.
>>>>>>>>>>
>>>>>>>>>> It dawns on me that jtreg may not be that smart (detecting 
>>>>>>>>>> controlling tty).
>>>>>>>>>>
>>>>>>>>>> I don't know for sure. Maybe /manual tests are only run when 
>>>>>>>>>> asked for...
>>>>>>>>>>
>>>>>>>>>> Dan
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2/23/18 3:26 PM, harold seigel wrote:
>>>>>>>>>>> So, /manual won't stop this test from intermittently failing 
>>>>>>>>>>> during mach5 and other test runs?
>>>>>>>>>>>
>>>>>>>>>>> Harold
>>>>>>>>>>>
>>>>>>>>>>> On 2/23/2018 3:24 PM, Daniel D. Daugherty wrote:
>>>>>>>>>>>> I believe if you just run jtreg on a target directory, it runs
>>>>>>>>>>>> everything in that hierarchy including the manual tests.
>>>>>>>>>>>>
>>>>>>>>>>>> However, I suspect that what you really want to know is 
>>>>>>>>>>>> whether
>>>>>>>>>>>> SQE/SQA has a specific phase of their testing where someone 
>>>>>>>>>>>> sits
>>>>>>>>>>>> down and runs jtreg with "-k manual" and checks the results...
>>>>>>>>>>>> That would be a question for Misha...
>>>>>>>>>>>>
>>>>>>>>>>>> Dan
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 2/23/18 2:31 PM, harold seigel wrote:
>>>>>>>>>>>>> That sounds good, but do the /manual tests ever get run?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Harold
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2/23/2018 2:30 PM, Gerald Thornbrugh wrote:
>>>>>>>>>>>>>> This sounds like a good idea, I vote for Dan's approach.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 02/23/2018 12:20 PM, Daniel D. Daugherty wrote:
>>>>>>>>>>>>>>> Removing a test for a security vulnerability is 
>>>>>>>>>>>>>>> problematic.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Perhaps modify the test to include "/manual" so that it 
>>>>>>>>>>>>>>> is only
>>>>>>>>>>>>>>> run when manual tests are run. Something like this:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>     @run shell/manual Test8050807.sh
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I see other tests do this:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> closed/test/hotspot/jtreg/compiler/c2/4523683/TestCase.java: 
>>>>>>>>>>>>>>> * @run shell/manual Test4523683.sh
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Dan
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 2/23/18 10:42 AM, Gerald Thornbrugh wrote:
>>>>>>>>>>>>>>>> The fix was for a security vulnerability. My fear is 
>>>>>>>>>>>>>>>> that someone may ask "Are we still vulnerable
>>>>>>>>>>>>>>>> to this security issue?". How do we go about proving 
>>>>>>>>>>>>>>>> that unless we still have a test available?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I say definitely remove it.  The trouble with putting 
>>>>>>>>>>>>>>>>> it in a place for intermittent failures is that we'll 
>>>>>>>>>>>>>>>>> never look at it, so it's as good as removed.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I think this was a transient problem and not likely to 
>>>>>>>>>>>>>>>>> reoccur.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Coleen
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 2/23/18 10:03 AM, Gerald Thornbrugh wrote:
>>>>>>>>>>>>>>>>>> Hi Harold,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Yes, the test is very problematic. The main reason 
>>>>>>>>>>>>>>>>>> the test was putback was
>>>>>>>>>>>>>>>>>> to meet the requirement of adding a test with each 
>>>>>>>>>>>>>>>>>> change.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I have no problem removing the test.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Is there a place for standalone tests that are 
>>>>>>>>>>>>>>>>>> problematic when they are run in
>>>>>>>>>>>>>>>>>> parallel? If there is a home for wayward test it 
>>>>>>>>>>>>>>>>>> would be nice to place it there.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Jerry
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Any objection if I permanently remove test 
>>>>>>>>>>>>>>>>>>> closed/test/hotspot/jtreg/runtime/8050807/Test8050807.sh 
>>>>>>>>>>>>>>>>>>> ?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> The test writes files to the 
>>>>>>>>>>>>>>>>>>> "/tmp/hsperfdata_<user>/" and checks if the perf 
>>>>>>>>>>>>>>>>>>> memory cleanup code removes files correctly.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> The problem with the test is that it requires 
>>>>>>>>>>>>>>>>>>> exclusive access to directory 
>>>>>>>>>>>>>>>>>>> "/tmp/hsperfdata_<user>/". So, even if JTReg ran the 
>>>>>>>>>>>>>>>>>>> test stand-alone, the test could still fail if the 
>>>>>>>>>>>>>>>>>>> same user was running a separate Java invocation on 
>>>>>>>>>>>>>>>>>>> the same system, such as a Mach5 task, another JTReg 
>>>>>>>>>>>>>>>>>>> stream, etc..
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> The name "/tmp/hsperfdata_<user>/" is hard-coded in 
>>>>>>>>>>>>>>>>>>> the JVM.  So, changing the test to use a different 
>>>>>>>>>>>>>>>>>>> directory is not an option.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Currently, the test is on the Problem List. (See 
>>>>>>>>>>>>>>>>>>> closed/test/hotspot/jtreg/ProblemList.txt.)
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Thanks, Harold
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>