From 38bf51466881b726f42832743d8cca6ee67bb148 Mon Sep 17 00:00:00 2001 From: Greg Hackmann Date: Wed, 19 Feb 2014 16:39:36 -0800 Subject: [PATCH 1/2] Move time setting code from SystemClock to AlarmManagerService On devices using /dev/rtc instead of /dev/alarm, updating the time-of-day clock and RTC are separate syscalls. Hence the clock and RTC could be left in inconsistent states if two threads called SystemClock.setCurrentTimeMillis() simultaneously. By moving this code into AlarmManagerService, we can put a global lock around AlarmManagerService.setTime() and prevent the race condition. Note that access to SystemClock.setCurrentTimeMillis() is now gated by android.permission.SET_TIME, where before it was gated by filesystem permissions (i.e., could the process write to /dev/alarm or /dev/rtc). Change-Id: Ia34899a4cde983656305fd2ef466dfe908ed23c8 Signed-off-by: Greg Hackmann --- core/java/android/app/IAlarmManager.aidl | 2 +- core/java/android/os/SystemClock.java | 23 +++- core/jni/android_os_SystemClock.cpp | 112 ------------------ .../android/server/AlarmManagerService.java | 12 +- ...com_android_server_AlarmManagerService.cpp | 86 ++++++++++++++ 5 files changed, 119 insertions(+), 116 deletions(-) diff --git a/core/java/android/app/IAlarmManager.aidl b/core/java/android/app/IAlarmManager.aidl index 8476609c83b97..ef9f26eeb6d4f 100644 --- a/core/java/android/app/IAlarmManager.aidl +++ b/core/java/android/app/IAlarmManager.aidl @@ -28,7 +28,7 @@ interface IAlarmManager { /** windowLength == 0 means exact; windowLength < 0 means the let the OS decide */ void set(int type, long triggerAtTime, long windowLength, long interval, in PendingIntent operation, in WorkSource workSource); - void setTime(long millis); + boolean setTime(long millis); void setTimeZone(String zone); void remove(in PendingIntent operation); } diff --git a/core/java/android/os/SystemClock.java b/core/java/android/os/SystemClock.java index 729c64bfad0c7..672df6d4da818 100644 --- a/core/java/android/os/SystemClock.java +++ b/core/java/android/os/SystemClock.java @@ -16,6 +16,9 @@ package android.os; +import android.app.IAlarmManager; +import android.content.Context; +import android.util.Slog; /** * Core timekeeping facilities. @@ -89,6 +92,8 @@ package android.os; * */ public final class SystemClock { + private static final String TAG = "SystemClock"; + /** * This class is uninstantiable. */ @@ -134,7 +139,23 @@ public final class SystemClock { * * @return if the clock was successfully set to the specified time. */ - native public static boolean setCurrentTimeMillis(long millis); + public static boolean setCurrentTimeMillis(long millis) { + IBinder b = ServiceManager.getService(Context.ALARM_SERVICE); + IAlarmManager mgr = IAlarmManager.Stub.asInterface(b); + if (mgr == null) { + return false; + } + + try { + return mgr.setTime(millis); + } catch (RemoteException e) { + Slog.e(TAG, "Unable to set RTC", e); + } catch (SecurityException e) { + Slog.e(TAG, "Unable to set RTC", e); + } + + return false; + } /** * Returns milliseconds since boot, not counting time spent in deep sleep. diff --git a/core/jni/android_os_SystemClock.cpp b/core/jni/android_os_SystemClock.cpp index 5f4d57080c40c..624784420aaf4 100644 --- a/core/jni/android_os_SystemClock.cpp +++ b/core/jni/android_os_SystemClock.cpp @@ -19,13 +19,6 @@ * System clock functions. */ -#ifdef HAVE_ANDROID_OS -#include -#include -#include -#include -#endif - #include #include #include @@ -43,109 +36,6 @@ namespace android { -static int setCurrentTimeMillisAlarmDriver(struct timeval *tv) -{ - struct timespec ts; - int fd; - int res; - - fd = open("/dev/alarm", O_RDWR); - if(fd < 0) { - ALOGV("Unable to open alarm driver: %s\n", strerror(errno)); - return -1; - } - ts.tv_sec = tv->tv_sec; - ts.tv_nsec = tv->tv_usec * 1000; - res = ioctl(fd, ANDROID_ALARM_SET_RTC, &ts); - if (res < 0) - ALOGV("ANDROID_ALARM_SET_RTC ioctl failed: %s\n", strerror(errno)); - close(fd); - return res; -} - -static int setCurrentTimeMillisRtc(struct timeval *tv) -{ - struct rtc_time rtc; - struct tm tm, *gmtime_res; - int fd; - int res; - - fd = open("/dev/rtc0", O_RDWR); - if (fd < 0) { - ALOGV("Unable to open RTC driver: %s\n", strerror(errno)); - return -1; - } - - res = settimeofday(tv, NULL); - if (res < 0) { - ALOGV("settimeofday() failed: %s\n", strerror(errno)); - goto done; - } - - gmtime_res = gmtime_r(&tv->tv_sec, &tm); - if (!gmtime_res) { - ALOGV("gmtime_r() failed: %s\n", strerror(errno)); - res = -1; - goto done; - } - - memset(&rtc, 0, sizeof(rtc)); - rtc.tm_sec = tm.tm_sec; - rtc.tm_min = tm.tm_min; - rtc.tm_hour = tm.tm_hour; - rtc.tm_mday = tm.tm_mday; - rtc.tm_mon = tm.tm_mon; - rtc.tm_year = tm.tm_year; - rtc.tm_wday = tm.tm_wday; - rtc.tm_yday = tm.tm_yday; - rtc.tm_isdst = tm.tm_isdst; - res = ioctl(fd, RTC_SET_TIME, &rtc); - if (res < 0) - ALOGV("RTC_SET_TIME ioctl failed: %s\n", strerror(errno)); -done: - close(fd); - return res; -} - -/* - * Set the current time. This only works when running as root. - */ -static int setCurrentTimeMillis(int64_t millis) -{ - struct timeval tv; - int ret; - - if (millis <= 0 || millis / 1000LL >= INT_MAX) { - return -1; - } - - tv.tv_sec = (time_t) (millis / 1000LL); - tv.tv_usec = (suseconds_t) ((millis % 1000LL) * 1000LL); - - ALOGD("Setting time of day to sec=%d\n", (int) tv.tv_sec); - - ret = setCurrentTimeMillisAlarmDriver(&tv); - if (ret < 0) - ret = setCurrentTimeMillisRtc(&tv); - - if(ret < 0) { - ALOGW("Unable to set rtc to %ld: %s\n", tv.tv_sec, strerror(errno)); - ret = -1; - } - return ret; -} - -/* - * native public static void setCurrentTimeMillis(long millis) - * - * Set the current time. This only works when running as root. - */ -static jboolean android_os_SystemClock_setCurrentTimeMillis(JNIEnv* env, - jobject clazz, jlong millis) -{ - return (setCurrentTimeMillis(millis) == 0); -} - /* * native public static long uptimeMillis(); */ @@ -230,8 +120,6 @@ static jlong android_os_SystemClock_elapsedRealtimeNano(JNIEnv* env, */ static JNINativeMethod gMethods[] = { /* name, signature, funcPtr */ - { "setCurrentTimeMillis", "(J)Z", - (void*) android_os_SystemClock_setCurrentTimeMillis }, { "uptimeMillis", "()J", (void*) android_os_SystemClock_uptimeMillis }, { "elapsedRealtime", "()J", diff --git a/services/java/com/android/server/AlarmManagerService.java b/services/java/com/android/server/AlarmManagerService.java index 2e1b0af7b099b..defc6eff50275 100644 --- a/services/java/com/android/server/AlarmManagerService.java +++ b/services/java/com/android/server/AlarmManagerService.java @@ -669,12 +669,19 @@ class AlarmManagerService extends IAlarmManager.Stub { } } - public void setTime(long millis) { + public boolean setTime(long millis) { mContext.enforceCallingOrSelfPermission( "android.permission.SET_TIME", "setTime"); - SystemClock.setCurrentTimeMillis(millis); + if (mNativeData == 0) { + Slog.w(TAG, "Not setting time since no alarm driver is available."); + return false; + } + + synchronized (mLock) { + return setKernelTime(mNativeData, millis) == 0; + } } public void setTimeZone(String tz) { @@ -1018,6 +1025,7 @@ class AlarmManagerService extends IAlarmManager.Stub { private native void close(long nativeData); private native void set(long nativeData, int type, long seconds, long nanoseconds); private native int waitForAlarm(long nativeData); + private native int setKernelTime(long nativeData, long millis); private native int setKernelTimezone(long nativeData, int minuteswest); private void triggerAlarmsLocked(ArrayList triggerList, long nowELAPSED, long nowRTC) { diff --git a/services/jni/com_android_server_AlarmManagerService.cpp b/services/jni/com_android_server_AlarmManagerService.cpp index 342515bb3bb02..bcb477eb77a7c 100644 --- a/services/jni/com_android_server_AlarmManagerService.cpp +++ b/services/jni/com_android_server_AlarmManagerService.cpp @@ -36,6 +36,7 @@ #include #include #include +#include namespace android { @@ -58,6 +59,7 @@ public: virtual ~AlarmImpl(); virtual int set(int type, struct timespec *ts) = 0; + virtual int setTime(struct timeval *tv) = 0; virtual int waitForAlarm() = 0; protected: @@ -71,6 +73,7 @@ public: AlarmImplAlarmDriver(int fd) : AlarmImpl(&fd, 1) { } int set(int type, struct timespec *ts); + int setTime(struct timeval *tv); int waitForAlarm(); }; @@ -82,6 +85,7 @@ public: ~AlarmImplTimerFd(); int set(int type, struct timespec *ts); + int setTime(struct timeval *tv); int waitForAlarm(); private: @@ -107,6 +111,19 @@ int AlarmImplAlarmDriver::set(int type, struct timespec *ts) return ioctl(fds[0], ANDROID_ALARM_SET(type), ts); } +int AlarmImplAlarmDriver::setTime(struct timeval *tv) +{ + struct timespec ts; + int res; + + ts.tv_sec = tv->tv_sec; + ts.tv_nsec = tv->tv_usec * 1000; + res = ioctl(fds[0], ANDROID_ALARM_SET_RTC, &ts); + if (res < 0) + ALOGV("ANDROID_ALARM_SET_RTC ioctl failed: %s\n", strerror(errno)); + return res; +} + int AlarmImplAlarmDriver::waitForAlarm() { return ioctl(fds[0], ANDROID_ALARM_WAIT); @@ -140,6 +157,50 @@ int AlarmImplTimerFd::set(int type, struct timespec *ts) return timerfd_settime(fds[type], TFD_TIMER_ABSTIME, &spec, NULL); } +int AlarmImplTimerFd::setTime(struct timeval *tv) +{ + struct rtc_time rtc; + struct tm tm, *gmtime_res; + int fd; + int res; + + fd = open("/dev/rtc0", O_RDWR); + if (fd < 0) { + ALOGV("Unable to open RTC driver: %s\n", strerror(errno)); + return -1; + } + + res = settimeofday(tv, NULL); + if (res < 0) { + ALOGV("settimeofday() failed: %s\n", strerror(errno)); + goto done; + } + + gmtime_res = gmtime_r(&tv->tv_sec, &tm); + if (!gmtime_res) { + ALOGV("gmtime_r() failed: %s\n", strerror(errno)); + res = -1; + goto done; + } + + memset(&rtc, 0, sizeof(rtc)); + rtc.tm_sec = tm.tm_sec; + rtc.tm_min = tm.tm_min; + rtc.tm_hour = tm.tm_hour; + rtc.tm_mday = tm.tm_mday; + rtc.tm_mon = tm.tm_mon; + rtc.tm_year = tm.tm_year; + rtc.tm_wday = tm.tm_wday; + rtc.tm_yday = tm.tm_yday; + rtc.tm_isdst = tm.tm_isdst; + res = ioctl(fd, RTC_SET_TIME, &rtc); + if (res < 0) + ALOGV("RTC_SET_TIME ioctl failed: %s\n", strerror(errno)); +done: + close(fd); + return res; +} + int AlarmImplTimerFd::waitForAlarm() { epoll_event events[N_ANDROID_TIMERFDS]; @@ -168,6 +229,30 @@ int AlarmImplTimerFd::waitForAlarm() return result; } +static jint android_server_AlarmManagerService_setKernelTime(JNIEnv*, jobject, jlong nativeData, jlong millis) +{ + AlarmImpl *impl = reinterpret_cast(nativeData); + struct timeval tv; + int ret; + + if (millis <= 0 || millis / 1000LL >= INT_MAX) { + return -1; + } + + tv.tv_sec = (time_t) (millis / 1000LL); + tv.tv_usec = (suseconds_t) ((millis % 1000LL) * 1000LL); + + ALOGD("Setting time of day to sec=%d\n", (int) tv.tv_sec); + + ret = impl->setTime(&tv); + + if(ret < 0) { + ALOGW("Unable to set rtc to %ld: %s\n", tv.tv_sec, strerror(errno)); + ret = -1; + } + return ret; +} + static jint android_server_AlarmManagerService_setKernelTimezone(JNIEnv*, jobject, jlong, jint minswest) { struct timezone tz; @@ -309,6 +394,7 @@ static JNINativeMethod sMethods[] = { {"close", "(J)V", (void*)android_server_AlarmManagerService_close}, {"set", "(JIJJ)V", (void*)android_server_AlarmManagerService_set}, {"waitForAlarm", "(J)I", (void*)android_server_AlarmManagerService_waitForAlarm}, + {"setKernelTime", "(JJ)I", (void*)android_server_AlarmManagerService_setKernelTime}, {"setKernelTimezone", "(JI)I", (void*)android_server_AlarmManagerService_setKernelTimezone}, }; From c9244720da829cac94918555761468ebdd2b7c5d Mon Sep 17 00:00:00 2001 From: Greg Hackmann Date: Mon, 27 Jan 2014 16:30:09 -0800 Subject: [PATCH 2/2] open("/dev/rtc0") failure in AlarmManagerService.setTime() should be non-fatal Setting the time-of-day clock is still useful on systems where the RTC device is not yet brought up or otherwise unavailable. This matches the in-kernel behavior of the Android alarm driver. Change-Id: I6d4fdadab12e241ada7419425efd55bd13873c55 Signed-off-by: Greg Hackmann --- .../jni/com_android_server_AlarmManagerService.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/services/jni/com_android_server_AlarmManagerService.cpp b/services/jni/com_android_server_AlarmManagerService.cpp index bcb477eb77a7c..c26a5164a5347 100644 --- a/services/jni/com_android_server_AlarmManagerService.cpp +++ b/services/jni/com_android_server_AlarmManagerService.cpp @@ -164,16 +164,16 @@ int AlarmImplTimerFd::setTime(struct timeval *tv) int fd; int res; - fd = open("/dev/rtc0", O_RDWR); - if (fd < 0) { - ALOGV("Unable to open RTC driver: %s\n", strerror(errno)); - return -1; - } - res = settimeofday(tv, NULL); if (res < 0) { ALOGV("settimeofday() failed: %s\n", strerror(errno)); - goto done; + return -1; + } + + fd = open("/dev/rtc0", O_RDWR); + if (fd < 0) { + ALOGV("Unable to open RTC driver: %s\n", strerror(errno)); + return res; } gmtime_res = gmtime_r(&tv->tv_sec, &tm);