RFR (S): 8202650: Enforce group for attach listener file

[Thomas Stüfe]

Hi Christoph,

You only want to change the group id of the created file, right? Could
you then please specify -1 as user id, instead of geteuid()? That
makes the intent clearer.

("If owner or group is specified as (uid_t)-1 or (gid_t)-1
respectively, the corresponding ID of the file is unchanged.")

small nit:
you check for == 0 to indicate success, some lines below we use != -1.

On both points I leave it up to you if you fix them. Patch is fine
also in its current form.


Best Regards, Thomas



On Sun, May 6, 2018 at 9:23 PM, Langer, Christoph
<christoph.langer at sap.com> wrote:
> Hi,
>
> with that information and Martins links to the specs, I suggest to add the patch to linux as well. I played with the sgid functionality on Linux and it is the same as on AIX. So, if somebody configured the directory where the attach listener file is created with sgid, the file will belong to the wrong group.
>
> See my new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8202650.1/
>
> Thanks
> Christoph
>
>> -----Original Message-----
>> From: Chris Plummer [mailto:chris.plummer at oracle.com]
>> Sent: Freitag, 4. Mai 2018 23:34
>> To: Langer, Christoph <christoph.langer at sap.com>; serviceability-
>> dev at openjdk.java.net
>> Cc: ppc-aix-port-dev at openjdk.java.net
>> Subject: Re: RFR (S): 8202650: Enforce group for attach listener file
>>
>> Hi Christoph,
>>
>> It looks like for bsd this code was added to fix JDK-7152800. In that CR
>> I see the following:
>>
>> "The attach framework will verify that the file has the same effective
>> owner and group as the currently running process. This will be true on
>> linux, since files are created with the effective user and group as
>> owner. This will NOT be true always on macos, since the file can have a
>> different group if the temporary directory has a different group than
>> what we are currently running as."
>>
>> So it looks like the fix is not necessary for Linux. It wouldn't hurt to
>> experiment by setting the s-bit on the directory and see if you have the
>> same problem as macos and AIX.
>>
>> thanks,
>>
>> Chris
>>
>> On 5/4/18 7:29 AM, Langer, Christoph wrote:
>> >
>> > Hi,
>> >
>> > please review a change for correctly setting the group for the attach
>> > listener file:
>> >
>> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8202650.0/
>> > <http://cr.openjdk.java.net/%7Eclanger/webrevs/8202650.0/>
>> >
>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8202650
>> > <https://bugs.openjdk.java.net/browse/JDK-8202650>
>> >
>> > The attach listener file, usually /tmp/.java_pid<pid> is created from
>> > the hotspot JVM process. Usually it will belong to the process user
>> > and group. However, when the directory where it is created has set the
>> > s-bit for groups, the group of the directory is taken. This will cause
>> > errors when the attach client tries to connect and it is checked
>> > whether the group of the attach file matches the client processes' group.
>> >
>> > In my webrev I only implemented the change for AIX because we have run
>> > into an issue on that platform. But I can see this code already in
>> > place for attachListener_bsd.cpp. And I’m wondering if this should
>> > also be added to attachListener_linux.cpp because the sticky-bit could
>> > be set with the same effects on Linux, too. Any opinions about that?
>> >
>> > Thanks and best regards
>> >
>> > Christoph
>> >
>>
>