diff options
author | Ytai Ben-Tsvi <ytai@google.com> | 2020-02-04 15:02:32 -0800 |
---|---|---|
committer | Ytai Ben-tsvi <ytai@google.com> | 2020-02-12 17:59:37 +0000 |
commit | 166a84bfe31457fe44dcb90294d33089d2a3f428 (patch) | |
tree | e232f73acaecaaa3a6668a0813d914f7a90c5640 | |
parent | 20758384397836d1e13bac08da571e07964c088a (diff) | |
download | frameworks_av-166a84bfe31457fe44dcb90294d33089d2a3f428.tar.gz frameworks_av-166a84bfe31457fe44dcb90294d33089d2a3f428.tar.bz2 frameworks_av-166a84bfe31457fe44dcb90294d33089d2a3f428.zip |
[DO NOT MERGE] Crash the process when midi extraction times out
This is intended to recover from cases when the MIDI extractor gets
stuck in an infinite (or very long) loop.
The extractor code is 3rd party and hard to harden - this gives us
at least an eventual recovery.
Change-Id: I8d2fd85f91d1d2749c5620cf8518583076d2f6d7
Bug: 127313764
Bug: 127313537
Bug: 127313223
Bug: 127312550
Bug: 127310810
Bug: 126380818
Bug: 123700383
Merged-In: I8d2fd85f91d1d2749c5620cf8518583076d2f6d7
-rw-r--r-- | media/extractors/midi/Android.bp | 5 | ||||
-rw-r--r-- | media/extractors/midi/MidiExtractor.cpp | 12 | ||||
-rw-r--r-- | media/libwatchdog/Android.bp | 35 | ||||
-rw-r--r-- | media/libwatchdog/Watchdog.cpp | 63 | ||||
-rw-r--r-- | media/libwatchdog/include/watchdog/Watchdog.h | 49 | ||||
-rw-r--r-- | services/mediaextractor/seccomp_policy/mediaextractor-arm.policy | 3 | ||||
-rw-r--r-- | services/mediaextractor/seccomp_policy/mediaextractor-arm64.policy | 3 | ||||
-rw-r--r-- | services/mediaextractor/seccomp_policy/mediaextractor-x86.policy | 3 | ||||
-rw-r--r-- | services/mediaextractor/seccomp_policy/mediaextractor-x86_64.policy | 3 |
9 files changed, 174 insertions, 2 deletions
diff --git a/media/extractors/midi/Android.bp b/media/extractors/midi/Android.bp index 7d42e703f3..91ce78eb9d 100644 --- a/media/extractors/midi/Android.bp +++ b/media/extractors/midi/Android.bp @@ -14,7 +14,9 @@ cc_library_shared { static_libs: [ "libmedia_midiiowrapper", "libsonivox", - "libstagefright_foundation" + "libstagefright_foundation", + "libwatchdog", + "libbase", ], name: "libmidiextractor", relative_install_path: "extractors", @@ -35,5 +37,4 @@ cc_library_shared { "signed-integer-overflow", ], }, - } diff --git a/media/extractors/midi/MidiExtractor.cpp b/media/extractors/midi/MidiExtractor.cpp index 9f4f9e69b4..d0efb2f652 100644 --- a/media/extractors/midi/MidiExtractor.cpp +++ b/media/extractors/midi/MidiExtractor.cpp @@ -26,6 +26,7 @@ #include <media/stagefright/MediaDefs.h> #include <media/stagefright/MediaErrors.h> #include <libsonivox/eas_reverb.h> +#include <watchdog/Watchdog.h> namespace android { @@ -116,6 +117,7 @@ media_status_t MidiSource::read( MediaBufferHelper **outBuffer, const ReadOptions *options) { ALOGV("MidiSource::read"); + MediaBufferHelper *buffer; // process an optional seek request int64_t seekTimeUs; @@ -139,6 +141,8 @@ status_t MidiSource::init() } // MidiEngine +using namespace std::chrono_literals; +static constexpr auto kTimeout = 10s; MidiEngine::MidiEngine(CDataSource *dataSource, AMediaFormat *fileMetadata, @@ -147,6 +151,8 @@ MidiEngine::MidiEngine(CDataSource *dataSource, mEasHandle(NULL), mEasConfig(NULL), mIsInitialized(false) { + Watchdog watchdog(kTimeout); + mIoWrapper = new MidiIoWrapper(dataSource); // spin up a new EAS engine EAS_I32 temp; @@ -186,6 +192,8 @@ MidiEngine::MidiEngine(CDataSource *dataSource, } MidiEngine::~MidiEngine() { + Watchdog watchdog(kTimeout); + if (mEasHandle) { EAS_CloseFile(mEasData, mEasHandle); } @@ -217,12 +225,16 @@ status_t MidiEngine::releaseBuffers() { } status_t MidiEngine::seekTo(int64_t positionUs) { + Watchdog watchdog(kTimeout); + ALOGV("seekTo %lld", (long long)positionUs); EAS_RESULT result = EAS_Locate(mEasData, mEasHandle, positionUs / 1000, false); return result == EAS_SUCCESS ? OK : UNKNOWN_ERROR; } MediaBufferHelper* MidiEngine::readBuffer() { + Watchdog watchdog(kTimeout); + EAS_STATE state; EAS_State(mEasData, mEasHandle, &state); if ((state == EAS_STATE_STOPPED) || (state == EAS_STATE_ERROR)) { diff --git a/media/libwatchdog/Android.bp b/media/libwatchdog/Android.bp new file mode 100644 index 0000000000..2bdf17262c --- /dev/null +++ b/media/libwatchdog/Android.bp @@ -0,0 +1,35 @@ +// Copyright 2020 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +cc_library { + name: "libwatchdog", + srcs: [ + "Watchdog.cpp", + ], + export_include_dirs: ["include"], + shared_libs: [ + "liblog", + ], + static_libs: [ + "libbase", + ], + target: { + windows: { + enabled: false, + }, + darwin: { + enabled: false, + }, + }, +} diff --git a/media/libwatchdog/Watchdog.cpp b/media/libwatchdog/Watchdog.cpp new file mode 100644 index 0000000000..bb012b9429 --- /dev/null +++ b/media/libwatchdog/Watchdog.cpp @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "Watchdog" + +#include <watchdog/Watchdog.h> + +#include <android-base/logging.h> +#include <android-base/threads.h> +#include <signal.h> +#include <time.h> +#include <cstring> +#include <utils/Log.h> + +namespace android { + +Watchdog::Watchdog(::std::chrono::steady_clock::duration timeout) { + // Create the timer. + struct sigevent sev; + sev.sigev_notify = SIGEV_THREAD_ID; + sev.sigev_notify_thread_id = base::GetThreadId(); + sev.sigev_signo = SIGABRT; + sev.sigev_value.sival_ptr = &mTimerId; + int err = timer_create(CLOCK_MONOTONIC, &sev, &mTimerId); + if (err != 0) { + PLOG(FATAL) << "Failed to create timer"; + } + + // Start the timer. + struct itimerspec spec; + memset(&spec, 0, sizeof(spec)); + auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(timeout); + LOG_ALWAYS_FATAL_IF(timeout.count() <= 0, "Duration must be positive"); + spec.it_value.tv_sec = ns.count() / 1000000000; + spec.it_value.tv_nsec = ns.count() % 1000000000; + err = timer_settime(mTimerId, 0, &spec, nullptr); + if (err != 0) { + PLOG(FATAL) << "Failed to start timer"; + } +} + +Watchdog::~Watchdog() { + // Delete the timer. + int err = timer_delete(mTimerId); + if (err != 0) { + PLOG(FATAL) << "Failed to delete timer"; + } +} + +} // namespace android diff --git a/media/libwatchdog/include/watchdog/Watchdog.h b/media/libwatchdog/include/watchdog/Watchdog.h new file mode 100644 index 0000000000..2819f8a270 --- /dev/null +++ b/media/libwatchdog/include/watchdog/Watchdog.h @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_WATCHDOG_H +#define ANDROID_WATCHDOG_H + +#include <chrono> +#include <time.h> + +namespace android { + +/* + * An RAII-style object, which would crash the process if a timeout expires + * before the object is destroyed. + * The calling thread would be sent a SIGABORT, which would typically result in + * a stack trace. + * + * Sample usage: + * { + * Watchdog watchdog(std::chrono::milliseconds(10)); + * DoSomething(); + * } + * // If we got here, the function completed in time. + */ +class Watchdog final { +public: + Watchdog(std::chrono::steady_clock::duration timeout); + ~Watchdog(); + +private: + timer_t mTimerId; +}; + +} // namespace android + +#endif // ANDROID_WATCHDOG_H diff --git a/services/mediaextractor/seccomp_policy/mediaextractor-arm.policy b/services/mediaextractor/seccomp_policy/mediaextractor-arm.policy index 38f9be6c7a..118072ec65 100644 --- a/services/mediaextractor/seccomp_policy/mediaextractor-arm.policy +++ b/services/mediaextractor/seccomp_policy/mediaextractor-arm.policy @@ -41,6 +41,9 @@ getegid32: 1 getgroups32: 1 nanosleep: 1 getrandom: 1 +timer_create: 1 +timer_settime: 1 +timer_delete: 1 # for dynamically loading extractors pread64: 1 diff --git a/services/mediaextractor/seccomp_policy/mediaextractor-arm64.policy b/services/mediaextractor/seccomp_policy/mediaextractor-arm64.policy index 8fd8787419..481e29e253 100644 --- a/services/mediaextractor/seccomp_policy/mediaextractor-arm64.policy +++ b/services/mediaextractor/seccomp_policy/mediaextractor-arm64.policy @@ -30,6 +30,9 @@ rt_sigreturn: 1 getrlimit: 1 nanosleep: 1 getrandom: 1 +timer_create: 1 +timer_settime: 1 +timer_delete: 1 # for FileSource readlinkat: 1 diff --git a/services/mediaextractor/seccomp_policy/mediaextractor-x86.policy b/services/mediaextractor/seccomp_policy/mediaextractor-x86.policy index 05915d11d9..15fb24e5ea 100644 --- a/services/mediaextractor/seccomp_policy/mediaextractor-x86.policy +++ b/services/mediaextractor/seccomp_policy/mediaextractor-x86.policy @@ -39,6 +39,9 @@ getegid32: 1 getgroups32: 1 nanosleep: 1 getrandom: 1 +timer_create: 1 +timer_settime: 1 +timer_delete: 1 # for dynamically loading extractors getdents64: 1 diff --git a/services/mediaextractor/seccomp_policy/mediaextractor-x86_64.policy b/services/mediaextractor/seccomp_policy/mediaextractor-x86_64.policy index e6a55d0586..4f2646c9cd 100644 --- a/services/mediaextractor/seccomp_policy/mediaextractor-x86_64.policy +++ b/services/mediaextractor/seccomp_policy/mediaextractor-x86_64.policy @@ -34,6 +34,9 @@ sched_setscheduler: 1 getrlimit: 1 nanosleep: 1 getrandom: 1 +timer_create: 1 +timer_settime: 1 +timer_delete: 1 # for dynamically loading extractors getdents64: 1 |