RFR: JDK-8149461: jmap kills process if non-java pid is specified in the command line

[Thomas Stüfe]

Hi Gary,

thanks for taking this on. I have occasionally killed a foreign process
with jcmd and it is a bit embarrassing :)

I think your patch is okay, but I wonder whether a check directly in
libattach would be simpler and cover all uses of the attach framework.

Such a test could be e.g. simply checking /proc/<pid>/cmdline whether it is
a java launcher. Or even better - since this would not work with a custom
launcher - check /proc/<pid>/maps for a mapping of a libjvm.so.

This could even be done in native coding, right before sending SIGQUIT.

Cheers, Thomas








On Thu, Feb 14, 2019 at 8:11 PM Gary Adams <gary.adams at oracle.com> wrote:

> The following commands present a similar kill process behavior:
>     jcmd
>     jinfo
>     jmap
>     jstack
>
> The following commands do not attach.
>     jstat sun.jvmstat.monitor.MonitorException "not found"
>     jps no pid arguments
>
> This update moves the checkJavaPid method into the
> common/ProcessArgumentsMatcher.java
> and applies the check before the pid is used for an attach operation.
>
>    Revised webrev: http://cr.openjdk.java.net/~gadams/8149461/webrev.01/
>
> On 2/14/19, 12:07 PM, Chris Plummer wrote:
> > Hi Gary,
> >
> > What about the other tools that attach to a user specified process. Do
> > they also have this issue?
> >
> > thanks,
> >
> > Chris
> >
> > On 2/14/19 8:35 AM, Gary Adams wrote:
> >> There is an old reported problem that using jmap on a process that is
> >> not running
> >> Java could cause the process to terminate. This is due to the SIGQUIT
> >> used
> >> when attaching to the process.
> >>
> >> It is a fairly simple operation to validate that the pid matches one
> >> of the known
> >> running Java processes using VirtualMachine.list().
> >>
> >>   Issue: https://bugs.openjdk.java.net/browse/JDK-8149461
> >>
> >> Proposed fix:
> >>
> >> diff --git a/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
> >> b/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
> >> --- a/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
> >> +++ b/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
> >> @@ -1,5 +1,5 @@
> >>  /*
> >> - * Copyright (c) 2005, 2018, Oracle and/or its affiliates. All
> >> rights reserved.
> >> + * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All
> >> rights reserved.
> >>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> >>   *
> >>   * This code is free software; you can redistribute it and/or modify it
> >> @@ -30,6 +30,7 @@
> >>  import java.io.InputStream;
> >>  import java.io.UnsupportedEncodingException;
> >>  import java.util.Collection;
> >> +import java.util.List;
> >>
> >>  import com.sun.tools.attach.VirtualMachine;
> >>  import com.sun.tools.attach.VirtualMachineDescriptor;
> >> @@ -125,6 +126,10 @@
> >>      private static void executeCommandForPid(String pid, String
> >> command, Object ... args)
> >>          throws AttachNotSupportedException, IOException,
> >>                 UnsupportedEncodingException {
> >> +        // Before attaching, confirm that pid is a known Java process
> >> +        if (!checkJavaPid(pid)) {
> >> +            throw new AttachNotSupportedException();
> >> +        }
> >>          VirtualMachine vm = VirtualMachine.attach(pid);
> >>
> >>          // Cast to HotSpotVirtualMachine as this is an
> >> @@ -196,6 +201,19 @@
> >>          executeCommandForPid(pid, "dumpheap", filename, liveopt);
> >>      }
> >>
> >> +    // Check that pid matches a known running Java process
> >> +    static boolean checkJavaPid(String pid) {
> >> +        List<VirtualMachineDescriptor> l = VirtualMachine.list();
> >> +        boolean found = false;
> >> +        for (VirtualMachineDescriptor vmd: l) {
> >> +            if (vmd.id().equals(pid)) {
> >> +                found = true;
> >> +                break;
> >> +            }
> >> +        }
> >> +        return found;
> >> +    }
> >> +
> >>      private static void checkForUnsupportedOptions(String[] args) {
> >>          // Check arguments for -F, -m, and non-numeric value
> >>          // and warn the user that SA is not supported anymore
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190214/fe698a5a/attachment.html>