Skip to content
Browse files

Handle multi-user mountObb() requests.

Since emulated external storage paths differ based on execution
context, carefully fix up paths for various use-cases:

1. When sending paths to DefaultContainerService, always scope
   OBB paths as belonging to USER_OWNER.
2. When sending paths to vold, always build emulated storage paths
   visible to root.
3. Always use the original untouched path when talking with apps.

Mount OBB containers using shared app GID, so that an app can read
the mount point across users.

Handle legacy paths like "/sdcard" by resolving the canonical path
before sending to MountService.  Move tests to servicestests, and
add tests for new path generation logic.

Bug: 7212801
Change-Id: I078c52879cd08d9c8a52cc8c83ac7ced1e8035e7
  • Loading branch information...
1 parent 5e21bf9 commit 4fbbda4cecb078bd3867f416b02cc75f5455284f @jsharkey jsharkey committed Sep 24, 2012
View
22 core/java/android/os/Environment.java
@@ -31,6 +31,7 @@
private static final String TAG = "Environment";
private static final String ENV_EXTERNAL_STORAGE = "EXTERNAL_STORAGE";
+ private static final String ENV_EMULATED_STORAGE_SOURCE = "EMULATED_STORAGE_SOURCE";
private static final String ENV_EMULATED_STORAGE_TARGET = "EMULATED_STORAGE_TARGET";
private static final String ENV_MEDIA_STORAGE = "MEDIA_STORAGE";
@@ -134,6 +135,10 @@ public File getExternalStorageDirectory() {
return mExternalStorage;
}
+ public File getExternalStorageObbDirectory() {
+ return mExternalStorageAndroidObb;
+ }
+
public File getExternalStoragePublicDirectory(String type) {
return new File(mExternalStorage, type);
}
@@ -302,6 +307,23 @@ public static File getLegacyExternalStorageDirectory() {
return new File(System.getenv(ENV_EXTERNAL_STORAGE));
}
+ /** {@hide} */
+ public static File getLegacyExternalStorageObbDirectory() {
+ return buildPath(getLegacyExternalStorageDirectory(), DIRECTORY_ANDROID, "obb");
+ }
+
+ /** {@hide} */
+ public static File getEmulatedStorageSource(int userId) {
+ // /mnt/shell/emulated/0
+ return new File(System.getenv(ENV_EMULATED_STORAGE_SOURCE), String.valueOf(userId));
+ }
+
+ /** {@hide} */
+ public static File getEmulatedStorageObbSource() {
+ // /mnt/shell/emulated/obb
+ return new File(System.getenv(ENV_EMULATED_STORAGE_SOURCE), "obb");
+ }
+
/**
* Standard directory in which to place any audio files that should be
* in the regular list of music for the user.
View
41 core/java/android/os/storage/IMountService.java
@@ -489,13 +489,14 @@ public void finishMediaUpdate() throws RemoteException {
* IObbActionListener to inform it of the terminal state of the
* call.
*/
- public void mountObb(String filename, String key, IObbActionListener token, int nonce)
- throws RemoteException {
+ public void mountObb(String rawPath, String canonicalPath, String key,
+ IObbActionListener token, int nonce) throws RemoteException {
Parcel _data = Parcel.obtain();
Parcel _reply = Parcel.obtain();
try {
_data.writeInterfaceToken(DESCRIPTOR);
- _data.writeString(filename);
+ _data.writeString(rawPath);
+ _data.writeString(canonicalPath);
_data.writeString(key);
_data.writeStrongBinder((token != null ? token.asBinder() : null));
_data.writeInt(nonce);
@@ -514,13 +515,14 @@ public void mountObb(String filename, String key, IObbActionListener token, int
* IObbActionListener to inform it of the terminal state of the
* call.
*/
- public void unmountObb(String filename, boolean force, IObbActionListener token,
- int nonce) throws RemoteException {
+ public void unmountObb(
+ String rawPath, boolean force, IObbActionListener token, int nonce)
+ throws RemoteException {
Parcel _data = Parcel.obtain();
Parcel _reply = Parcel.obtain();
try {
_data.writeInterfaceToken(DESCRIPTOR);
- _data.writeString(filename);
+ _data.writeString(rawPath);
_data.writeInt((force ? 1 : 0));
_data.writeStrongBinder((token != null ? token.asBinder() : null));
_data.writeInt(nonce);
@@ -536,13 +538,13 @@ public void unmountObb(String filename, boolean force, IObbActionListener token,
* Checks whether the specified Opaque Binary Blob (OBB) is mounted
* somewhere.
*/
- public boolean isObbMounted(String filename) throws RemoteException {
+ public boolean isObbMounted(String rawPath) throws RemoteException {
Parcel _data = Parcel.obtain();
Parcel _reply = Parcel.obtain();
boolean _result;
try {
_data.writeInterfaceToken(DESCRIPTOR);
- _data.writeString(filename);
+ _data.writeString(rawPath);
mRemote.transact(Stub.TRANSACTION_isObbMounted, _data, _reply, 0);
_reply.readException();
_result = 0 != _reply.readInt();
@@ -556,13 +558,13 @@ public boolean isObbMounted(String filename) throws RemoteException {
/**
* Gets the path to the mounted Opaque Binary Blob (OBB).
*/
- public String getMountedObbPath(String filename) throws RemoteException {
+ public String getMountedObbPath(String rawPath) throws RemoteException {
Parcel _data = Parcel.obtain();
Parcel _reply = Parcel.obtain();
String _result;
try {
_data.writeInterfaceToken(DESCRIPTOR);
- _data.writeString(filename);
+ _data.writeString(rawPath);
mRemote.transact(Stub.TRANSACTION_getMountedObbPath, _data, _reply, 0);
_reply.readException();
_result = _reply.readString();
@@ -1042,15 +1044,14 @@ public boolean onTransact(int code, Parcel data, Parcel reply,
}
case TRANSACTION_mountObb: {
data.enforceInterface(DESCRIPTOR);
- String filename;
- filename = data.readString();
- String key;
- key = data.readString();
+ final String rawPath = data.readString();
+ final String canonicalPath = data.readString();
+ final String key = data.readString();
IObbActionListener observer;
observer = IObbActionListener.Stub.asInterface(data.readStrongBinder());
int nonce;
nonce = data.readInt();
- mountObb(filename, key, observer, nonce);
+ mountObb(rawPath, canonicalPath, key, observer, nonce);
reply.writeNoException();
return true;
}
@@ -1194,7 +1195,7 @@ public int createSecureContainer(String id, int sizeMb, String fstype, String ke
/**
* Gets the path to the mounted Opaque Binary Blob (OBB).
*/
- public String getMountedObbPath(String filename) throws RemoteException;
+ public String getMountedObbPath(String rawPath) throws RemoteException;
/**
* Gets an Array of currently known secure container IDs
@@ -1220,7 +1221,7 @@ public int createSecureContainer(String id, int sizeMb, String fstype, String ke
* Checks whether the specified Opaque Binary Blob (OBB) is mounted
* somewhere.
*/
- public boolean isObbMounted(String filename) throws RemoteException;
+ public boolean isObbMounted(String rawPath) throws RemoteException;
/*
* Returns true if the specified container is mounted
@@ -1243,8 +1244,8 @@ public int createSecureContainer(String id, int sizeMb, String fstype, String ke
* MountService will call back to the supplied IObbActionListener to inform
* it of the terminal state of the call.
*/
- public void mountObb(String filename, String key, IObbActionListener token, int nonce)
- throws RemoteException;
+ public void mountObb(String rawPath, String canonicalPath, String key,
+ IObbActionListener token, int nonce) throws RemoteException;
/*
* Mount a secure container with the specified key and owner UID. Returns an
@@ -1287,7 +1288,7 @@ public void mountObb(String filename, String key, IObbActionListener token, int
* MountService will call back to the supplied IObbActionListener to inform
* it of the terminal state of the call.
*/
- public void unmountObb(String filename, boolean force, IObbActionListener token, int nonce)
+ public void unmountObb(String rawPath, boolean force, IObbActionListener token, int nonce)
throws RemoteException;
/*
View
57 core/java/android/os/storage/StorageManager.java
@@ -28,6 +28,10 @@
import android.util.Log;
import android.util.SparseArray;
+import com.android.internal.util.Preconditions;
+
+import java.io.File;
+import java.io.IOException;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.List;
@@ -443,25 +447,23 @@ public boolean isUsbMassStorageEnabled() {
* That is, shared UID applications can attempt to mount any other
* application's OBB that shares its UID.
*
- * @param filename the path to the OBB file
+ * @param rawPath the path to the OBB file
* @param key secret used to encrypt the OBB; may be <code>null</code> if no
* encryption was used on the OBB.
* @param listener will receive the success or failure of the operation
* @return whether the mount call was successfully queued or not
*/
- public boolean mountObb(String filename, String key, OnObbStateChangeListener listener) {
- if (filename == null) {
- throw new IllegalArgumentException("filename cannot be null");
- }
-
- if (listener == null) {
- throw new IllegalArgumentException("listener cannot be null");
- }
+ public boolean mountObb(String rawPath, String key, OnObbStateChangeListener listener) {
+ Preconditions.checkNotNull(rawPath, "rawPath cannot be null");
+ Preconditions.checkNotNull(listener, "listener cannot be null");
try {
+ final String canonicalPath = new File(rawPath).getCanonicalPath();
final int nonce = mObbActionListener.addListener(listener);
- mMountService.mountObb(filename, key, mObbActionListener, nonce);
+ mMountService.mountObb(rawPath, canonicalPath, key, mObbActionListener, nonce);
return true;
+ } catch (IOException e) {
+ throw new IllegalArgumentException("Failed to resolve path: " + rawPath, e);
} catch (RemoteException e) {
Log.e(TAG, "Failed to mount OBB", e);
}
@@ -483,24 +485,19 @@ public boolean mountObb(String filename, String key, OnObbStateChangeListener li
* application's OBB that shares its UID.
* <p>
*
- * @param filename path to the OBB file
+ * @param rawPath path to the OBB file
* @param force whether to kill any programs using this in order to unmount
* it
* @param listener will receive the success or failure of the operation
* @return whether the unmount call was successfully queued or not
*/
- public boolean unmountObb(String filename, boolean force, OnObbStateChangeListener listener) {
- if (filename == null) {
- throw new IllegalArgumentException("filename cannot be null");
- }
-
- if (listener == null) {
- throw new IllegalArgumentException("listener cannot be null");
- }
+ public boolean unmountObb(String rawPath, boolean force, OnObbStateChangeListener listener) {
+ Preconditions.checkNotNull(rawPath, "rawPath cannot be null");
+ Preconditions.checkNotNull(listener, "listener cannot be null");
try {
final int nonce = mObbActionListener.addListener(listener);
- mMountService.unmountObb(filename, force, mObbActionListener, nonce);
+ mMountService.unmountObb(rawPath, force, mObbActionListener, nonce);
return true;
} catch (RemoteException e) {
Log.e(TAG, "Failed to mount OBB", e);
@@ -512,16 +509,14 @@ public boolean unmountObb(String filename, boolean force, OnObbStateChangeListen
/**
* Check whether an Opaque Binary Blob (OBB) is mounted or not.
*
- * @param filename path to OBB image
+ * @param rawPath path to OBB image
* @return true if OBB is mounted; false if not mounted or on error
*/
- public boolean isObbMounted(String filename) {
- if (filename == null) {
- throw new IllegalArgumentException("filename cannot be null");
- }
+ public boolean isObbMounted(String rawPath) {
+ Preconditions.checkNotNull(rawPath, "rawPath cannot be null");
try {
- return mMountService.isObbMounted(filename);
+ return mMountService.isObbMounted(rawPath);
} catch (RemoteException e) {
Log.e(TAG, "Failed to check if OBB is mounted", e);
}
@@ -534,17 +529,15 @@ public boolean isObbMounted(String filename) {
* give you the path to where you can obtain access to the internals of the
* OBB.
*
- * @param filename path to OBB image
+ * @param rawPath path to OBB image
* @return absolute path to mounted OBB image data or <code>null</code> if
* not mounted or exception encountered trying to read status
*/
- public String getMountedObbPath(String filename) {
- if (filename == null) {
- throw new IllegalArgumentException("filename cannot be null");
- }
+ public String getMountedObbPath(String rawPath) {
+ Preconditions.checkNotNull(rawPath, "rawPath cannot be null");
try {
- return mMountService.getMountedObbPath(filename);
+ return mMountService.getMountedObbPath(rawPath);
} catch (RemoteException e) {
Log.e(TAG, "Failed to find mounted path for OBB", e);
}
View
7 include/storage/IMountService.h
@@ -21,6 +21,8 @@
#include <storage/IMountShutdownObserver.h>
#include <storage/IObbActionListener.h>
+#include <utils/String8.h>
+
#include <binder/IInterface.h>
#include <binder/Parcel.h>
@@ -60,8 +62,9 @@ class IMountService: public IInterface {
String16*& containers) = 0;
virtual void shutdown(const sp<IMountShutdownObserver>& observer) = 0;
virtual void finishMediaUpdate() = 0;
- virtual void mountObb(const String16& filename, const String16& key,
- const sp<IObbActionListener>& token, const int32_t nonce) = 0;
+ virtual void mountObb(const String16& rawPath, const String16& canonicalPath,
+ const String16& key, const sp<IObbActionListener>& token,
+ const int32_t nonce) = 0;
virtual void unmountObb(const String16& filename, const bool force,
const sp<IObbActionListener>& token, const int32_t nonce) = 0;
virtual bool isObbMounted(const String16& filename) = 0;
View
5 libs/storage/IMountService.cpp
@@ -433,12 +433,13 @@ class BpMountService: public BpInterface<IMountService>
reply.readExceptionCode();
}
- void mountObb(const String16& filename, const String16& key,
+ void mountObb(const String16& rawPath, const String16& canonicalPath, const String16& key,
const sp<IObbActionListener>& token, int32_t nonce)
{
Parcel data, reply;
data.writeInterfaceToken(IMountService::getInterfaceDescriptor());
- data.writeString16(filename);
+ data.writeString16(rawPath);
+ data.writeString16(canonicalPath);
data.writeString16(key);
data.writeStrongBinder(token->asBinder());
data.writeInt32(nonce);
View
15 native/android/storage_manager.cpp
@@ -125,11 +125,20 @@ struct AStorageManager : public RefBase {
}
}
- void mountObb(const char* filename, const char* key, AStorageManager_obbCallbackFunc func, void* data) {
+ void mountObb(const char* rawPath, const char* key, AStorageManager_obbCallbackFunc func,
+ void* data) {
+ // Resolve path before sending to MountService
+ char canonicalPath[PATH_MAX];
+ if (realpath(rawPath, canonicalPath) == NULL) {
+ ALOGE("mountObb failed to resolve path %s: %s", rawPath, strerror(errno));
+ return;
+ }
+
ObbCallback* cb = registerObbCallback(func, data);
- String16 filename16(filename);
+ String16 rawPath16(rawPath);
+ String16 canonicalPath16(canonicalPath);
String16 key16(key);
- mMountService->mountObb(filename16, key16, mObbActionListener, cb->nonce);
+ mMountService->mountObb(rawPath16, canonicalPath16, key16, mObbActionListener, cb->nonce);
}
void unmountObb(const char* filename, const bool force, AStorageManager_obbCallbackFunc func, void* data) {
View
252 services/java/com/android/server/MountService.java
@@ -58,6 +58,7 @@
import android.util.Xml;
import com.android.internal.app.IMediaContainerService;
+import com.android.internal.util.Preconditions;
import com.android.internal.util.XmlUtils;
import com.android.server.NativeDaemonConnector.Command;
import com.android.server.am.ActivityManagerService;
@@ -224,22 +225,31 @@
* OBBs.
*/
final private Map<IBinder, List<ObbState>> mObbMounts = new HashMap<IBinder, List<ObbState>>();
+
+ /** Map from raw paths to {@link ObbState}. */
final private Map<String, ObbState> mObbPathToStateMap = new HashMap<String, ObbState>();
class ObbState implements IBinder.DeathRecipient {
- public ObbState(String filename, int callerUid, IObbActionListener token, int nonce)
- throws RemoteException {
- this.filename = filename;
- this.callerUid = callerUid;
+ public ObbState(String rawPath, String canonicalPath, int callingUid,
+ IObbActionListener token, int nonce) {
+ this.rawPath = rawPath;
+ this.canonicalPath = canonicalPath.toString();
+
+ final int userId = UserHandle.getUserId(callingUid);
+ this.ownerPath = buildObbPath(canonicalPath, userId, false);
+ this.voldPath = buildObbPath(canonicalPath, userId, true);
+
+ this.ownerGid = UserHandle.getSharedAppGid(callingUid);
this.token = token;
this.nonce = nonce;
}
- // OBB source filename
- String filename;
+ final String rawPath;
+ final String canonicalPath;
+ final String ownerPath;
+ final String voldPath;
- // Binder.callingUid()
- final public int callerUid;
+ final int ownerGid;
// Token of remote Binder caller
final IObbActionListener token;
@@ -268,12 +278,13 @@ public void unlink() {
@Override
public String toString() {
StringBuilder sb = new StringBuilder("ObbState{");
- sb.append("filename=");
- sb.append(filename);
- sb.append(",token=");
- sb.append(token.toString());
- sb.append(",callerUid=");
- sb.append(callerUid);
+ sb.append("rawPath=").append(rawPath);
+ sb.append(",canonicalPath=").append(canonicalPath);
+ sb.append(",ownerPath=").append(ownerPath);
+ sb.append(",voldPath=").append(voldPath);
+ sb.append(",ownerGid=").append(ownerGid);
+ sb.append(",token=").append(token);
+ sb.append(",binder=").append(getBinder());
sb.append('}');
return sb.toString();
}
@@ -1853,17 +1864,24 @@ private boolean isUidOwnerOfPackageOrSystem(String packageName, int callerUid) {
return callerUid == packageUid;
}
- public String getMountedObbPath(String filename) {
- if (filename == null) {
- throw new IllegalArgumentException("filename cannot be null");
- }
+ public String getMountedObbPath(String rawPath) {
+ Preconditions.checkNotNull(rawPath, "rawPath cannot be null");
waitForReady();
warnOnNotMounted();
+ final ObbState state;
+ synchronized (mObbPathToStateMap) {
+ state = mObbPathToStateMap.get(rawPath);
+ }
+ if (state == null) {
+ Slog.w(TAG, "Failed to find OBB mounted at " + rawPath);
+ return null;
+ }
+
final NativeDaemonEvent event;
try {
- event = mConnector.execute("obb", "path", filename);
+ event = mConnector.execute("obb", "path", state.voldPath);
event.checkCode(VoldResponseCode.AsecPathResult);
return event.getMessage();
} catch (NativeDaemonConnectorException e) {
@@ -1876,48 +1894,52 @@ public String getMountedObbPath(String filename) {
}
}
- public boolean isObbMounted(String filename) {
- if (filename == null) {
- throw new IllegalArgumentException("filename cannot be null");
- }
-
+ @Override
+ public boolean isObbMounted(String rawPath) {
+ Preconditions.checkNotNull(rawPath, "rawPath cannot be null");
synchronized (mObbMounts) {
- return mObbPathToStateMap.containsKey(filename);
+ return mObbPathToStateMap.containsKey(rawPath);
}
}
- public void mountObb(String filename, String key, IObbActionListener token, int nonce)
- throws RemoteException {
- if (filename == null) {
- throw new IllegalArgumentException("filename cannot be null");
- }
-
- if (token == null) {
- throw new IllegalArgumentException("token cannot be null");
- }
-
- final int callerUid = Binder.getCallingUid();
- final ObbState obbState = new ObbState(filename, callerUid, token, nonce);
- final ObbAction action = new MountObbAction(obbState, key);
+ @Override
+ public void mountObb(
+ String rawPath, String canonicalPath, String key, IObbActionListener token, int nonce) {
+ Preconditions.checkNotNull(rawPath, "rawPath cannot be null");
+ Preconditions.checkNotNull(canonicalPath, "canonicalPath cannot be null");
+ Preconditions.checkNotNull(token, "token cannot be null");
+
+ final int callingUid = Binder.getCallingUid();
+ final ObbState obbState = new ObbState(rawPath, canonicalPath, callingUid, token, nonce);
+ final ObbAction action = new MountObbAction(obbState, key, callingUid);
mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
if (DEBUG_OBB)
Slog.i(TAG, "Send to OBB handler: " + action.toString());
}
- public void unmountObb(String filename, boolean force, IObbActionListener token, int nonce)
- throws RemoteException {
- if (filename == null) {
- throw new IllegalArgumentException("filename cannot be null");
+ @Override
+ public void unmountObb(String rawPath, boolean force, IObbActionListener token, int nonce) {
+ Preconditions.checkNotNull(rawPath, "rawPath cannot be null");
+
+ final ObbState existingState;
+ synchronized (mObbPathToStateMap) {
+ existingState = mObbPathToStateMap.get(rawPath);
}
- final int callerUid = Binder.getCallingUid();
- final ObbState obbState = new ObbState(filename, callerUid, token, nonce);
- final ObbAction action = new UnmountObbAction(obbState, force);
- mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
+ if (existingState != null) {
+ // TODO: separate state object from request data
+ final int callingUid = Binder.getCallingUid();
+ final ObbState newState = new ObbState(
+ rawPath, existingState.canonicalPath, callingUid, token, nonce);
+ final ObbAction action = new UnmountObbAction(newState, force);
+ mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
- if (DEBUG_OBB)
- Slog.i(TAG, "Send to OBB handler: " + action.toString());
+ if (DEBUG_OBB)
+ Slog.i(TAG, "Send to OBB handler: " + action.toString());
+ } else {
+ Slog.w(TAG, "Unknown OBB mount at " + rawPath);
+ }
}
@Override
@@ -2094,7 +2116,7 @@ private void addObbStateLocked(ObbState obbState) throws RemoteException {
mObbMounts.put(binder, obbStates);
} else {
for (final ObbState o : obbStates) {
- if (o.filename.equals(obbState.filename)) {
+ if (o.rawPath.equals(obbState.rawPath)) {
throw new IllegalStateException("Attempt to add ObbState twice. "
+ "This indicates an error in the MountService logic.");
}
@@ -2118,7 +2140,7 @@ private void addObbStateLocked(ObbState obbState) throws RemoteException {
throw e;
}
- mObbPathToStateMap.put(obbState.filename, obbState);
+ mObbPathToStateMap.put(obbState.rawPath, obbState);
}
private void removeObbStateLocked(ObbState obbState) {
@@ -2133,7 +2155,7 @@ private void removeObbStateLocked(ObbState obbState) {
}
}
- mObbPathToStateMap.remove(obbState.filename);
+ mObbPathToStateMap.remove(obbState.rawPath);
}
private class ObbActionHandler extends Handler {
@@ -2241,33 +2263,32 @@ public void handleMessage(Message msg) {
synchronized (mObbMounts) {
final List<ObbState> obbStatesToRemove = new LinkedList<ObbState>();
- final Iterator<Entry<String, ObbState>> i =
- mObbPathToStateMap.entrySet().iterator();
+ final Iterator<ObbState> i = mObbPathToStateMap.values().iterator();
while (i.hasNext()) {
- final Entry<String, ObbState> obbEntry = i.next();
+ final ObbState state = i.next();
/*
* If this entry's source file is in the volume path
* that got unmounted, remove it because it's no
* longer valid.
*/
- if (obbEntry.getKey().startsWith(path)) {
- obbStatesToRemove.add(obbEntry.getValue());
+ if (state.canonicalPath.startsWith(path)) {
+ obbStatesToRemove.add(state);
}
}
for (final ObbState obbState : obbStatesToRemove) {
if (DEBUG_OBB)
- Slog.i(TAG, "Removing state for " + obbState.filename);
+ Slog.i(TAG, "Removing state for " + obbState.rawPath);
removeObbStateLocked(obbState);
try {
- obbState.token.onObbResult(obbState.filename, obbState.nonce,
+ obbState.token.onObbResult(obbState.rawPath, obbState.nonce,
OnObbStateChangeListener.UNMOUNTED);
} catch (RemoteException e) {
Slog.i(TAG, "Couldn't send unmount notification for OBB: "
- + obbState.filename);
+ + obbState.rawPath);
}
}
}
@@ -2339,14 +2360,14 @@ public void execute(ObbActionHandler handler) {
protected ObbInfo getObbInfo() throws IOException {
ObbInfo obbInfo;
try {
- obbInfo = mContainerService.getObbInfo(mObbState.filename);
+ obbInfo = mContainerService.getObbInfo(mObbState.ownerPath);
} catch (RemoteException e) {
Slog.d(TAG, "Couldn't call DefaultContainerService to fetch OBB info for "
- + mObbState.filename);
+ + mObbState.ownerPath);
obbInfo = null;
}
if (obbInfo == null) {
- throw new IOException("Couldn't read OBB file: " + mObbState.filename);
+ throw new IOException("Couldn't read OBB file: " + mObbState.ownerPath);
}
return obbInfo;
}
@@ -2357,7 +2378,7 @@ protected void sendNewStatusOrIgnore(int status) {
}
try {
- mObbState.token.onObbResult(mObbState.filename, mObbState.nonce, status);
+ mObbState.token.onObbResult(mObbState.rawPath, mObbState.nonce, status);
} catch (RemoteException e) {
Slog.w(TAG, "MountServiceListener went away while calling onObbStateChanged");
}
@@ -2366,10 +2387,12 @@ protected void sendNewStatusOrIgnore(int status) {
class MountObbAction extends ObbAction {
private final String mKey;
+ private final int mCallingUid;
- MountObbAction(ObbState obbState, String key) {
+ MountObbAction(ObbState obbState, String key, int callingUid) {
super(obbState);
mKey = key;
+ mCallingUid = callingUid;
}
@Override
@@ -2379,7 +2402,7 @@ public void handleExecute() throws IOException, RemoteException {
final ObbInfo obbInfo = getObbInfo();
- if (!isUidOwnerOfPackageOrSystem(obbInfo.packageName, mObbState.callerUid)) {
+ if (!isUidOwnerOfPackageOrSystem(obbInfo.packageName, mCallingUid)) {
Slog.w(TAG, "Denied attempt to mount OBB " + obbInfo.filename
+ " which is owned by " + obbInfo.packageName);
sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_PERMISSION_DENIED);
@@ -2388,20 +2411,14 @@ public void handleExecute() throws IOException, RemoteException {
final boolean isMounted;
synchronized (mObbMounts) {
- isMounted = mObbPathToStateMap.containsKey(obbInfo.filename);
+ isMounted = mObbPathToStateMap.containsKey(mObbState.rawPath);
}
if (isMounted) {
Slog.w(TAG, "Attempt to mount OBB which is already mounted: " + obbInfo.filename);
sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_ALREADY_MOUNTED);
return;
}
- /*
- * The filename passed in might not be the canonical name, so just
- * set the filename to the canonicalized version.
- */
- mObbState.filename = obbInfo.filename;
-
final String hashedKey;
if (mKey == null) {
hashedKey = "none";
@@ -2428,7 +2445,7 @@ public void handleExecute() throws IOException, RemoteException {
int rc = StorageResultCode.OperationSucceeded;
try {
mConnector.execute(
- "obb", "mount", mObbState.filename, hashedKey, mObbState.callerUid);
+ "obb", "mount", mObbState.voldPath, hashedKey, mObbState.ownerGid);
} catch (NativeDaemonConnectorException e) {
int code = e.getCode();
if (code != VoldResponseCode.OpFailedStorageBusy) {
@@ -2438,7 +2455,7 @@ public void handleExecute() throws IOException, RemoteException {
if (rc == StorageResultCode.OperationSucceeded) {
if (DEBUG_OBB)
- Slog.d(TAG, "Successfully mounted OBB " + mObbState.filename);
+ Slog.d(TAG, "Successfully mounted OBB " + mObbState.voldPath);
synchronized (mObbMounts) {
addObbStateLocked(mObbState);
@@ -2461,14 +2478,7 @@ public void handleError() {
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("MountObbAction{");
- sb.append("filename=");
- sb.append(mObbState.filename);
- sb.append(",callerUid=");
- sb.append(mObbState.callerUid);
- sb.append(",token=");
- sb.append(mObbState.token != null ? mObbState.token.toString() : "NULL");
- sb.append(",binder=");
- sb.append(mObbState.token != null ? mObbState.getBinder().toString() : "null");
+ sb.append(mObbState);
sb.append('}');
return sb.toString();
}
@@ -2489,28 +2499,26 @@ public void handleExecute() throws IOException {
final ObbInfo obbInfo = getObbInfo();
- final ObbState obbState;
+ final ObbState existingState;
synchronized (mObbMounts) {
- obbState = mObbPathToStateMap.get(obbInfo.filename);
+ existingState = mObbPathToStateMap.get(mObbState.rawPath);
}
- if (obbState == null) {
+ if (existingState == null) {
sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_NOT_MOUNTED);
return;
}
- if (obbState.callerUid != mObbState.callerUid) {
- Slog.w(TAG, "Permission denied attempting to unmount OBB " + obbInfo.filename
- + " (owned by " + obbInfo.packageName + ")");
+ if (existingState.ownerGid != mObbState.ownerGid) {
+ Slog.w(TAG, "Permission denied attempting to unmount OBB " + existingState.rawPath
+ + " (owned by GID " + existingState.ownerGid + ")");
sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_PERMISSION_DENIED);
return;
}
- mObbState.filename = obbInfo.filename;
-
int rc = StorageResultCode.OperationSucceeded;
try {
- final Command cmd = new Command("obb", "unmount", mObbState.filename);
+ final Command cmd = new Command("obb", "unmount", mObbState.voldPath);
if (mForceUnmount) {
cmd.appendArg("force");
}
@@ -2529,12 +2537,12 @@ public void handleExecute() throws IOException {
if (rc == StorageResultCode.OperationSucceeded) {
synchronized (mObbMounts) {
- removeObbStateLocked(obbState);
+ removeObbStateLocked(existingState);
}
sendNewStatusOrIgnore(OnObbStateChangeListener.UNMOUNTED);
} else {
- Slog.w(TAG, "Could not mount OBB: " + mObbState.filename);
+ Slog.w(TAG, "Could not unmount OBB: " + existingState);
sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_COULD_NOT_UNMOUNT);
}
}
@@ -2548,21 +2556,63 @@ public void handleError() {
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("UnmountObbAction{");
- sb.append("filename=");
- sb.append(mObbState.filename != null ? mObbState.filename : "null");
+ sb.append(mObbState);
sb.append(",force=");
sb.append(mForceUnmount);
- sb.append(",callerUid=");
- sb.append(mObbState.callerUid);
- sb.append(",token=");
- sb.append(mObbState.token != null ? mObbState.token.toString() : "null");
- sb.append(",binder=");
- sb.append(mObbState.token != null ? mObbState.getBinder().toString() : "null");
sb.append('}');
return sb.toString();
}
}
+ // @VisibleForTesting
+ public static String buildObbPath(final String canonicalPath, int userId, boolean forVold) {
+ // TODO: allow caller to provide Environment for full testing
+
+ // Only adjust paths when storage is emulated
+ if (!Environment.isExternalStorageEmulated()) {
+ return canonicalPath;
+ }
+
+ String path = canonicalPath.toString();
+
+ // First trim off any external storage prefix
+ final UserEnvironment userEnv = new UserEnvironment(userId);
+
+ // /storage/emulated/0
+ final String externalPath = userEnv.getExternalStorageDirectory().toString();
+ // /storage/emulated_legacy
+ final String legacyExternalPath = Environment.getLegacyExternalStorageDirectory()
+ .toString();
+
+ if (path.startsWith(externalPath)) {
+ path = path.substring(externalPath.length() + 1);
+ } else if (path.startsWith(legacyExternalPath)) {
+ path = path.substring(legacyExternalPath.length() + 1);
+ } else {
+ return canonicalPath;
+ }
+
+ // Handle special OBB paths on emulated storage
+ final String obbPath = "Android/obb";
+ if (path.startsWith(obbPath)) {
+ path = path.substring(obbPath.length() + 1);
+
+ if (forVold) {
+ return new File(Environment.getEmulatedStorageObbSource(), path).toString();
+ } else {
+ final UserEnvironment ownerEnv = new UserEnvironment(UserHandle.USER_OWNER);
+ return new File(ownerEnv.getExternalStorageObbDirectory(), path).toString();
+ }
+ }
+
+ // Handle normal external storage paths
+ if (forVold) {
+ return new File(Environment.getEmulatedStorageSource(userId), path).toString();
+ } else {
+ return new File(userEnv.getExternalStorageDirectory(), path).toString();
+ }
+ }
+
@Override
protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.DUMP) != PackageManager.PERMISSION_GRANTED) {
View
BIN core/tests/coretests/res/raw/test1.obb → ...ces/tests/servicestests/res/raw/test1.obb
Binary file not shown.
View
0 core/tests/coretests/res/raw/test1_nosig.obb → ...sts/servicestests/res/raw/test1_nosig.obb
File renamed without changes.
View
0 .../coretests/res/raw/test1_wrongpackage.obb → ...vicestests/res/raw/test1_wrongpackage.obb
File renamed without changes.
View
36 ...com/android/server/MountServiceTests.java → ...com/android/server/MountServiceTests.java
@@ -16,8 +16,6 @@
package com.android.server;
-import com.android.frameworks.coretests.R;
-
import android.content.Context;
import android.content.res.Resources;
import android.content.res.Resources.NotFoundException;
@@ -29,6 +27,10 @@
import android.test.suitebuilder.annotation.LargeTest;
import android.util.Log;
+import static com.android.server.MountService.buildObbPath;
+
+import com.android.frameworks.servicestests.R;
+
import java.io.File;
import java.io.InputStream;
@@ -282,4 +284,34 @@ public void testMountAndUnmountTwoObbs() {
unmountObb(sm, file1, OnObbStateChangeListener.UNMOUNTED);
unmountObb(sm, file2, OnObbStateChangeListener.UNMOUNTED);
}
+
+ public void testBuildObbPath() {
+ final int userId = 10;
+
+ // Paths outside external storage should remain untouched
+ assertEquals("/storage/random/foo",
+ buildObbPath("/storage/random/foo", userId, true));
+ assertEquals("/storage/random/foo",
+ buildObbPath("/storage/random/foo", userId, false));
+
+ // Paths on user-specific emulated storage
+ assertEquals("/mnt/shell/emulated/10/foo",
+ buildObbPath("/storage/emulated_legacy/foo", userId, true));
+ assertEquals("/storage/emulated/10/foo",
+ buildObbPath("/storage/emulated_legacy/foo", userId, false));
+ assertEquals("/mnt/shell/emulated/10/foo",
+ buildObbPath("/storage/emulated/10/foo", userId, true));
+ assertEquals("/storage/emulated/10/foo",
+ buildObbPath("/storage/emulated/10/foo", userId, false));
+
+ // Paths on shared OBB emulated storage
+ assertEquals("/mnt/shell/emulated/obb/foo",
+ buildObbPath("/storage/emulated_legacy/Android/obb/foo", userId, true));
+ assertEquals("/storage/emulated/0/Android/obb/foo",
+ buildObbPath("/storage/emulated_legacy/Android/obb/foo", userId, false));
+ assertEquals("/mnt/shell/emulated/obb/foo",
+ buildObbPath("/storage/emulated/10/Android/obb/foo", userId, true));
+ assertEquals("/storage/emulated/0/Android/obb/foo",
+ buildObbPath("/storage/emulated/10/Android/obb/foo", userId, false));
+ }
}

0 comments on commit 4fbbda4

Please sign in to comment.
Something went wrong with that request. Please try again.