On Tue, 2 Dec 2025 15:46:10 GMT, Sergey Chernyshev <[email protected]> wrote:
>> Hi all, >> >> I would like to propose a fix for JDK-8319589. This will allow jcmd and jps >> running as root to get the complete list of JVMs running by all users, and >> to attach from root to non-root JVMs. Previously, JDK-8197387 introduced the >> same possibility on Linux. >> >> This change affects macOS, that uses "secure" per-user temporary >> directories. It only affects JVMs running as root, the behavior in >> non-privileged JVMs remains unchanged. >> >> Jcmd and jps rely on LocalVmManager to get the initial list of the local >> VMs. The LocalVmManager uses sun.jvmstat.PlatformSupport to get the list of >> temp directories, where it searches for user's PerfData directory such as >> "hsperfdata_<username\>". In macosx the temp directories are per-user, the >> temp path is returned by confstr(_CS_DARWIN_USER_TEMP_DIR). The per-user >> directories are mode 700 and so they are read-protected from non-privileged >> users and can be accessed by the owner and the root. >> >> Both jps and jcmd (HotSpotAttachProvider) create MonitoredVm objects, that >> have PerfDataBuffer that performs attachment to the target. Only the >> attachable VMs are listed in jcmd output. >> >> The proposed patch changes the list of directories returned by the >> PlatformSupport#getTemporaryDirectories() in VMs running as root. The list >> is later used in VirtualMachineImpl (jdk.attach). It changes also the way >> mmap_attach_shared() searches for hsperfdata_<username\>/<pid\> files to map >> the shared memory. Mmap_attach_shared() and VirtualMachineImpl (via >> PlatformSupport) list the content of /var/folders, where the temp >> directories are located, more specificly the temp directories are >> /var/folders/<BUCKET\>/<ENCODED_UUID_UID\>/T as hinted in [1]. The full list >> is returned by newly added PlatformSupportImpl#getTemporaryDirectories(). >> >> The attaching client's VirtualMachineImpl needs the target process's temp >> directory to find .java<pid\> and create .attach<pid\> files. It uses the >> list returned by PlatformSupportImpl#getTemporaryDirectories() and the >> ProcessHandle of the target process to search for user's PerfData directory, >> e.g. hsperfdata_<username\>, which is in the target process's temp >> directory, exactly where it expects to see the .java<pid\> in return on >> sending SIGQUIT to the target VM. >> >> Mmap_attach_shared() traverses the /var/folders in get_user_tmp_dir() and >> looks for a hsperfdata_<username\> folder. If that folder is found in >> /var/folders/*/*/T, that means the temp folder corresponds to the >> <username\> and to t... > > Sergey Chernyshev has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains eight additional > commits since the last revision: > > - Merge branch 'master' into JDK-8319589 > - Merge branch 'master' into JDK-8319589 > - Removed unused native method > - addressed review comment #2 > - Apply suggestions from code review > > Co-authored-by: David Holmes > <[email protected]> > - Update > src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java > > Co-authored-by: Andrey Turbanov <[email protected]> > - addressed review comments > - 8319589: Attach from root to a user java process not supported in Mac src/hotspot/os/bsd/os_bsd.cpp line 881: > 879: uid_t uid = get_process_uid(pid); > 880: return (uid != (uid_t)-1) ? os::Posix::is_root(uid) : false; > 881: } Nit: - missed dot at the end of comments at lines 864, 877 - indent above has to be 2, not 4 src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java line 78: > 76: // Then we attempt to find the socket file again. > 77: // In macOS the socket file is located in per-user temp directory. > 78: String tempDir = getTempDirFromPid(pid); Nit: I'd suggest to rename it to `tempdir` to follow the local naming convention which prevails. src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java line 223: > 221: } > 222: > 223: private File createAttachFile(String dir, int pid) throws > IOException { Nit: I'd suggest to rename `dir` to `tmpdir` to keep it unified. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2583476499 PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2583485123 PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2583487596
