8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

[Laurence Cable]

On 5/2/24 3:09 AM, Sebastian Lövdahl wrote:
> Interesting, TIL about /proc/<pid>/ns. I tried to look for something 
> like that but couldn't find anything relevant in /proc/<pid>/status.

ok

>
> So, a pixel perfect solution could compare these IDs to know whether 
> /tmp or /proc/<pid>/root/tmp should be used.
>
> > 2. jcmd treats it as a heuristic and attempts each way during the 
> socket file read - first /proc/<pid>/root/tmp and then /tmp.
>
> This is what I had in mind as well, and what I actually implemented 
> and tested already. I think I'll just go ahead and open a PR with 
> those changes, and we can take it from there.

yes, IMO the attachee should always use /proc/self/root/tmp and the 
attacher should compare /proc/<attachee_pid>/ns/mnt with its 
/proc/self/ns/mnt with Path::toString comparison,
if they match then it is "safe" to use /tmp, if they do not then it must 
resort to using /proc/<attachee_pid>/ns/mnt if the attacher does not 
have sufficient privilege to access that due
to attachee capabilities etc then the attach will fail

Rgds

- Larry
>
> /Sebastian
>
> On 2024-05-02 03:43, Laurence Cable wrote:
>> just to demonstrate:
>>
>> $ docker run -it --name=js1  openjdk:17.0.1-jdk  /bin/jshell
>> ...
>> $ docker run -it --name js2 --pid=container:js1 openjdk:17.0.1 
>> /bin/jshell
>>
>>
>> $ docker exec -it js1  bash
>> bash-4.4# ls /tmp/hsperfdata_root
>> 1  26
>> bash-4.4# readlink /proc/26/ns/pid
>> pid:[4026532751]
>> bash-4.4# readlink /proc/26/ns/mnt
>> mnt:[4026532747]
>> bash-4.4# exit
>> [lpgc at arran ~]$ docker exec -it js2  bash
>> bash-4.4# ls /tmp/hsperfdata_root
>> 107  82
>> bash-4.4# readlink /proc/107/ns/pid
>> pid:[4026532751]
>> bash-4.4# readlink /proc/107/ns/mnt
>> mnt:[4026532941]
>>
>> you will note that the JVM pid: 26 and 107 occupy the same pid 
>> namespace (4026532751) but occupy different mnt namespaces 
>> (4026532747, 4026532941)
>>
>> therefore attempting to attach via '/tmp' will fail, 
>> /proc/<pid>/root/tmp must be used to rendezvous
>>
>> - Larry
>>
>>
>> On 5/1/24 2:03 PM, Doyle, James, K wrote:
>>> Hi Sebastian,
>>>
>>>> I think I can confirm that there is a regression.
>>> Thanks for reproducing the regression, your test makes sense to me, 
>>> and I think it is similar to the scenario we have with Kubernetes 
>>> debug containers (separate filesystems, but same PID namespace).
>>>
>>> I noticed some of the other recent Pull Request comments on 
>>> https://github.com/openjdk/jdk/pull/17628:
>>>
>>>> should it not be comparing pid namespace ids and not pids?
>>> and wanted to give a little feedback.  I think more refined 
>>> approaches to figuring out whether the target JVM is in the same PID 
>>> namespace make sense and could be an improvement, but it's still 
>>> different from figuring out whether the target JVM has the same 
>>> filesystem (specifically, I guess, the filesystem containing /tmp or 
>>> java.io.tmpdir).  That seems like the crucial thing for deciding 
>>> what socket file path to read, and whether /tmp is sufficient or 
>>> /proc/<pid>/root/tmp is needed.  I can think of a couple different 
>>> approaches to the filesystem issue:
>>>
>>> 1. There is some Linux kernel information that can be obtained about 
>>> the jcmd process and the target JVM process to figure out 
>>> unequivocally what their root filesystems are from the host's point 
>>> of view, and whether they're the same.  (I don't know what this 
>>> might be, though!)
>>> 2. jcmd treats it as a heuristic and attempts each way during the 
>>> socket file read - first /proc/<pid>/root/tmp and then /tmp.
>>> 3. jcmd has some option or environment variable where the user can 
>>> tell it the socket file path.
>>>
>>> Do you agree that these are the types of choices available?
>>>
>>> Thanks,
>>> Jim
>>>
>>
>