Hello, Yes the frames must have the security granted, isnt that the whole purpose of a security permission check?
With the current state of the patch unprivileged application code can read the PID. If this is intentional I would simply remove the access check completely instead. Gruss Bernd -- http://bernd.eckenfels.net ________________________________ From: serviceability-dev <serviceability-dev-boun...@openjdk.java.net> on behalf of mandy chung <mandy.ch...@oracle.com> Sent: Tuesday, October 24, 2017 7:20:27 PM To: serviceability-dev@openjdk.java.net Subject: Re: RFR : JDK-8044122 MBean access to the PID The permission check should be done separately, if needed. Otherwise, the frames in the stack when this method is called must have the security permission granted. Mandy On 10/23/17 1:00 PM, Bernd Eckenfels wrote: Hello, When running this privileged it means one can bypass the permission by using the MBean, is that intentional? (Besides it is already available as the JMVID) Gruss Bernd -- http://bernd.eckenfels.net ________________________________ From: serviceability-dev <serviceability-dev-boun...@openjdk.java.net><mailto:serviceability-dev-boun...@openjdk.java.net> on behalf of mandy chung <mandy.ch...@oracle.com><mailto:mandy.ch...@oracle.com> Sent: Friday, October 20, 2017 4:42:00 PM To: Ujwal Vangapally; Roger Riggs Cc: serviceability-dev Subject: Re: RFR : JDK-8044122 MBean access to the PID Process::pid may throw SecurityException. You have to wrap the call with doPrivileged. Process::pid can throw UOE on platform that doesn't support this operation. RuntimeMXBean::getPid should specify when the platform does not support this operation. ProcessIdTest - can you also check if getName contains the pid matching the value of getPid()? Mandy On 10/20/17 2:07 AM, Ujwal Vangapally wrote: > kindly see the new webrev incorporating review comments. > > webrev: > http://cr.openjdk.java.net/~uvangapally/webrev/2017/8044122/webrev.01/<http://cr.openjdk.java.net/%7Euvangapally/webrev/2017/8044122/webrev.01/> > > Thanks, > > Ujwal. > > > On 10/11/2017 3:50 PM, Ujwal Vangapally wrote: >> Thanks for the review and suggestions Mandy, Roger. >> >> kindly see my comments inline. >> >> >> On 10/10/2017 11:25 PM, Roger Riggs wrote: >>> Hi Ujwal, >>> >>> In the implementation RuntimeMXBean.java: 72: Include a message >>> "getProcessId" in the throw new Unsupported... >>> In the text and @return change "PID" to "process ID" as Alan suggested. >>> 66: the @implSpec should be on its own line so the text starts on a >>> new line to make the source more readable. >>> >>> Adding a test for getProcessId() should fit into one of the existing >>> tests that spawns and then checks >>> the attributes of a vm. Perhaps MXBeanInteropTest1.java >> I will make changes as suggested. >>> >>> Roger >>> >>> >>> >>> On 10/10/2017 1:20 PM, mandy chung wrote: >>>> >>>> >>>> On 10/10/17 4:47 AM, Ujwal Vangapally wrote: >>>>> Kindly review the changes made. >>>>> >>>>> https://bugs.openjdk.java.net/browse/JDK-8044122 >>>>> >>>>> webrev : >>>>> http://cr.openjdk.java.net/~uvangapally/webrev/2017/8044122/webrev.00/<http://cr.openjdk.java.net/%7Euvangapally/webrev/2017/8044122/webrev.00/> >>>>> >>>>> >>>> >>>> RuntimeMXBean.java >>>> @since is missing >>>> >> I will add it. >>>> Process::pid is long rather than int. The javadoc for this >>>> method should be consistent with Process::pid, as Alan points out. >> will do it. >>>> >>>> VMManagementImpl.java >>>> I think getProcessId should probably be replaced to implement >>>> with ProcessHandle.current().pid(); >>>> >> you mean it would be better to use ProcessHandle.current().pid(); in >> RuntimeImpl.java instead of jvm.getVmPid(); >> >> kindly clarify. >>>> Please include an unit test for it. >>>> >> will it be sufficient to add it to existing test MXBeanInteropTest1.java >> >> System.out.println("getName\t\t" >> + runtime.getName()); >> + System.out.println("getPid\t\t" >> + + runtime.getPid()); >> System.out.println("getSpecName\t\t" >> + runtime.getSpecName()); >> >>>> Mandy >>> >> >> Ujwal >
