Sergey, Looks good for me.
-Dmitry On 2014-09-12 12:46, Sergey Gabdurakhmanov wrote: > Dmitry, > > New patch: > http://cr.openjdk.java.net/~sgabdura/8057564/webrev.01/ > > > My answers: > > 1. You should not free lpSecurityDescriptor if it's null (ll.291) > > I checked MSDN > http://msdn.microsoft.com/en-us/library/windows/desktop/aa366730%28v=vs.85%29.aspx > "If the /hMem/ parameter is *NULL*, *LocalFree* ignores the parameter > and returns *NULL*." > > 2. It's better to re-arrange code a bit: > > if InitializeSecurityDescriptor or SetSecurityDescriptorDacl fails, > free lpSecurityDescriptor immediately and continue with > lpSecurityDescriptor == NULL > > Done. > > > 3. Make sure it works on all supported platforms: this code rise minimal > server version to windows 2003 server. > > In Windows 2003 server my fix will create a new security attributes. > If SetSecurityDescriptorDacl or InitializeSecurityDescriptor will return > false on Windows XP > then my patch will pass NULL to CreateNamedPipe and the code will use > default security descriptor. > > > BR, > Sergey > > On 11.09.2014 16:16, Dmitry Samersoff wrote: >> Sergey, >> >> 1. You should not free lpSecurityDescriptor if it's null (ll.291) >> >> 2. It's better to re-arrange code a bit: >> >> if InitializeSecurityDescriptor or SetSecurityDescriptorDacl fails, >> free lpSecurityDescriptor immediately and continue with >> lpSecurityDescriptor == NULL >> >> >> 3. Make sure it works on all supported platforms: this code rise minimal >> server version to windows 2003 server. >> >> -Dmitry >> >> >> >> On 2014-09-11 15:49, Sergey Gabdurakhmanov wrote: >>> Hi, >>> >>> Could I please have a review of this small fix. >>> >>> webrev: http://cr.openjdk.java.net/~sgabdura/8057564/webrev.00/ >>> bug: https://jbs.oracle.com/bugs/browse/JDK-8057564 >>> >>> Problem description: >>> On Windows 7 with User Account Control (UAC) enabled, JVM hangs at >>> getAgentProperties or getSystemProperties after attaching from a "high" >>> IntegrityLevel JVM to a "medium" IntegrityLevel JVM, using Attach API: >>> attachedVM = com.sun.tools.attach.VirtualMachine.attach(pid); >>> final Properties systemProperties = attachedVM.getSystemProperties(); >>> >>> Root cause: >>> In WindowsVirtualMachine.attach is implemented with named pipes. >>> If named pipe was created with default security properties then windows >>> will not allow process with"medium" IntegrityLevel to be attached to a >>> processwith "high" IntegrityLevel. >>> >>> Solution: >>> Create security properties that allow requested connection. >>> >>> I'm going to push this fix into JDK9, 8 and 7. >>> BR, >>> Sergey >>> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
