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

Commit 0484b3b5 authored by Mark Salyzyn's avatar Mark Salyzyn
Browse files

logd: ASAN cleansing 

A mixture of fixes and cleanup for LogKlog.cpp and friends.

- sscanf calls strlen.  Check if the string is missing a nul
  terminator, if it is, do not call sscanf.
- replace NULL with nullptr for stronger typechecking.
- pass by reference for simpler code.
- Use ssize_t where possible to check for negative values.
- fix FastCmp to add some validity checking since ASAN reports that
  callers are not making sure pre-conditions are met.
- add fasticmp templates for completeness.
- if the buffer is too small to contain a meaningful time, do not
  call down to log_time::strptime() because it does not limit its
  accesses to the buffer boundaries, instead stopping at a
  terminating nul or invalid match.
- move strnstr to LogUtils.h, drop size checking of needle and
  clearly report the list of needles used with android::strnstr
- replace 'sizeof(static const char[]) - 1' with strlen.

Test: gTest liblog-unit-test, logd-unit-tests & logcat-unit-tests
Bug: 30792935
Bug: 36536248
Bug: 35468874
Bug: 34949125
Bug: 34606909
Bug: 36075298
Bug: 36608728
Change-Id: I161bf03ba029050e809b31cceef03f729d318866
parent 784c8517

libutils/include/utils/FastStrcmp.h

+35 −12
Original line number Diff line number Diff line
@@ -17,6 +17,13 @@ 
#ifndef _ANDROID_UTILS_FASTSTRCMP_H__

#define _ANDROID_UTILS_FASTSTRCMP_H__



#include <ctype.h>

#include <string.h>



#ifndef __predict_true

#define __predict_true(exp) __builtin_expect((exp) != 0, 1)

#endif



#ifdef __cplusplus



// Optimized for instruction cache locality
@@ -28,25 +35,41 @@ 
//

//  fastcmp<strncmp>(str1, str2, len)

//

// NB: Does not work for the case insensitive str*cmp functions.

// NB: use fasticmp for the case insensitive str*cmp functions.

// NB: Returns boolean, do not use if expecting to check negative value.

//     Thus not semantically identical to the expected function behavior.



template <int (*cmp)(const char* l, const char* r, const size_t s)>

static inline int fastcmp(const char* l, const char* r, const size_t s) {

    return (*l != *r) || cmp(l + 1, r + 1, s - 1);

    const ssize_t n = s;  // To help reject negative sizes, treat like zero

    return __predict_true(n > 0) &&

           ((*l != *r) || (__predict_true(n > 1) && cmp(l + 1, r + 1, n - 1)));

}



template <int (*cmp)(const char* l, const char* r, const size_t s)>

static inline int fasticmp(const char* l, const char* r, const size_t s) {

    const ssize_t n = s;  // To help reject negative sizes, treat like zero

    return __predict_true(n > 0) &&

           ((tolower(*l) != tolower(*r)) || (__predict_true(n > 1) && cmp(l + 1, r + 1, n - 1)));

}



template <int (*cmp)(const void* l, const void* r, const size_t s)>

static inline int fastcmp(const void* lv, const void* rv, const size_t s) {

    const char* l = static_cast<const char*>(lv);

    const char* r = static_cast<const char*>(rv);

    return (*l != *r) || cmp(l + 1, r + 1, s - 1);

    const ssize_t n = s;  // To help reject negative sizes, treat like zero

    return __predict_true(n > 0) &&

           ((*l != *r) || (__predict_true(n > 1) && cmp(l + 1, r + 1, n - 1)));

}



template <int (*cmp)(const char* l, const char* r)>

static inline int fastcmp(const char* l, const char* r) {

    return (*l != *r) || cmp(l + 1, r + 1);

    return (*l != *r) || (__predict_true(*l) && cmp(l + 1, r + 1));

}



template <int (*cmp)(const char* l, const char* r)>

static inline int fasticmp(const char* l, const char* r) {

    return (tolower(*l) != tolower(*r)) || (__predict_true(*l) && cmp(l + 1, r + 1));

}



#endif

logd/LogBuffer.cpp

+6 −10
Original line number Diff line number Diff line
@@ -132,11 +132,11 @@ static enum match_type identical(LogBufferElement* elem, 
                                 LogBufferElement* last) {

    // is it mostly identical?

    //  if (!elem) return DIFFERENT;

    unsigned short lenl = elem->getMsgLen();

    if (!lenl) return DIFFERENT;

    ssize_t lenl = elem->getMsgLen();

    if (lenl <= 0) return DIFFERENT;  // value if this represents a chatty elem

    //  if (!last) return DIFFERENT;

    unsigned short lenr = last->getMsgLen();

    if (!lenr) return DIFFERENT;

    ssize_t lenr = last->getMsgLen();

    if (lenr <= 0) return DIFFERENT;  // value if this represents a chatty elem

    //  if (elem->getLogId() != last->getLogId()) return DIFFERENT;

    if (elem->getUid() != last->getUid()) return DIFFERENT;

    if (elem->getPid() != last->getPid()) return DIFFERENT;
@@ -162,8 +162,6 @@ static enum match_type identical(LogBufferElement* elem, 
    }



    // audit message (except sequence number) identical?

    static const char avc[] = "): avc: ";



    if (last->isBinary()) {

        if (fastcmp<memcmp>(msgl, msgr, sizeof(android_log_event_string_t) -

                                            sizeof(int32_t)))
@@ -173,6 +171,7 @@ static enum match_type identical(LogBufferElement* elem, 
        msgr += sizeof(android_log_event_string_t);

        lenr -= sizeof(android_log_event_string_t);

    }

    static const char avc[] = "): avc: ";

    const char* avcl = android::strnstr(msgl, lenl, avc);

    if (!avcl) return DIFFERENT;

    lenl -= avcl - msgl;
@@ -180,10 +179,7 @@ static enum match_type identical(LogBufferElement* elem, 
    if (!avcr) return DIFFERENT;

    lenr -= avcr - msgr;

    if (lenl != lenr) return DIFFERENT;

    // TODO: After b/35468874 is addressed, revisit "lenl > strlen(avc)"

    // condition, it might become superfluous.

    if (lenl > strlen(avc) &&

        fastcmp<memcmp>(avcl + strlen(avc), avcr + strlen(avc),

    if (fastcmp<memcmp>(avcl + strlen(avc), avcr + strlen(avc),

                        lenl - strlen(avc))) {

        return DIFFERENT;

    }

logd/LogKlog.cpp

+154 −175

File changed.

Preview size limit exceeded, changes collapsed.

logd/LogKlog.h

+5 −7
Original line number Diff line number Diff line
@@ -20,8 +20,6 @@ 
#include <private/android_logger.h>

#include <sysutils/SocketListener.h>



char* log_strntok_r(char* s, size_t* len, char** saveptr, size_t* sublen);



class LogBuffer;

class LogReader;
@@ -43,8 +41,8 @@ class LogKlog : public SocketListener { 
   public:

    LogKlog(LogBuffer* buf, LogReader* reader, int fdWrite, int fdRead,

            bool auditd);

    int log(const char* buf, size_t len);

    void synchronize(const char* buf, size_t len);

    int log(const char* buf, ssize_t len);

    void synchronize(const char* buf, ssize_t len);



    bool isMonotonic() {

        return logbuf->isMonotonic();
@@ -57,10 +55,10 @@ class LogKlog : public SocketListener { 
    }



   protected:

    void sniffTime(log_time& now, const char** buf, size_t len, bool reverse);

    pid_t sniffPid(const char** buf, size_t len);

    void sniffTime(log_time& now, const char*& buf, ssize_t len, bool reverse);

    pid_t sniffPid(const char*& buf, ssize_t len);

    void calculateCorrection(const log_time& monotonic, const char* real_string,

                             size_t len);

                             ssize_t len);

    virtual bool onDataAvailable(SocketClient* cli);

};

logd/LogUtils.h

+19 −2
Original line number Diff line number Diff line
@@ -43,8 +43,25 @@ char* tidToName(pid_t tid); 
const char* tagToName(uint32_t tag);

void ReReadEventLogTags();



// Furnished by LogKlog.cpp.

const char* strnstr(const char* s, size_t len, const char* needle);

// Furnished by LogKlog.cpp

char* log_strntok_r(char* s, ssize_t& len, char*& saveptr, ssize_t& sublen);



// needle should reference a string longer than 1 character

static inline const char* strnstr(const char* s, ssize_t len,

                                  const char* needle) {

    if (len <= 0) return nullptr;



    const char c = *needle++;

    const size_t needleLen = strlen(needle);

    do {

        do {

            if (len <= (ssize_t)needleLen) return nullptr;

            --len;

        } while (*s++ != c);

    } while (fastcmp<memcmp>(s, needle, needleLen));

    s--;

    return s;

}

}



// Furnished in LogCommand.cpp