On Tue, 30 Jan 2024 12:13:05 GMT, Per Lundberg <d...@openjdk.org> wrote:
>> 8226919: attach in linux hangs due to permission denied accessing >> /proc/pid/root > > src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java line > 217: > >> 215: // Instead, attach relative to the target root filesystem as >> exposed by >> 216: // procfs regardless of namespaces. >> 217: String root = "/proc/" + pid + "/root/" + tmpdir; > > Helping myself and other future readers understand this: the problem with the > previous implementation is that the code _assumed_ that the tmpdir could be > accessed this way (`/proc/<pid>/root/<tmpdir>`). In other words: > > * The code for creating the socket would correctly check if `pid != ns_pid` > and then act accordingly (`/proc/<pid>/root/<tmpdir>` or just plain > `<tmpdir>`) > * The code for reading the socket would not have the check the above. It > would resort to always use `/proc/<pid>/root/<tmpdir>`. > * For certain scenarios (`CAP_NET_BIND_SERVICE`-processes, as described in > https://github.com/openjdk/jdk/pull/17628#issuecomment-1916589081), we would > get a `Permission denied` when trying to access the temporary directory like > this. > > What this PR does is to ensure that the same `pid != ns_pid` check is used > both when creating and reading the socket, and fall back to `<tmpdir>` when > no namespacing is being used. This seems to work better for these processes > with elevated permissions. should it not be comparing pid namespace ids and not pids? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17628#discussion_r1586542476
