diff options
author | Etan Cohen <etancohen@google.com> | 2019-10-29 12:26:09 -0700 |
---|---|---|
committer | Etan Cohen <etancohen@google.com> | 2019-10-29 14:34:11 -0700 |
commit | 31f4cf48ac922a0ab79eb1a3d94b147dc72b4b1b (patch) | |
tree | dedf0552efd31c62aef67460af71d22d61b422d3 | |
parent | 5147e660f665977a7262696d2936ff5595e143d2 (diff) |
[WifiLock] Move arg validity check earlier - prevent service crash
Move the argument validity check into the binder thread - which crashes
the caller as opposed to the service.
Bug: 143518156
Test: atest com.android.server.wifi
Merged-In: Ic7c3f59524aa6a0e2aa41b34dd8179b31dba84e3
Change-Id: Ic7c3f59524aa6a0e2aa41b34dd8179b31dba84e3
4 files changed, 24 insertions, 16 deletions
diff --git a/service/java/com/android/server/wifi/WifiLockManager.java b/service/java/com/android/server/wifi/WifiLockManager.java index e292a84d8..45e68824c 100644 --- a/service/java/com/android/server/wifi/WifiLockManager.java +++ b/service/java/com/android/server/wifi/WifiLockManager.java @@ -183,10 +183,6 @@ public class WifiLockManager { * @return true if the lock was successfully acquired, false if the lockMode was invalid. */ public boolean acquireWifiLock(int lockMode, String tag, IBinder binder, WorkSource ws) { - if (!isValidLockMode(lockMode)) { - throw new IllegalArgumentException("lockMode =" + lockMode); - } - // Make a copy of the WorkSource before adding it to the WakeLock // This is to make sure worksource value can not be changed by caller // after function returns. @@ -384,7 +380,13 @@ public class WifiLockManager { } } - private static boolean isValidLockMode(int lockMode) { + /** + * Validate that the lock mode is valid - i.e. one of the supported enumerations. + * + * @param lockMode The lock mode to verify. + * @return true for valid lock modes, false otherwise. + */ + public static boolean isValidLockMode(int lockMode) { if (lockMode != WifiManager.WIFI_MODE_FULL && lockMode != WifiManager.WIFI_MODE_SCAN_ONLY && lockMode != WifiManager.WIFI_MODE_FULL_HIGH_PERF diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java index 4dbf71fd7..0afa317c9 100644 --- a/service/java/com/android/server/wifi/WifiServiceImpl.java +++ b/service/java/com/android/server/wifi/WifiServiceImpl.java @@ -2823,6 +2823,10 @@ public class WifiServiceImpl extends BaseWifiService { WorkSource updatedWs = (ws == null || ws.isEmpty()) ? new WorkSource(Binder.getCallingUid()) : ws; + if (!WifiLockManager.isValidLockMode(lockMode)) { + throw new IllegalArgumentException("lockMode =" + lockMode); + } + Mutable<Boolean> lockSuccess = new Mutable<>(); boolean runWithScissorsSuccess = mWifiInjector.getClientModeImplHandler().runWithScissors( () -> { diff --git a/tests/wifitests/src/com/android/server/wifi/WifiLockManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiLockManagerTest.java index a40de55e9..255073b8b 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiLockManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiLockManagerTest.java @@ -161,17 +161,6 @@ public class WifiLockManagerTest { } /** - * Test to verify that the lock mode is verified before adding a lock. - * - * Steps: call acquireWifiLock with an invalid lock mode. - * Expected: the call should throw an IllegalArgumentException. - */ - @Test(expected = IllegalArgumentException.class) - public void acquireWifiLockShouldThrowExceptionOnInivalidLockMode() throws Exception { - mWifiLockManager.acquireWifiLock(WIFI_LOCK_MODE_INVALID, "", mBinder, mWorkSource); - } - - /** * Test that a call to acquireWifiLock with valid parameters works. * * Steps: call acquireWifiLock on the empty WifiLockManager. diff --git a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java index bd8129a38..d5f7dd981 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java @@ -4271,4 +4271,17 @@ public class WifiServiceImplTest { } catch (RemoteException e) { } } + + /** + * Test to verify that the lock mode is verified before dispatching the operation + * + * Steps: call acquireWifiLock with an invalid lock mode. + * Expected: the call should throw an IllegalArgumentException. + */ + @Test(expected = IllegalArgumentException.class) + public void acquireWifiLockShouldThrowExceptionOnInvalidLockMode() throws Exception { + final int wifiLockModeInvalid = -1; + + mWifiServiceImpl.acquireWifiLock(mAppBinder, wifiLockModeInvalid, "", null); + } } |