Fix bug 1829561 ("am profile" with bad filename kills process).

The am command is now the one that takes care of opening the target file,
handling the opened file descriptor to the process that will be profiled.
This allows you to send profile data to anywhere the shell can access, and
avoids any problems coming up from the target process trying to open the
file.

8 files changed, 132 insertions, 55 deletions

diff --git a/cmds/am/src/com/android/commands/am/Am.java b/cmds/am/src/com/android/commands/am/Am.java
index 2a4a672..3782136 100644
--- a/cmds/am/src/com/android/commands/am/Am.java
+++ b/cmds/am/src/com/android/commands/am/Am.java
@@ -26,10 +26,13 @@ import android.content.ComponentName;
 import android.content.Intent;
 import android.net.Uri;
 import android.os.Bundle;
+import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.view.IWindowManager;
 
+import java.io.File;
+import java.io.FileNotFoundException;
 import java.util.Iterator;
 import java.util.Set;
 
@@ -446,6 +449,8 @@ public class Am {
             return;
         }
         
+        ParcelFileDescriptor fd = null;
+        
         String cmd = nextArg();
         if ("start".equals(cmd)) {
             start = true;
@@ -455,6 +460,16 @@ public class Am {
                 showUsage();
                 return;
             }
+            try {
+                fd = ParcelFileDescriptor.open(
+                        new File(profileFile),
+                        ParcelFileDescriptor.MODE_CREATE |
+                        ParcelFileDescriptor.MODE_TRUNCATE |
+                        ParcelFileDescriptor.MODE_READ_WRITE);
+            } catch (FileNotFoundException e) {
+                System.err.println("Error: Unable to open file: " + profileFile);
+                return;
+            }
         } else if (!"stop".equals(cmd)) {
             System.err.println("Error: Profile command " + cmd + " not valid");
             showUsage();
@@ -462,8 +477,8 @@ public class Am {
         }
         
         try {
-            if (!mAm.profileControl(process, start, profileFile)) {
-                System.out.println("PROFILE FAILED on process " + process);
+            if (!mAm.profileControl(process, start, profileFile, fd)) {
+                System.err.println("PROFILE FAILED on process " + process);
                 return;
             }
         } catch (IllegalArgumentException e) {
diff --git a/core/java/android/app/ActivityManagerNative.java b/core/java/android/app/ActivityManagerNative.java
index b6f855a..dfa8139 100644
--- a/core/java/android/app/ActivityManagerNative.java
+++ b/core/java/android/app/ActivityManagerNative.java
@@ -986,7 +986,9 @@ public abstract class ActivityManagerNative extends Binder implements IActivityM
             String process = data.readString();
             boolean start = data.readInt() != 0;
             String path = data.readString();
-            boolean res = profileControl(process, start, path);
+            ParcelFileDescriptor fd = data.readInt() != 0
+                    ? data.readFileDescriptor() : null;
+            boolean res = profileControl(process, start, path, fd);
             reply.writeNoException();
             reply.writeInt(res ? 1 : 0);
             return true;
@@ -2232,7 +2234,7 @@ class ActivityManagerProxy implements IActivityManager
     }
     
     public boolean profileControl(String process, boolean start,
-            String path) throws RemoteException
+            String path, ParcelFileDescriptor fd) throws RemoteException
     {
         Parcel data = Parcel.obtain();
         Parcel reply = Parcel.obtain();
@@ -2240,6 +2242,12 @@ class ActivityManagerProxy implements IActivityManager
         data.writeString(process);
         data.writeInt(start ? 1 : 0);
         data.writeString(path);
+        if (fd != null) {
+            data.writeInt(1);
+            fd.writeToParcel(data, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+        } else {
+            data.writeInt(0);
+        }
         mRemote.transact(PROFILE_CONTROL_TRANSACTION, data, reply, 0);
         reply.readException();
         boolean res = reply.readInt() != 0;
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 98bd45a..79588ea 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -48,6 +48,7 @@ import android.os.IBinder;
 import android.os.Looper;
 import android.os.Message;
 import android.os.MessageQueue;
+import android.os.ParcelFileDescriptor;
 import android.os.Process;
 import android.os.RemoteException;
 import android.os.ServiceManager;
@@ -74,6 +75,7 @@ import org.apache.harmony.xnet.provider.jsse.OpenSSLSocketImpl;
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FileOutputStream;
+import java.io.IOException;
 import java.io.PrintWriter;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
@@ -1236,6 +1238,11 @@ public final class ActivityThread {
         String who;
     }
 
+    private static final class ProfilerControlData {
+        String path;
+        ParcelFileDescriptor fd;
+    }
+
     private final class ApplicationThread extends ApplicationThreadNative {
         private static final String HEAP_COLUMN = "%17s %8s %8s %8s %8s";
         private static final String ONE_COUNT_COLUMN = "%17s %8d";
@@ -1494,8 +1501,11 @@ public final class ActivityThread {
             }
         }
         
-        public void profilerControl(boolean start, String path) {
-            queueOrSendMessage(H.PROFILER_CONTROL, path, start ? 1 : 0);
+        public void profilerControl(boolean start, String path, ParcelFileDescriptor fd) {
+            ProfilerControlData pcd = new ProfilerControlData();
+            pcd.path = path;
+            pcd.fd = fd;
+            queueOrSendMessage(H.PROFILER_CONTROL, pcd, start ? 1 : 0);
         }
 
         public void setSchedulingGroup(int group) {
@@ -1838,7 +1848,7 @@ public final class ActivityThread {
                     handleActivityConfigurationChanged((IBinder)msg.obj);
                     break;
                 case PROFILER_CONTROL:
-                    handleProfilerControl(msg.arg1 != 0, (String)msg.obj);
+                    handleProfilerControl(msg.arg1 != 0, (ProfilerControlData)msg.obj);
                     break;
                 case CREATE_BACKUP_AGENT:
                     handleCreateBackupAgent((CreateBackupAgentData)msg.obj);
@@ -3618,15 +3628,20 @@ public final class ActivityThread {
         performConfigurationChanged(r.activity, mConfiguration);
     }
 
-    final void handleProfilerControl(boolean start, String path) {
+    final void handleProfilerControl(boolean start, ProfilerControlData pcd) {
         if (start) {
-            File file = new File(path);
-            file.getParentFile().mkdirs();
             try {
-                Debug.startMethodTracing(file.toString(), 8 * 1024 * 1024);
+                Debug.startMethodTracing(pcd.path, pcd.fd.getFileDescriptor(),
+                        8 * 1024 * 1024, 0);
             } catch (RuntimeException e) {
-                Log.w(TAG, "Profiling failed on path " + path
+                Log.w(TAG, "Profiling failed on path " + pcd.path
                         + " -- can the process access this path?");
+            } finally {
+                try {
+                    pcd.fd.close();
+                } catch (IOException e) {
+                    Log.w(TAG, "Failure closing profile fd", e);
+                }
             }
         } else {
             Debug.stopMethodTracing();
diff --git a/core/java/android/app/ApplicationThreadNative.java b/core/java/android/app/ApplicationThreadNative.java
index 4b64c94..b052c99 100644
--- a/core/java/android/app/ApplicationThreadNative.java
+++ b/core/java/android/app/ApplicationThreadNative.java
@@ -26,6 +26,7 @@ import android.content.pm.ServiceInfo;
 import android.content.res.Configuration;
 import android.os.Binder;
 import android.os.Bundle;
+import android.os.Parcelable;
 import android.os.RemoteException;
 import android.os.IBinder;
 import android.os.Parcel;
@@ -331,7 +332,9 @@ public abstract class ApplicationThreadNative extends Binder
             data.enforceInterface(IApplicationThread.descriptor);
             boolean start = data.readInt() != 0;
             String path = data.readString();
-            profilerControl(start, path);
+            ParcelFileDescriptor fd = data.readInt() != 0
+                    ? data.readFileDescriptor() : null;
+            profilerControl(start, path, fd);
             return true;
         }
         
@@ -711,11 +714,18 @@ class ApplicationThreadProxy implements IApplicationThread {
         data.recycle();
     }
     
-    public void profilerControl(boolean start, String path) throws RemoteException {
+    public void profilerControl(boolean start, String path,
+            ParcelFileDescriptor fd) throws RemoteException {
         Parcel data = Parcel.obtain();
         data.writeInterfaceToken(IApplicationThread.descriptor);
         data.writeInt(start ? 1 : 0);
         data.writeString(path);
+        if (fd != null) {
+            data.writeInt(1);
+            fd.writeToParcel(data, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+        } else {
+            data.writeInt(0);
+        }
         mRemote.transact(PROFILER_CONTROL_TRANSACTION, data, null,
                 IBinder.FLAG_ONEWAY);
         data.recycle();
diff --git a/core/java/android/app/IActivityManager.java b/core/java/android/app/IActivityManager.java
index 66bc85b..3ec7938 100644
--- a/core/java/android/app/IActivityManager.java
+++ b/core/java/android/app/IActivityManager.java
@@ -250,7 +250,7 @@ public interface IActivityManager extends IInterface {
     
     // Turn on/off profiling in a particular process.
     public boolean profileControl(String process, boolean start,
-            String path) throws RemoteException;
+            String path, ParcelFileDescriptor fd) throws RemoteException;
     
     public boolean shutdown(int timeout) throws RemoteException;
     
diff --git a/core/java/android/app/IApplicationThread.java b/core/java/android/app/IApplicationThread.java
index 029c650..c0bc2a0 100644
--- a/core/java/android/app/IApplicationThread.java
+++ b/core/java/android/app/IApplicationThread.java
@@ -25,6 +25,7 @@ import android.content.pm.ProviderInfo;
 import android.content.pm.ServiceInfo;
 import android.content.res.Configuration;
 import android.os.Bundle;
+import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 import android.os.IBinder;
 import android.os.IInterface;
@@ -92,7 +93,8 @@ public interface IApplicationThread extends IInterface {
     void scheduleLowMemory() throws RemoteException;
     void scheduleActivityConfigurationChanged(IBinder token) throws RemoteException;
     void requestPss() throws RemoteException;
-    void profilerControl(boolean start, String path) throws RemoteException;
+    void profilerControl(boolean start, String path, ParcelFileDescriptor fd)
+            throws RemoteException;
     void setSchedulingGroup(int group) throws RemoteException;
     
     String descriptor = "android.app.IApplicationThread";
diff --git a/core/java/android/os/Debug.java b/core/java/android/os/Debug.java
index 8fcb4d7..d40ea6b 100644
--- a/core/java/android/os/Debug.java
+++ b/core/java/android/os/Debug.java
@@ -21,6 +21,7 @@ import com.android.internal.util.TypedProperties;
 import android.util.Config;
 import android.util.Log;
 
+import java.io.FileDescriptor;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.FileReader;
@@ -378,6 +379,20 @@ href="{@docRoot}guide/developing/tools/traceview.html">Traceview: A Graphical Lo
     }
 
     /**
+     * Like startMethodTracing(String, int, int), but taking an already-opened
+     * FileDescriptor in which the trace is written.  The file name is also
+     * supplied simply for logging.  Makes a dup of the file descriptor.
+     * 
+     * Not exposed in the SDK unless we are really comfortable with supporting
+     * this and find it would be useful.
+     * @hide
+     */
+    public static void startMethodTracing(String traceName, FileDescriptor fd,
+        int bufferSize, int flags) {
+        VMDebug.startMethodTracing(traceName, fd, bufferSize, flags);
+    }
+
+    /**
      * Determine whether method tracing is currently active.
      * @hide
      */
diff --git a/services/java/com/android/server/am/ActivityManagerService.java b/services/java/com/android/server/am/ActivityManagerService.java
index 62b4d5e..0d9d2b0 100644
--- a/services/java/com/android/server/am/ActivityManagerService.java
+++ b/services/java/com/android/server/am/ActivityManagerService.java
@@ -12601,51 +12601,63 @@ public final class ActivityManagerService extends ActivityManagerNative implemen
     }
 
     public boolean profileControl(String process, boolean start,
-            String path) throws RemoteException {
+            String path, ParcelFileDescriptor fd) throws RemoteException {
 
-        synchronized (this) {
-            // note: hijacking SET_ACTIVITY_WATCHER, but should be changed to
-            // its own permission.
-            if (checkCallingPermission(android.Manifest.permission.SET_ACTIVITY_WATCHER)
-                    != PackageManager.PERMISSION_GRANTED) {
-                throw new SecurityException("Requires permission "
-                        + android.Manifest.permission.SET_ACTIVITY_WATCHER);
-            }
-            
-            ProcessRecord proc = null;
-            try {
-                int pid = Integer.parseInt(process);
-                synchronized (mPidsSelfLocked) {
-                    proc = mPidsSelfLocked.get(pid);
+        try {
+            synchronized (this) {
+                // note: hijacking SET_ACTIVITY_WATCHER, but should be changed to
+                // its own permission.
+                if (checkCallingPermission(android.Manifest.permission.SET_ACTIVITY_WATCHER)
+                        != PackageManager.PERMISSION_GRANTED) {
+                    throw new SecurityException("Requires permission "
+                            + android.Manifest.permission.SET_ACTIVITY_WATCHER);
                 }
-            } catch (NumberFormatException e) {
-            }
-            
-            if (proc == null) {
-                HashMap<String, SparseArray<ProcessRecord>> all
-                        = mProcessNames.getMap();
-                SparseArray<ProcessRecord> procs = all.get(process);
-                if (procs != null && procs.size() > 0) {
-                    proc = procs.valueAt(0);
+                
+                if (start && fd == null) {
+                    throw new IllegalArgumentException("null fd");
                 }
-            }
-            
-            if (proc == null || proc.thread == null) {
-                throw new IllegalArgumentException("Unknown process: " + process);
-            }
-            
-            boolean isSecure = "1".equals(SystemProperties.get(SYSTEM_SECURE, "0"));
-            if (isSecure) {
-                if ((proc.info.flags&ApplicationInfo.FLAG_DEBUGGABLE) == 0) {
-                    throw new SecurityException("Process not debuggable: " + proc);
+                
+                ProcessRecord proc = null;
+                try {
+                    int pid = Integer.parseInt(process);
+                    synchronized (mPidsSelfLocked) {
+                        proc = mPidsSelfLocked.get(pid);
+                    }
+                } catch (NumberFormatException e) {
+                }
+                
+                if (proc == null) {
+                    HashMap<String, SparseArray<ProcessRecord>> all
+                            = mProcessNames.getMap();
+                    SparseArray<ProcessRecord> procs = all.get(process);
+                    if (procs != null && procs.size() > 0) {
+                        proc = procs.valueAt(0);
+                    }
+                }
+                
+                if (proc == null || proc.thread == null) {
+                    throw new IllegalArgumentException("Unknown process: " + process);
+                }
+                
+                boolean isSecure = "1".equals(SystemProperties.get(SYSTEM_SECURE, "0"));
+                if (isSecure) {
+                    if ((proc.info.flags&ApplicationInfo.FLAG_DEBUGGABLE) == 0) {
+                        throw new SecurityException("Process not debuggable: " + proc);
+                    }
                 }
-            }
             
-            try {
-                proc.thread.profilerControl(start, path);
+                proc.thread.profilerControl(start, path, fd);
+                fd = null;
                 return true;
-            } catch (RemoteException e) {
-                throw new IllegalStateException("Process disappeared");
+            }
+        } catch (RemoteException e) {
+            throw new IllegalStateException("Process disappeared");
+        } finally {
+            if (fd != null) {
+                try {
+                    fd.close();
+                } catch (IOException e) {
+                }
             }
         }
     }