Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 385c2c93 authored by Tom Cherry's avatar Tom Cherry
Browse files

logd: refactor mLast setting into a GetOldest function 

This code was duplicated throughout LogBuffer.cpp, so refactor it into
a single location and clean up some comments along the way.

This may fix a subtle bug: if `logcat -c` is used from a
non-privileged UID, the current code may set mLast to the oldest seen
log message *from that UID* and not not the oldest log message for
that log id.  That may prevent pruning from the start of that log,
resulting in old log entries that are impossible to delete.

Bug: 144382260
Test: logging works, and above scenario is not seen
Change-Id: I1749293ce6ea1697dd8a9258cfd7eab29dbeac6e
parent 2e3af867

logd/LogBuffer.cpp

+27 −40
Original line number Diff line number Diff line
@@ -46,9 +46,6 @@ 


void LogBuffer::init() {

    log_id_for_each(i) {

        mLastSet[i] = false;

        mLast[i] = mLogElements.begin();



        if (setSize(i, __android_logger_get_buffer_size(i))) {

            setSize(i, LOG_BUFFER_MIN_SIZE);

        }
@@ -131,6 +128,20 @@ LogBuffer::~LogBuffer() { 
    }

}



LogBufferElementCollection::iterator LogBuffer::GetOldest(log_id_t log_id) {

    auto it = mLogElements.begin();

    if (mOldest[log_id]) {

        it = *mOldest[log_id];

    }

    while (it != mLogElements.end() && (*it)->getLogId() != log_id) {

        it++;

    }

    if (it != mLogElements.end()) {

        mOldest[log_id] = it;

    }

    return it;

}



enum match_type { DIFFERENT, SAME, SAME_LIBLOG };



static enum match_type identical(LogBufferElement* elem,
@@ -450,9 +461,7 @@ LogBufferElementCollection::iterator LogBuffer::erase( 


    bool setLast[LOG_ID_MAX];

    bool doSetLast = false;

    log_id_for_each(i) {

        doSetLast |= setLast[i] = mLastSet[i] && (it == mLast[i]);

    }

    log_id_for_each(i) { doSetLast |= setLast[i] = mOldest[i] && it == *mOldest[i]; }

#ifdef DEBUG_CHECK_FOR_STALE_ENTRIES

    LogBufferElementCollection::iterator bad = it;

    int key = ((id == LOG_ID_EVENTS) || (id == LOG_ID_SECURITY))
@@ -463,11 +472,11 @@ LogBufferElementCollection::iterator LogBuffer::erase( 
    if (doSetLast) {

        log_id_for_each(i) {

            if (setLast[i]) {

                if (__predict_false(it == mLogElements.end())) {  // impossible

                    mLastSet[i] = false;

                    mLast[i] = mLogElements.begin();

                if (__predict_false(it == mLogElements.end())) {

                    mOldest[i] = std::nullopt;

                } else {

                    mLast[i] = it;  // push down the road as next-best-watermark

                    mOldest[i] = it;  // Store the next iterator even if it does not correspond to

                                      // the same log_id, as a starting point for GetOldest().

                }

            }

        }
@@ -486,11 +495,6 @@ LogBufferElementCollection::iterator LogBuffer::erase( 
                                 b.first);

            }

        }

        if (mLastSet[i] && (bad == mLast[i])) {

            android::prdebug("stale mLast[%d]\n", i);

            mLastSet[i] = false;

            mLast[i] = mLogElements.begin();

        }

    }

#endif

    if (coalesce) {
@@ -668,7 +672,7 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
    if (__predict_false(caller_uid != AID_ROOT)) {  // unlikely

        // Only here if clear all request from non system source, so chatty

        // filter logistics is not required.

        it = mLastSet[id] ? mLast[id] : mLogElements.begin();

        it = GetOldest(id);

        while (it != mLogElements.end()) {

            LogBufferElement* element = *it;
@@ -678,11 +682,6 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
                continue;

            }



            if (!mLastSet[id] || ((*mLast[id])->getLogId() != id)) {

                mLast[id] = it;

                mLastSet[id] = true;

            }



            if (oldest && oldest->mStart <= element->getSequence()) {

                busy = true;

                kickMe(oldest, id, pruneRows);
@@ -734,8 +733,8 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
        }



        bool kick = false;

        bool leading = true;

        it = mLastSet[id] ? mLast[id] : mLogElements.begin();

        bool leading = true;  // true if starting from the oldest log entry, false if starting from

                              // a specific chatty entry.

        // Perform at least one mandatory garbage collection cycle in following

        // - clear leading chatty tags

        // - coalesce chatty tags
@@ -763,6 +762,9 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
                }

            }

        }

        if (leading) {

            it = GetOldest(id);

        }

        static const timespec too_old = { EXPIRE_HOUR_THRESHOLD * 60 * 60, 0 };

        LogBufferElementCollection::iterator lastt;

        lastt = mLogElements.end();
@@ -783,11 +785,6 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
            }

            // below this point element->getLogId() == id



            if (leading && (!mLastSet[id] || ((*mLast[id])->getLogId() != id))) {

                mLast[id] = it;

                mLastSet[id] = true;

            }



            uint16_t dropped = element->getDropped();



            // remove any leading drops
@@ -909,7 +906,7 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 


    bool whitelist = false;

    bool hasWhitelist = (id != LOG_ID_SECURITY) && mPrune.nice() && !clearAll;

    it = mLastSet[id] ? mLast[id] : mLogElements.begin();

    it = GetOldest(id);

    while ((pruneRows > 0) && (it != mLogElements.end())) {

        LogBufferElement* element = *it;
@@ -918,11 +915,6 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
            continue;

        }



        if (!mLastSet[id] || ((*mLast[id])->getLogId() != id)) {

            mLast[id] = it;

            mLastSet[id] = true;

        }



        if (oldest && oldest->mStart <= element->getSequence()) {

            busy = true;

            if (!whitelist) kickMe(oldest, id, pruneRows);
@@ -942,7 +934,7 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 


    // Do not save the whitelist if we are reader range limited

    if (whitelist && (pruneRows > 0)) {

        it = mLastSet[id] ? mLast[id] : mLogElements.begin();

        it = GetOldest(id);

        while ((it != mLogElements.end()) && (pruneRows > 0)) {

            LogBufferElement* element = *it;
@@ -951,11 +943,6 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
                continue;

            }



            if (!mLastSet[id] || ((*mLast[id])->getLogId() != id)) {

                mLast[id] = it;

                mLastSet[id] = true;

            }



            if (oldest && oldest->mStart <= element->getSequence()) {

                busy = true;

                kickMe(oldest, id, pruneRows);

logd/LogBuffer.h

+8 −3
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ 
#include <sys/types.h>



#include <list>

#include <optional>

#include <string>



#include <android/log.h>
@@ -81,9 +82,9 @@ class LogBuffer { 
    LogStatistics stats;



    PruneList mPrune;

    // watermark for last per log id

    LogBufferElementCollection::iterator mLast[LOG_ID_MAX];

    bool mLastSet[LOG_ID_MAX];

    // Keeps track of the iterator to the oldest log message of a given log type, as an

    // optimization when pruning logs.  Use GetOldest() to retrieve.

    std::optional<LogBufferElementCollection::iterator> mOldest[LOG_ID_MAX];

    // watermark of any worst/chatty uid processing

    typedef std::unordered_map<uid_t, LogBufferElementCollection::iterator>

        LogBufferIteratorMap;
@@ -181,6 +182,10 @@ class LogBuffer { 
    bool prune(log_id_t id, unsigned long pruneRows, uid_t uid = AID_ROOT);

    LogBufferElementCollection::iterator erase(

        LogBufferElementCollection::iterator it, bool coalesce = false);



    // Returns an iterator to the oldest element for a given log type, or mLogElements.end() if

    // there are no logs for the given log type. Requires mLogElementsLock to be held.

    LogBufferElementCollection::iterator GetOldest(log_id_t log_id);

};



#endif  // _LOGD_LOG_BUFFER_H__