[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR 8197387: jcmd started by "root" must be allowed to access all VM processes
From: "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date: 2018-05-31 18:45:31
Message-ID: ef582273-1fa8-862c-0259-226e733946f7 () oracle ! com
[Download RAW message or body]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Looks good.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 5/31/18 11:30, Daniil Titov wrote:<br>
</div>
<blockquote type="cite"
cite="mid:DCB008EA-7FDF-4D8C-88C1-5B82207A9D3F@oracle.com">
<meta http-equiv="Context-Type" content="text/html; charset=UTF-8">
<meta name="Generator" content="Microsoft Word 15 (filtered
medium)">
<div class="WordSection1">
<p class="MsoNormal"><span lang="ES">Thank you, Serguei!</span></p>
<p class="MsoNormal"><span lang="ES"> </span></p>
<p class="MsoNormal">I updated the webrev with this minor change
in the comment.</p>
<p class="MsoNormal"> </p>
<p class="MsoNormal"><span lang="ES">Webrev: <a
href="http://cr.openjdk.java.net/%7Edtitov/8197387/webrev.04"
moz-do-not-send="true">http://cr.openjdk.java.net/~dtitov/8197387/webrev.04</a>
</span></p>
<p class="MsoNormal">Issue: <a
href="https://bugs.openjdk.java.net/browse/JDK-8197387"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8197387</a></p>
<p class="MsoNormal"> </p>
<p class="MsoNormal">Best regards,</p>
<p class="MsoNormal">Daniil</p>
<p class="MsoNormal"> </p>
<div>
<p class="MsoNormal"><b><span>From: </span></b><span><a \
class="moz-txt-link-rfc2396E" \
href="mailto:serguei.spitsyn@oracle.com">"serguei.spitsyn@oracle.com"</a>
<a class="moz-txt-link-rfc2396E" \
href="mailto:serguei.spitsyn@oracle.com"><serguei.spitsyn@oracle.com></a><br> \
<b>Date: </b>Thursday, May 31, 2018 at 10:33 AM<br> <b>To: </b>Daniil Titov
<a class="moz-txt-link-rfc2396E" \
href="mailto:daniil.x.titov@oracle.com"><daniil.x.titov@oracle.com></a>, Thomas \
Stüfe
<a class="moz-txt-link-rfc2396E" \
href="mailto:thomas.stuefe@gmail.com"><thomas.stuefe@gmail.com></a>, <a \
class="moz-txt-link-rfc2396E" \
href="mailto:serviceability-dev@openjdk.java.netserviceability-dev@openjdk.java.net">"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"</a>
<a class="moz-txt-link-rfc2396E" \
href="mailto:serviceability-dev@openjdk.java.net"><serviceability-dev@openjdk.java.net></a><br>
<b>Cc: </b><a class="moz-txt-link-rfc2396E" \
href="mailto:ppc-aix-port-dev@openjdk.java.net"><ppc-aix-port-dev@openjdk.java.net></a><br>
<b>Subject: </b>Re: RFR 8197387: jcmd started by "root"
must be allowed to access all VM processes</span></p>
</div>
<div>
<p class="MsoNormal"> </p>
</div>
<div>
<p class="MsoNormal">Hi Daniil,<br>
<br>
It looks good to me.<br>
<br>
A minor comment:<br>
<br>
<a
href="http://cr.openjdk.java.net/%7Edtitov/8197387/webrev.03/src/hotspot/os/posix/os_posix.hpp.udiff.html"
moz-do-not-send="true">http://cr.openjdk.java.net/~dtitov/8197387/webrev.03/src/hotspot/os/posix/os_posix.hpp.udiff.html</a></p>
<pre><span class="new">+ // Returns true if given uid is effective uid or \
if given uid is root.</span></pre>
<pre><span class="new">+ static bool matches_effective_uid_or_root(uid_t \
uid);</span></pre> <pre><span class="new"> </span></pre>
<pre><span class="new">What about to simplify the comment above? \
:</span></pre>
<pre><span class="new"> // Returns true if given uid is effective or root \
uid.</span></pre> <pre> </pre>
<p class="MsoNormal">Thanks,<br>
Serguei<br>
<br>
<br>
On 5/30/18 12:08, Daniil Titov wrote:</p>
</div>
<blockquote>
<pre>Thank you, Thomas, for verifying this!</pre>
<pre> </pre>
<pre>I checked over this email thread and believe I still need one more \
reviewer for this fix.</pre> <pre> </pre>
<pre>Webrev: <a \
href="http://cr.openjdk.java.net/%7Edtitov/8197387/webrev.03/" \
moz-do-not-send="true">http://cr.openjdk.java.net/~dtitov/8197387/webrev.03/</a></pre>
<pre>Issue: <a href="https://bugs.openjdk.java.net/browse/JDK-8197387" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8197387</a></pre> \
<pre> </pre> <pre>Best regards,</pre>
<pre>Daniil</pre>
<pre> </pre>
<pre>On 5/30/18, 11:16 AM, "Thomas Stüfe" <a \
href="mailto:thomas.stuefe@gmail.com" \
moz-do-not-send="true"><thomas.stuefe@gmail.com></a> wrote:</pre> <pre> \
</pre> <pre> On Wed, May 30, 2018 at 7:27 PM, Daniil Titov <a \
href="mailto:daniil.x.titov@oracle.com" \
moz-do-not-send="true"><daniil.x.titov@oracle.com></a> wrote:</pre> <pre> \
> Hi Thomas,</pre> <pre> ></pre>
<pre> > Thank you for the review. Just in in case I put an \
updated webrev with suggested changes at <a \
href="http://cr.openjdk.java.net/%7Edtitov/8197387/webrev.03/" \
moz-do-not-send="true">http://cr.openjdk.java.net/~dtitov/8197387/webrev.03/</a> \
.</pre> <pre> ></pre>
<pre> > I am also including <a \
href="mailto:ppc-aix-port-dev@openjdk.java.net" \
moz-do-not-send="true">ppc-aix-port-dev@openjdk.java.net</a> mail list since the \
changes affect AIX native code. I did these AIX changes myself and I need to get them \
verified before the push.</pre> <pre> ></pre>
<pre> > May I ask someone who has access to an AIX machine try \
this patch to ensure that AIX build is fine?</pre> <pre> ></pre>
<pre> </pre>
<pre> AIX builds fine.</pre>
<pre> </pre>
<pre> Thanks, Thomas</pre>
<pre> </pre>
<pre> > Webrev: <a \
href="http://cr.openjdk.java.net/%7Edtitov/8197387/webrev.03/" \
moz-do-not-send="true">http://cr.openjdk.java.net/~dtitov/8197387/webrev.03/</a></pre>
<pre> > Issue: <a href="https://bugs.openjdk.java.net/browse/JDK-8197387" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8197387</a></pre> \
<pre> ></pre> <pre> > Thanks a lot!</pre>
<pre> ></pre>
<pre> > Best regards,</pre>
<pre> > Daniil</pre>
<pre> ></pre>
<pre> > On 5/29/18, 9:54 PM, "Thomas Stüfe" <a \
href="mailto:thomas.stuefe@gmail.com" \
moz-do-not-send="true"><thomas.stuefe@gmail.com></a> wrote:</pre> <pre> \
></pre> <pre> > Hi Daniil,</pre>
<pre> ></pre>
<pre> > Looks fine. Small nits:</pre>
<pre> ></pre>
<pre> > - please add short comments to os_posix.hpp on the \
new prototypes. At</pre>
<pre> > least on \
matches_effective_uid_and_gid_or_root:</pre>
<pre> > e.g. // Returns true if either given uid is \
effective uid and given</pre>
<pre> > gid is effective gid, or if given uid is \
root.</pre> <pre> ></pre>
<pre> > - src/hotspot/os/posix/os_posix.cpp:</pre>
<pre> ></pre>
<pre> > +bool \
os::Posix::matches_effective_uid_and_gid_or_root(uid_t uid, gid_t \
gid) {</pre>
<pre> > + return is_root(uid) || ( geteuid() == uid \
&& getegid() == gid);</pre> <pre> > +}</pre>
<pre> > +</pre>
<pre> > please remove extra space before geteuid()</pre>
<pre> ></pre>
<pre> > If you change above points, I do not need another \
webrev. Reviewed.</pre> <pre> ></pre>
<pre> > --</pre>
<pre> ></pre>
<pre> > Btw: I wonder why we find it necessary in the \
hotspot to check for</pre>
<pre> > both uid AND gid to match effective uid and \
effective gid? Seems</pre>
<pre> > strange. But since this logic is not touched by \
your change, your</pre> <pre> > change is okay.</pre>
<pre> ></pre>
<pre> > Best Regards, Thomas</pre>
<pre> ></pre>
<pre> ></pre>
<pre> ></pre>
<pre> > On Tue, May 29, 2018 at 11:33 PM, Daniil \
Titov</pre>
<pre> > <a href="mailto:daniil.x.titov@oracle.com" \
moz-do-not-send="true"><daniil.x.titov@oracle.com></a> wrote:</pre> <pre> \
> > Hi Thomas,</pre> <pre> > ></pre>
<pre> > > Please review a new version of the fix that \
includes the changes suggested.</pre> <pre> > ></pre>
<pre> > > Webrev: <a \
href="http://cr.openjdk.java.net/%7Edtitov/8197387/webrev.02/" \
moz-do-not-send="true">http://cr.openjdk.java.net/~dtitov/8197387/webrev.02/</a></pre>
<pre> > > Bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8197387" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8197387</a></pre> \
<pre> > ></pre> <pre> > > Thank you,</pre>
<pre> > > Daniil</pre>
<pre> > ></pre>
<pre> > ></pre>
<pre> > > On 5/24/18, 10:51 PM, "Thomas Stüfe" <a \
href="mailto:thomas.stuefe@gmail.com" \
moz-do-not-send="true"><thomas.stuefe@gmail.com></a> wrote:</pre> <pre> \
> ></pre> <pre> > > Hi Daniil,</pre>
<pre> > ></pre>
<pre> > > here is my review:</pre>
<pre> > ></pre>
<pre> > > - Like Roger I would prefer to have \
the uid checks factored out. At</pre>
<pre> > > least for the hotspot coding, I do \
not know where to put it in jdk</pre>
<pre> > > coding. For the hotspot parts, I \
would add something like:</pre> <pre> > ></pre>
<pre> > > os::Posix::is_root(uid_t uid) ;</pre>
<pre> > > \
os::Posix::matches_effective_uid_or_root(uid_t uid) // return</pre>
<pre> > > isroot(uid) || uid == geteuid</pre>
<pre> > > \
os::Posix::matches_effective_group_id(gid_t gid) // return gid == \
getegid</pre> <pre> > ></pre>
<pre> > > to os_posix.hpp/os_posix.cpp</pre>
<pre> > ></pre>
<pre> > > Other than that, the changes make \
sense.</pre> <pre> > ></pre>
<pre> > > Kind Regards, Thomas</pre>
<pre> > ></pre>
<pre> > ></pre>
<pre> > ></pre>
<pre> > ></pre>
<pre> > > On Thu, May 24, 2018 at 3:11 AM, \
Daniil Titov <a href="mailto:daniil.x.titov@oracle.com" \
moz-do-not-send="true"><daniil.x.titov@oracle.com></a> \
wrote:</pre>
<pre> > > > Please review the changes that \
fix JDK-8197387.</pre> <pre> > > ></pre>
<pre> > > > There are 2 problems here:</pre>
<pre> > > > 1. JVM ignores \
.attach_pid<pid> file if it is owned by the user different from the one that \
owns this JVM process</pre> <pre> > > > 2. jcmd \
checks that .java_pid<pid> socket is owned by the same user that runs jcmd and \
reports an error otherwise</pre> <pre> > > ></pre>
<pre> > > > The fix relaxes these checks to \
allow jcmd started by "root" (UID = 0) access JVMs started by another \
users.</pre> <pre> > > ></pre>
<pre> > > > Bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8197387" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8197387</a></pre> \
<pre> > > > Webrev: <a \
href="http://cr.openjdk.java.net/%7Edtitov/8197387/webrev.01/" \
moz-do-not-send="true">http://cr.openjdk.java.net/~dtitov/8197387/webrev.01/</a></pre>
<pre> > > ></pre>
<pre> > > > Best regards,</pre>
<pre> > > > Daniil</pre>
<pre> > > ></pre>
<pre> > > ></pre>
<pre> > ></pre>
<pre> > ></pre>
<pre> > ></pre>
<pre> ></pre>
<pre> ></pre>
<pre> ></pre>
<pre> </pre>
<pre> </pre>
<pre> </pre>
</blockquote>
<p class="MsoNormal"><br>
<br>
</p>
</div>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic