On Tue, 30 Jan 2024 10:47:22 GMT, Sebastian Lövdahl <d...@openjdk.org> wrote:

> 8307977: jcmd and jstack broken for target processes running with elevated 
> capabilities

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) 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.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17628#discussion_r1471105891

Reply via email to