Merge "Camera: Fix possible invalid metadata access" into rvc-dev
diff --git a/media/codec2/components/avc/C2SoftAvcDec.cpp b/media/codec2/components/avc/C2SoftAvcDec.cpp
index 56813c4..d686eb1 100644
--- a/media/codec2/components/avc/C2SoftAvcDec.cpp
+++ b/media/codec2/components/avc/C2SoftAvcDec.cpp
@@ -35,6 +35,7 @@
constexpr char COMPONENT_NAME[] = "c2.android.avc.decoder";
constexpr uint32_t kDefaultOutputDelay = 8;
constexpr uint32_t kMaxOutputDelay = 16;
+constexpr uint32_t kMinInputBytes = 4;
} // namespace
class C2SoftAvcDec::IntfImpl : public SimpleInterface<void>::BaseParams {
@@ -817,7 +818,7 @@
inSize, (int)work->input.ordinal.timestamp.peeku(),
(int)work->input.ordinal.frameIndex.peeku(), work->input.flags);
size_t inPos = 0;
- while (inPos < inSize) {
+ while (inPos < inSize && inSize - inPos >= kMinInputBytes) {
if (C2_OK != ensureDecoderState(pool)) {
mSignalledError = true;
work->workletsProcessed = 1u;
@@ -904,7 +905,6 @@
work->result = C2_CORRUPTED;
return;
}
- continue;
}
if (0 < s_decode_op.u4_pic_wd && 0 < s_decode_op.u4_pic_ht) {
if (mHeaderDecoded == false) {
@@ -937,16 +937,7 @@
if (s_decode_op.u4_output_present) {
finishWork(s_decode_op.u4_ts, work);
}
- if (0 == s_decode_op.u4_num_bytes_consumed) {
- ALOGD("Bytes consumed is zero. Ignoring remaining bytes");
- break;
- }
inPos += s_decode_op.u4_num_bytes_consumed;
- if (hasPicture && (inSize - inPos)) {
- ALOGD("decoded frame in current access nal, ignoring further trailing bytes %d",
- (int)inSize - (int)inPos);
- break;
- }
}
if (eos) {
drainInternal(DRAIN_COMPONENT_WITH_EOS, pool, work);
diff --git a/media/extractors/mp4/MPEG4Extractor.cpp b/media/extractors/mp4/MPEG4Extractor.cpp
index b72a8cc..aea28c0 100755
--- a/media/extractors/mp4/MPEG4Extractor.cpp
+++ b/media/extractors/mp4/MPEG4Extractor.cpp
@@ -2896,7 +2896,7 @@
return ERROR_MALFORMED;
}
- parseID3v2MetaData(data_offset + 6);
+ parseID3v2MetaData(data_offset + 6, chunk_data_size - 6);
break;
}
@@ -4167,8 +4167,19 @@
return OK;
}
-void MPEG4Extractor::parseID3v2MetaData(off64_t offset) {
- ID3 id3(mDataSource, true /* ignorev1 */, offset);
+void MPEG4Extractor::parseID3v2MetaData(off64_t offset, uint64_t size) {
+ uint8_t *buffer = new (std::nothrow) uint8_t[size];
+ if (buffer == NULL) {
+ return;
+ }
+ if (mDataSource->readAt(offset, buffer, size) != (ssize_t)size) {
+ delete[] buffer;
+ buffer = NULL;
+ return;
+ }
+
+ ID3 id3(buffer, size, true /* ignorev1 */);
+ delete[] buffer;
if (id3.isValid()) {
struct Map {
diff --git a/media/extractors/mp4/MPEG4Extractor.h b/media/extractors/mp4/MPEG4Extractor.h
index 3af432d..1e49d50 100644
--- a/media/extractors/mp4/MPEG4Extractor.h
+++ b/media/extractors/mp4/MPEG4Extractor.h
@@ -161,7 +161,7 @@
status_t parseITunesMetaData(off64_t offset, size_t size);
status_t parseColorInfo(off64_t offset, size_t size);
status_t parse3GPPMetaData(off64_t offset, size_t size, int depth);
- void parseID3v2MetaData(off64_t offset);
+ void parseID3v2MetaData(off64_t offset, uint64_t size);
status_t parseQTMetaKey(off64_t data_offset, size_t data_size);
status_t parseQTMetaVal(int32_t keyId, off64_t data_offset, size_t data_size);
diff --git a/media/libmediametrics/include/media/MediaMetricsItem.h b/media/libmediametrics/include/media/MediaMetricsItem.h
index 08720f1..591e714 100644
--- a/media/libmediametrics/include/media/MediaMetricsItem.h
+++ b/media/libmediametrics/include/media/MediaMetricsItem.h
@@ -208,11 +208,11 @@
template<size_t N>
static inline bool startsWith(const std::string &s, const char (&comp)[N]) {
- return !strncmp(s.c_str(), comp, N - 1);
+ return !strncmp(s.c_str(), comp, N - 1); // last char is null termination
}
static inline bool startsWith(const std::string& s, const std::string& comp) {
- return !strncmp(s.c_str(), comp.c_str(), comp.size() - 1);
+ return !strncmp(s.c_str(), comp.c_str(), comp.size());
}
/**
diff --git a/media/libstagefright/timedtext/TextDescriptions.cpp b/media/libstagefright/timedtext/TextDescriptions.cpp
index 0dc7722..6c94754 100644
--- a/media/libstagefright/timedtext/TextDescriptions.cpp
+++ b/media/libstagefright/timedtext/TextDescriptions.cpp
@@ -445,51 +445,75 @@
| *(tmpData + 10) << 8 | *(tmpData + 11);
parcel->writeInt32(rgba);
+ // tx3g box contains class FontTableBox() which extends ftab box
+ // This information is part of the 3gpp Timed Text Format
+ // Specification#: 26.245 / Section: 5.16(Sample Description Format)
+ // https://www.3gpp.org/ftp/Specs/archive/26_series/26.245/
+
tmpData += 12;
remaining -= 12;
- if (remaining < 2) {
+ if (remaining < 8) {
return OK;
}
- size_t dataPos = parcel->dataPosition();
-
- parcel->writeInt32(KEY_STRUCT_FONT_LIST);
- uint16_t count = U16_AT(tmpData);
- parcel->writeInt32(count);
-
- tmpData += 2;
- remaining -= 2;
-
- for (int i = 0; i < count; i++) {
- if (remaining < 3) {
- // roll back
- parcel->setDataPosition(dataPos);
- return OK;
- }
- // font ID
- parcel->writeInt32(U16_AT(tmpData));
-
- // font name length
- parcel->writeInt32(*(tmpData + 2));
-
- size_t len = *(tmpData + 2);
-
- tmpData += 3;
- remaining -= 3;
-
- if (remaining < len) {
- // roll back
- parcel->setDataPosition(dataPos);
- return OK;
- }
-
- parcel->write(tmpData, len);
- tmpData += len;
- remaining -= len;
+ size_t subChunkSize = U32_AT(tmpData);
+ if(remaining < subChunkSize) {
+ return OK;
}
- // there is a "DisparityBox" after this according to the spec, but we ignore it
+ uint32_t subChunkType = U32_AT(tmpData + 4);
+
+ if (subChunkType == FOURCC('f', 't', 'a', 'b'))
+ {
+ tmpData += 8;
+ size_t subChunkRemaining = subChunkSize - 8;
+
+ if(subChunkRemaining < 2) {
+ return OK;
+ }
+ size_t dataPos = parcel->dataPosition();
+
+ parcel->writeInt32(KEY_STRUCT_FONT_LIST);
+ uint16_t count = U16_AT(tmpData);
+ parcel->writeInt32(count);
+
+ tmpData += 2;
+ subChunkRemaining -= 2;
+
+ for (int i = 0; i < count; i++) {
+ if (subChunkRemaining < 3) {
+ // roll back
+ parcel->setDataPosition(dataPos);
+ return OK;
+ }
+ // font ID
+ parcel->writeInt32(U16_AT(tmpData));
+
+ // font name length
+ size_t len = *(tmpData + 2);
+
+ parcel->writeInt32(len);
+
+ tmpData += 3;
+ subChunkRemaining -=3;
+
+ if (subChunkRemaining < len) {
+ // roll back
+ parcel->setDataPosition(dataPos);
+ return OK;
+ }
+
+ parcel->write(tmpData, len);
+ tmpData += len;
+ subChunkRemaining -= len;
+ }
+ tmpData += subChunkRemaining;
+ remaining -= subChunkSize;
+ } else {
+ tmpData += subChunkSize;
+ remaining -= subChunkSize;
+ }
break;
}
default:
diff --git a/services/mediametrics/AnalyticsState.h b/services/mediametrics/AnalyticsState.h
index 290ed21..e8fedcf 100644
--- a/services/mediametrics/AnalyticsState.h
+++ b/services/mediametrics/AnalyticsState.h
@@ -84,8 +84,11 @@
* delivered.
*
* \param lines the maximum number of lines in the string returned.
+ * \param sinceNs the nanoseconds since Unix epoch to start dump (0 shows all)
+ * \param prefix the desired key prefix to match (nullptr shows all)
*/
- std::pair<std::string, int32_t> dump(int32_t lines = INT32_MAX) const {
+ std::pair<std::string, int32_t> dump(
+ int32_t lines = INT32_MAX, int64_t sinceNs = 0, const char *prefix = nullptr) const {
std::stringstream ss;
int32_t ll = lines;
@@ -94,8 +97,8 @@
--ll;
}
if (ll > 0) {
- auto [s, l] = mTransactionLog.dump(ll);
- ss << s;
+ auto [s, l] = mTransactionLog.dump(ll, sinceNs, prefix);
+ ss << std::move(s);
ll -= l;
}
if (ll > 0) {
@@ -103,8 +106,8 @@
--ll;
}
if (ll > 0) {
- auto [s, l] = mTimeMachine.dump(ll);
- ss << s;
+ auto [s, l] = mTimeMachine.dump(ll, sinceNs, prefix);
+ ss << std::move(s);
ll -= l;
}
return { ss.str(), lines - ll };
diff --git a/services/mediametrics/AudioAnalytics.cpp b/services/mediametrics/AudioAnalytics.cpp
index 126e501..f80516e 100644
--- a/services/mediametrics/AudioAnalytics.cpp
+++ b/services/mediametrics/AudioAnalytics.cpp
@@ -107,14 +107,15 @@
return NO_ERROR;
}
-std::pair<std::string, int32_t> AudioAnalytics::dump(int32_t lines) const
+std::pair<std::string, int32_t> AudioAnalytics::dump(
+ int32_t lines, int64_t sinceNs, const char *prefix) const
{
std::stringstream ss;
int32_t ll = lines;
if (ll > 0) {
- auto [s, l] = mAnalyticsState->dump(ll);
- ss << s;
+ auto [s, l] = mAnalyticsState->dump(ll, sinceNs, prefix);
+ ss << std::move(s);
ll -= l;
}
if (ll > 0) {
@@ -122,8 +123,8 @@
--ll;
}
if (ll > 0) {
- auto [s, l] = mPreviousAnalyticsState->dump(ll);
- ss << s;
+ auto [s, l] = mPreviousAnalyticsState->dump(ll, sinceNs, prefix);
+ ss << std::move(s);
ll -= l;
}
return { ss.str(), lines - ll };
diff --git a/services/mediametrics/AudioAnalytics.h b/services/mediametrics/AudioAnalytics.h
index 4a42e22..ba4c3f2 100644
--- a/services/mediametrics/AudioAnalytics.h
+++ b/services/mediametrics/AudioAnalytics.h
@@ -60,8 +60,17 @@
* delivered.
*
* \param lines the maximum number of lines in the string returned.
+ * \param sinceNs the nanoseconds since Unix epoch to start dump (0 shows all)
+ * \param prefix the desired key prefix to match (nullptr shows all)
*/
- std::pair<std::string, int32_t> dump(int32_t lines = INT32_MAX) const;
+ std::pair<std::string, int32_t> dump(
+ int32_t lines = INT32_MAX, int64_t sinceNs = 0, const char *prefix = nullptr) const;
+
+ void clear() {
+ // underlying state is locked.
+ mPreviousAnalyticsState->clear();
+ mAnalyticsState->clear();
+ }
private:
diff --git a/services/mediametrics/MediaMetricsService.cpp b/services/mediametrics/MediaMetricsService.cpp
index d76bc2c..b9703ba 100644
--- a/services/mediametrics/MediaMetricsService.cpp
+++ b/services/mediametrics/MediaMetricsService.cpp
@@ -207,69 +207,66 @@
return NO_ERROR;
}
+ static const String16 allOption("--all");
+ static const String16 clearOption("--clear");
static const String16 heapOption("--heap");
+ static const String16 helpOption("--help");
+ static const String16 prefixOption("--prefix");
+ static const String16 sinceOption("--since");
static const String16 unreachableOption("--unreachable");
+
+ bool all = false;
+ bool clear = false;
bool heap = false;
bool unreachable = false;
+ int64_t sinceNs = 0;
+ std::string prefix;
- const String16 protoOption("--proto");
- const String16 clearOption("--clear");
- bool clear = false;
- const String16 sinceOption("--since");
- nsecs_t ts_since = 0;
- const String16 helpOption("--help");
- const String16 onlyOption("--only");
- std::string only;
- const int n = args.size();
- for (int i = 0; i < n; i++) {
- if (args[i] == clearOption) {
+ const size_t n = args.size();
+ for (size_t i = 0; i < n; i++) {
+ if (args[i] == allOption) {
+ all = true;
+ } else if (args[i] == clearOption) {
clear = true;
} else if (args[i] == heapOption) {
heap = true;
- } else if (args[i] == protoOption) {
- i++;
- if (i < n) {
- // ignore
- } else {
- result.append("missing value for -proto\n\n");
- }
- } else if (args[i] == sinceOption) {
- i++;
- if (i < n) {
- String8 value(args[i]);
- char *endp;
- const char *p = value.string();
- ts_since = strtoll(p, &endp, 10);
- if (endp == p || *endp != '\0') {
- ts_since = 0;
- }
- } else {
- ts_since = 0;
- }
- // command line is milliseconds; internal units are nano-seconds
- ts_since *= NANOS_PER_MILLISECOND;
- } else if (args[i] == onlyOption) {
- i++;
- if (i < n) {
- String8 value(args[i]);
- only = value.string();
- }
} else if (args[i] == helpOption) {
// TODO: consider function area dumping.
// dumpsys media.metrics audiotrack,codec
// or dumpsys media.metrics audiotrack codec
result.append("Recognized parameters:\n");
- result.append("--heap heap usage (top 100)\n");
- result.append("--help this help message\n");
- result.append("--proto # dump using protocol #");
- result.append("--clear clears out saved records\n");
- result.append("--only X process records for component X\n");
- result.append("--since X include records since X\n");
- result.append(" (X is milliseconds since the UNIX epoch)\n");
- result.append("--unreachable unreachable memory (leaks)\n");
+ result.append("--all show all records\n");
+ result.append("--clear clear out saved records\n");
+ result.append("--heap show heap usage (top 100)\n");
+ result.append("--help display help\n");
+ result.append("--prefix X process records for component X\n");
+ result.append("--since X X < 0: records from -X seconds in the past\n");
+ result.append(" X = 0: ignore\n");
+ result.append(" X > 0: records from X seconds since Unix epoch\n");
+ result.append("--unreachable show unreachable memory (leaks)\n");
write(fd, result.string(), result.size());
return NO_ERROR;
+ } else if (args[i] == prefixOption) {
+ ++i;
+ if (i < n) {
+ prefix = String8(args[i]).string();
+ }
+ } else if (args[i] == sinceOption) {
+ ++i;
+ if (i < n) {
+ String8 value(args[i]);
+ char *endp;
+ const char *p = value.string();
+ long long sec = strtoll(p, &endp, 10);
+ if (endp == p || *endp != '\0' || sec == 0) {
+ sinceNs = 0;
+ } else if (sec < 0) {
+ sinceNs = systemTime(SYSTEM_TIME_REALTIME) + sec * NANOS_PER_SECOND;
+ } else {
+ sinceNs = sec * NANOS_PER_SECOND;
+ }
+ }
} else if (args[i] == unreachableOption) {
unreachable = true;
}
@@ -278,21 +275,23 @@
{
std::lock_guard _l(mLock);
- result.appendFormat("Dump of the %s process:\n", kServiceName);
- dumpHeaders_l(result, ts_since);
- dumpRecent_l(result, ts_since, only.c_str());
-
if (clear) {
mItemsDiscarded += mItems.size();
mItems.clear();
- // shall we clear the summary data too?
- }
- // TODO: maybe consider a better way of dumping audio analytics info.
- constexpr int32_t linesToDump = 1000;
- auto [ dumpString, lines ] = mAudioAnalytics.dump(linesToDump);
- result.append(dumpString.c_str());
- if (lines == linesToDump) {
- result.append("-- some lines may be truncated --\n");
+ mAudioAnalytics.clear();
+ } else {
+ result.appendFormat("Dump of the %s process:\n", kServiceName);
+ const char *prefixptr = prefix.size() > 0 ? prefix.c_str() : nullptr;
+ dumpHeaders_l(result, sinceNs, prefixptr);
+ dumpQueue_l(result, sinceNs, prefixptr);
+
+ // TODO: maybe consider a better way of dumping audio analytics info.
+ const int32_t linesToDump = all ? INT32_MAX : 1000;
+ auto [ dumpString, lines ] = mAudioAnalytics.dump(linesToDump, sinceNs, prefixptr);
+ result.append(dumpString.c_str());
+ if (lines == linesToDump) {
+ result.append("-- some lines may be truncated --\n");
+ }
}
}
write(fd, result.string(), result.size());
@@ -313,7 +312,7 @@
}
// dump headers
-void MediaMetricsService::dumpHeaders_l(String8 &result, nsecs_t ts_since)
+void MediaMetricsService::dumpHeaders_l(String8 &result, int64_t sinceNs, const char* prefix)
{
if (mediametrics::Item::isEnabled()) {
result.append("Metrics gathering: enabled\n");
@@ -327,54 +326,36 @@
"Records Discarded: %lld (by Count: %lld by Expiration: %lld)\n",
(long long)mItemsDiscarded, (long long)mItemsDiscardedCount,
(long long)mItemsDiscardedExpire);
- if (ts_since != 0) {
+ if (prefix != nullptr) {
+ result.appendFormat("Restricting to prefix %s", prefix);
+ }
+ if (sinceNs != 0) {
result.appendFormat(
"Emitting Queue entries more recent than: %lld\n",
- (long long)ts_since);
+ (long long)sinceNs);
}
}
-void MediaMetricsService::dumpRecent_l(
- String8 &result, nsecs_t ts_since, const char * only)
+// TODO: should prefix be a set<string>?
+void MediaMetricsService::dumpQueue_l(String8 &result, int64_t sinceNs, const char* prefix)
{
- if (only != nullptr && *only == '\0') {
- only = nullptr;
- }
- result.append("\nFinalized Metrics (oldest first):\n");
- dumpQueue_l(result, ts_since, only);
-
- // show who is connected and injecting records?
- // talk about # records fed to the 'readers'
- // talk about # records we discarded, perhaps "discarded w/o reading" too
-}
-
-void MediaMetricsService::dumpQueue_l(String8 &result) {
- dumpQueue_l(result, (nsecs_t) 0, nullptr /* only */);
-}
-
-void MediaMetricsService::dumpQueue_l(
- String8 &result, nsecs_t ts_since, const char * only) {
- int slot = 0;
-
if (mItems.empty()) {
result.append("empty\n");
- } else {
- for (const auto &item : mItems) {
- nsecs_t when = item->getTimestamp();
- if (when < ts_since) {
- continue;
- }
- // TODO: Only should be a set<string>
- if (only != nullptr &&
- item->getKey() /* std::string */ != only) {
- ALOGV("%s: omit '%s', it's not '%s'",
- __func__, item->getKey().c_str(), only);
- continue;
- }
- result.appendFormat("%5d: %s\n",
- slot, item->toString().c_str());
- slot++;
+ return;
+ }
+
+ int slot = 0;
+ for (const auto &item : mItems) { // TODO: consider std::lower_bound() on mItems
+ if (item->getTimestamp() < sinceNs) { // sinceNs == 0 means all items shown
+ continue;
}
+ if (prefix != nullptr && !startsWith(item->getKey(), prefix)) {
+ ALOGV("%s: omit '%s', it's not '%s'",
+ __func__, item->getKey().c_str(), prefix);
+ continue;
+ }
+ result.appendFormat("%5d: %s\n", slot, item->toString().c_str());
+ slot++;
}
}
diff --git a/services/mediametrics/MediaMetricsService.h b/services/mediametrics/MediaMetricsService.h
index 935bee2..a93f7fb 100644
--- a/services/mediametrics/MediaMetricsService.h
+++ b/services/mediametrics/MediaMetricsService.h
@@ -85,11 +85,8 @@
bool expirations_l(const std::shared_ptr<const mediametrics::Item>& item);
// support for generating output
- void dumpQueue_l(String8 &result);
- void dumpQueue_l(String8 &result, nsecs_t, const char *only);
- void dumpHeaders_l(String8 &result, nsecs_t ts_since);
- void dumpSummaries_l(String8 &result, nsecs_t ts_since, const char * only);
- void dumpRecent_l(String8 &result, nsecs_t ts_since, const char * only);
+ void dumpQueue_l(String8 &result, int64_t sinceNs, const char* prefix);
+ void dumpHeaders_l(String8 &result, int64_t sinceNs, const char* prefix);
// The following variables accessed without mLock
diff --git a/services/mediametrics/TimeMachine.h b/services/mediametrics/TimeMachine.h
index a4c3693..75819ba 100644
--- a/services/mediametrics/TimeMachine.h
+++ b/services/mediametrics/TimeMachine.h
@@ -149,8 +149,11 @@
int32_t ll = lines;
for (auto& tsPair : mPropertyMap) {
if (ll <= 0) break;
- ss << dump(mKey, tsPair, time);
- --ll;
+ std::string s = dump(mKey, tsPair, time);
+ if (s.size() > 0) {
+ --ll;
+ ss << std::move(s);
+ }
}
return { ss.str(), lines - ll };
}
@@ -165,7 +168,7 @@
const auto timeSequence = tsPair.second;
auto eptr = timeSequence.lower_bound(time);
if (eptr == timeSequence.end()) {
- return tsPair.first + "={};\n";
+ return {}; // don't dump anything. tsPair.first + "={};\n";
}
std::stringstream ss;
ss << key << "." << tsPair.first << "={";
@@ -407,25 +410,23 @@
*
* \param lines the maximum number of lines in the string returned.
* \param key selects only that key.
- * \param time to start the dump from.
+ * \param sinceNs the nanoseconds since Unix epoch to start dump (0 shows all)
+ * \param prefix the desired key prefix to match (nullptr shows all)
*/
std::pair<std::string, int32_t> dump(
- int32_t lines = INT32_MAX, const std::string &key = {}, int64_t time = 0) const {
+ int32_t lines = INT32_MAX, int64_t sinceNs = 0, const char *prefix = nullptr) const {
std::lock_guard lock(mLock);
- if (!key.empty()) { // use std::regex
- const auto it = mHistory.find(key);
- if (it == mHistory.end()) return {};
- std::lock_guard lock(getLockForKey(it->first));
- return it->second->dump(lines, time);
- }
-
std::stringstream ss;
int32_t ll = lines;
- for (const auto &[lkey, lhist] : mHistory) {
- std::lock_guard lock(getLockForKey(lkey));
- if (lines <= 0) break;
- auto [s, l] = lhist->dump(ll, time);
- ss << s;
+
+ for (auto it = prefix != nullptr ? mHistory.lower_bound(prefix) : mHistory.begin();
+ it != mHistory.end();
+ ++it) {
+ if (ll <= 0) break;
+ if (prefix != nullptr && !startsWith(it->first, prefix)) break;
+ std::lock_guard lock(getLockForKey(it->first));
+ auto [s, l] = it->second->dump(ll, sinceNs);
+ ss << std::move(s);
ll -= l;
}
return { ss.str(), lines - ll };
diff --git a/services/mediametrics/TransactionLog.h b/services/mediametrics/TransactionLog.h
index 190a99e..2359eec 100644
--- a/services/mediametrics/TransactionLog.h
+++ b/services/mediametrics/TransactionLog.h
@@ -127,8 +127,11 @@
* for subsequent line limiting.
*
* \param lines the maximum number of lines in the string returned.
+ * \param sinceNs the nanoseconds since Unix epoch to start dump (0 shows all)
+ * \param prefix the desired key prefix to match (nullptr shows all)
*/
- std::pair<std::string, int32_t> dump(int32_t lines) const {
+ std::pair<std::string, int32_t> dump(
+ int32_t lines, int64_t sinceNs, const char *prefix = nullptr) const {
std::stringstream ss;
int32_t ll = lines;
std::lock_guard lock(mLock);
@@ -138,26 +141,25 @@
ss << "Consolidated:\n";
--ll;
}
- for (const auto &log : mLog) {
- if (ll <= 0) break;
- ss << " " << log.second->toString() << "\n";
- --ll;
- }
+ auto [s, l] = dumpMapTimeItem(mLog, ll, sinceNs, prefix);
+ ss << std::move(s);
+ ll -= l;
// Grouped by item key (category)
if (ll > 0) {
ss << "Categorized:\n";
--ll;
}
- for (const auto &itemMap : mItemMap) {
+
+ for (auto it = prefix != nullptr ? mItemMap.lower_bound(prefix) : mItemMap.begin();
+ it != mItemMap.end();
+ ++it) {
if (ll <= 0) break;
- ss << " " << itemMap.first << "\n";
- --ll;
- for (const auto &item : itemMap.second) {
- if (ll <= 0) break;
- ss << " " << item.second->toString() << "\n";
- --ll;
- }
+ if (prefix != nullptr && !startsWith(it->first, prefix)) break;
+ auto [s, l] = dumpMapTimeItem(it->second, ll - 1, sinceNs, prefix);
+ if (l == 0) continue; // don't show empty groups (due to sinceNs).
+ ss << " " << it->first << "\n" << std::move(s);
+ ll -= l + 1;
}
return { ss.str(), lines - ll };
}
@@ -184,6 +186,24 @@
using MapTimeItem =
std::multimap<int64_t /* time */, std::shared_ptr<const mediametrics::Item>>;
+ static std::pair<std::string, int32_t> dumpMapTimeItem(
+ const MapTimeItem& mapTimeItem,
+ int32_t lines, int64_t sinceNs = 0, const char *prefix = nullptr) {
+ std::stringstream ss;
+ int32_t ll = lines;
+ // Note: for our data, mapTimeItem.lower_bound(0) == mapTimeItem.begin().
+ for (auto it = mapTimeItem.lower_bound(sinceNs);
+ it != mapTimeItem.end(); ++it) {
+ if (ll <= 0) break;
+ if (prefix != nullptr && !startsWith(it->second->getKey(), prefix)) {
+ continue;
+ }
+ ss << " " << it->second->toString() << "\n";
+ --ll;
+ }
+ return { ss.str(), lines - ll };
+ }
+
// GUARDED_BY mLock
/**
* Garbage collects if the TimeMachine size exceeds the high water mark.
diff --git a/services/minijail/Android.bp b/services/minijail/Android.bp
index 0713a87..5ea6d1e 100644
--- a/services/minijail/Android.bp
+++ b/services/minijail/Android.bp
@@ -39,4 +39,5 @@
srcs: [
"av_services_minijail_unittest.cpp",
],
+ test_suites: ["device-tests"],
}
diff --git a/services/minijail/TEST_MAPPING b/services/minijail/TEST_MAPPING
new file mode 100644
index 0000000..0d89760
--- /dev/null
+++ b/services/minijail/TEST_MAPPING
@@ -0,0 +1,5 @@
+{
+ "presubmit": [
+ { "name": "libavservices_minijail_unittest" }
+ ]
+}
diff --git a/services/minijail/av_services_minijail_unittest.cpp b/services/minijail/av_services_minijail_unittest.cpp
index 31313f8..896a764 100644
--- a/services/minijail/av_services_minijail_unittest.cpp
+++ b/services/minijail/av_services_minijail_unittest.cpp
@@ -34,13 +34,32 @@
"mmap: 1\n"
"munmap: 1\n";
+ const std::string third_policy_ =
+ "open: 1\n"
+ "close: 1\n";
+
const std::string full_policy_ = base_policy_ + std::string("\n") + additional_policy_;
+ const std::string triple_policy_ = base_policy_ +
+ std::string("\n") + additional_policy_ +
+ std::string("\n") + third_policy_;
};
TEST_F(WritePolicyTest, OneFile)
{
std::string final_string;
- android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, std::string()));
+ // vector with an empty pathname
+ android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, {std::string()}));
+ EXPECT_LE(0, fd.get());
+ bool success = android::base::ReadFdToString(fd.get(), &final_string);
+ EXPECT_TRUE(success);
+ EXPECT_EQ(final_string, base_policy_);
+}
+
+TEST_F(WritePolicyTest, OneFileAlternate)
+{
+ std::string final_string;
+ // empty vector
+ android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, {}));
EXPECT_LE(0, fd.get());
bool success = android::base::ReadFdToString(fd.get(), &final_string);
EXPECT_TRUE(success);
@@ -50,9 +69,19 @@
TEST_F(WritePolicyTest, TwoFiles)
{
std::string final_string;
- android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, additional_policy_));
+ android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, {additional_policy_}));
EXPECT_LE(0, fd.get());
bool success = android::base::ReadFdToString(fd.get(), &final_string);
EXPECT_TRUE(success);
EXPECT_EQ(final_string, full_policy_);
}
+
+TEST_F(WritePolicyTest, ThreeFiles)
+{
+ std::string final_string;
+ android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, {additional_policy_, third_policy_}));
+ EXPECT_LE(0, fd.get());
+ bool success = android::base::ReadFdToString(fd.get(), &final_string);
+ EXPECT_TRUE(success);
+ EXPECT_EQ(final_string, triple_policy_);
+}
diff --git a/services/minijail/minijail.cpp b/services/minijail/minijail.cpp
index f213287..f40f0c5 100644
--- a/services/minijail/minijail.cpp
+++ b/services/minijail/minijail.cpp
@@ -29,7 +29,7 @@
namespace android {
int WritePolicyToPipe(const std::string& base_policy_content,
- const std::string& additional_policy_content)
+ const std::vector<std::string>& additional_policy_contents)
{
int pipefd[2];
if (pipe(pipefd) == -1) {
@@ -40,9 +40,11 @@
base::unique_fd write_end(pipefd[1]);
std::string content = base_policy_content;
- if (additional_policy_content.length() > 0) {
- content += "\n";
- content += additional_policy_content;
+ for (auto one_content : additional_policy_contents) {
+ if (one_content.length() > 0) {
+ content += "\n";
+ content += one_content;
+ }
}
if (!base::WriteStringToFd(content, write_end.get())) {
@@ -53,29 +55,40 @@
return pipefd[0];
}
-void SetUpMinijail(const std::string& base_policy_path, const std::string& additional_policy_path)
+void SetUpMinijail(const std::string& base_policy_path,
+ const std::string& additional_policy_path)
+{
+ SetUpMinijailList(base_policy_path, {additional_policy_path});
+}
+
+void SetUpMinijailList(const std::string& base_policy_path,
+ const std::vector<std::string>& additional_policy_paths)
{
// No seccomp policy defined for this architecture.
if (access(base_policy_path.c_str(), R_OK) == -1) {
- LOG(WARNING) << "No seccomp policy defined for this architecture.";
+ // LOG(WARNING) << "No seccomp policy defined for this architecture.";
+ LOG(WARNING) << "missing base seccomp_policy file '" << base_policy_path << "'";
return;
}
std::string base_policy_content;
- std::string additional_policy_content;
+ std::vector<std::string> additional_policy_contents;
if (!base::ReadFileToString(base_policy_path, &base_policy_content,
false /* follow_symlinks */)) {
LOG(FATAL) << "Could not read base policy file '" << base_policy_path << "'";
}
- if (additional_policy_path.length() > 0 &&
- !base::ReadFileToString(additional_policy_path, &additional_policy_content,
- false /* follow_symlinks */)) {
- LOG(WARNING) << "Could not read additional policy file '" << additional_policy_path << "'";
- additional_policy_content = std::string();
+ for (auto one_policy_path : additional_policy_paths) {
+ std::string one_policy_content;
+ if (one_policy_path.length() > 0 &&
+ !base::ReadFileToString(one_policy_path, &one_policy_content,
+ false /* follow_symlinks */)) {
+ LOG(WARNING) << "Could not read additional policy file '" << one_policy_path << "'";
+ }
+ additional_policy_contents.push_back(one_policy_content);
}
- base::unique_fd policy_fd(WritePolicyToPipe(base_policy_content, additional_policy_content));
+ base::unique_fd policy_fd(WritePolicyToPipe(base_policy_content, additional_policy_contents));
if (policy_fd.get() == -1) {
LOG(FATAL) << "Could not write seccomp policy to fd";
}
diff --git a/services/minijail/minijail.h b/services/minijail/minijail.h
index c8a2149..298af86 100644
--- a/services/minijail/minijail.h
+++ b/services/minijail/minijail.h
@@ -16,11 +16,15 @@
#define AV_SERVICES_MINIJAIL_MINIJAIL
#include <string>
+#include <vector>
namespace android {
int WritePolicyToPipe(const std::string& base_policy_content,
- const std::string& additional_policy_content);
-void SetUpMinijail(const std::string& base_policy_path, const std::string& additional_policy_path);
+ const std::vector<std::string>& additional_policy_contents);
+void SetUpMinijail(const std::string& base_policy_path,
+ const std::string& additional_policy_path);
+void SetUpMinijailList(const std::string& base_policy_path,
+ const std::vector<std::string>& additional_policy_paths);
}
#endif // AV_SERVICES_MINIJAIL_MINIJAIL