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

Commit db8a266a authored by Mark Salyzyn's avatar Mark Salyzyn
Browse files

liblog: __android_log_pmsg_file_write() cleanup 

__android_log_pmsg_file_write() will open /dev/pmsg0 if not
already, and will close it if we opened it.

Added atomic access to the android_log_context as insurance.

Fortify and correct pmsg tests.

Test: gTest liblog-unit-tests --gtest_filter=liblog.__android_log_pmsg_file_*
Bug: 31958686
Change-Id: I2cf6f971b6968938f471fda67367efe20dae3004
parent bf0cf4e2

liblog/logd_reader.c

+14 −10
Original line number Diff line number Diff line
@@ -482,17 +482,17 @@ static int logdOpen(struct android_log_logger_list *logger_list, 
    struct sigaction old_sigaction;

    unsigned int old_alarm = 0;

    char buffer[256], *cp, c;

    int e, ret, remaining;



    int sock = transp->context.sock;

    if (sock > 0) {

        return sock;

    }

    int e, ret, remaining, sock;



    if (!logger_list) {

        return -EINVAL;

    }



    sock = atomic_load(&transp->context.sock);

    if (sock > 0) {

        return sock;

    }



    sock = socket_local_client("logdr",

                               ANDROID_SOCKET_NAMESPACE_RESERVED,

                               SOCK_SEQPACKET);
@@ -587,7 +587,11 @@ static int logdOpen(struct android_log_logger_list *logger_list, 
        return ret;

    }



    return transp->context.sock = sock;

    ret = atomic_exchange(&transp->context.sock, sock);

    if ((ret > 0) && (ret != sock)) {

        close(ret);

    }

    return sock;

}



/* Read from the selected logs */
@@ -662,8 +666,8 @@ static int logdPoll(struct android_log_logger_list *logger_list, 
static void logdClose(struct android_log_logger_list *logger_list __unused,

                      struct android_log_transport_context *transp)

{

    if (transp->context.sock > 0) {

        close (transp->context.sock);

        transp->context.sock = -1;

    int sock = atomic_exchange(&transp->context.sock, -1);

    if (sock > 0) {

        close (sock);

    }

}

liblog/logd_writer.c

+21 −13
Original line number Diff line number Diff line
@@ -31,7 +31,6 @@ 
#include <time.h>

#include <unistd.h>



#include <android/log.h>

#include <cutils/sockets.h>

#include <log/logger.h>

#include <private/android_filesystem_config.h>
@@ -65,7 +64,8 @@ static int logdOpen() 
{

    int i, ret = 0;



    if (logdLoggerWrite.context.sock < 0) {

    i = atomic_load(&logdLoggerWrite.context.sock);

    if (i < 0) {

        i = TEMP_FAILURE_RETRY(socket(PF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0));

        if (i < 0) {

            ret = -errno;
@@ -80,7 +80,11 @@ static int logdOpen() 
                ret = -errno;

                close(i);

            } else {

                logdLoggerWrite.context.sock = i;

                ret = atomic_exchange(&logdLoggerWrite.context.sock, i);

                if ((ret >= 0) && (ret != i)) {

                    close(ret);

                }

                ret = 0;

            }

        }

    }
@@ -90,9 +94,9 @@ static int logdOpen() 


static void logdClose()

{

    if (logdLoggerWrite.context.sock >= 0) {

        close(logdLoggerWrite.context.sock);

        logdLoggerWrite.context.sock = -1;

    int sock = atomic_exchange(&logdLoggerWrite.context.sock, -1);

    if (sock >= 0) {

        close(sock);

    }

}
@@ -101,7 +105,7 @@ static int logdAvailable(log_id_t logId) 
    if (logId > LOG_ID_SECURITY) {

        return -EINVAL;

    }

    if (logdLoggerWrite.context.sock < 0) {

    if (atomic_load(&logdLoggerWrite.context.sock) < 0) {

        if (access("/dev/socket/logdw", W_OK) == 0) {

            return 0;

        }
@@ -121,7 +125,7 @@ static int logdWrite(log_id_t logId, struct timespec *ts, 
    static atomic_int_fast32_t dropped;

    static atomic_int_fast32_t droppedSecurity;



    if (logdLoggerWrite.context.sock < 0) {

    if (atomic_load(&logdLoggerWrite.context.sock) < 0) {

        return -EBADF;

    }
@@ -160,7 +164,7 @@ static int logdWrite(log_id_t logId, struct timespec *ts, 
    newVec[0].iov_base = (unsigned char *)&header;

    newVec[0].iov_len  = sizeof(header);



    if (logdLoggerWrite.context.sock > 0) {

    if (atomic_load(&logdLoggerWrite.context.sock) > 0) {

        int32_t snapshot = atomic_exchange_explicit(&droppedSecurity, 0,

                                                    memory_order_relaxed);

        if (snapshot) {
@@ -174,7 +178,8 @@ static int logdWrite(log_id_t logId, struct timespec *ts, 
            newVec[headerLength].iov_base = &buffer;

            newVec[headerLength].iov_len  = sizeof(buffer);



            ret = TEMP_FAILURE_RETRY(writev(logdLoggerWrite.context.sock, newVec, 2));

            ret = TEMP_FAILURE_RETRY(writev(

                    atomic_load(&logdLoggerWrite.context.sock), newVec, 2));

            if (ret != (ssize_t)(sizeof(header) + sizeof(buffer))) {

                atomic_fetch_add_explicit(&droppedSecurity, snapshot,

                                          memory_order_relaxed);
@@ -194,7 +199,8 @@ static int logdWrite(log_id_t logId, struct timespec *ts, 
            newVec[headerLength].iov_base = &buffer;

            newVec[headerLength].iov_len  = sizeof(buffer);



            ret = TEMP_FAILURE_RETRY(writev(logdLoggerWrite.context.sock, newVec, 2));

            ret = TEMP_FAILURE_RETRY(writev(

                      atomic_load(&logdLoggerWrite.context.sock), newVec, 2));

            if (ret != (ssize_t)(sizeof(header) + sizeof(buffer))) {

                atomic_fetch_add_explicit(&dropped, snapshot,

                                          memory_order_relaxed);
@@ -223,7 +229,8 @@ static int logdWrite(log_id_t logId, struct timespec *ts, 
     * ENOTCONN occurs if logd dies.

     * EAGAIN occurs if logd is overloaded.

     */

    ret = TEMP_FAILURE_RETRY(writev(logdLoggerWrite.context.sock, newVec, i));

    ret = TEMP_FAILURE_RETRY(writev(

            atomic_load(&logdLoggerWrite.context.sock), newVec, i));

    if (ret < 0) {

        ret = -errno;

        if (ret == -ENOTCONN) {
@@ -236,7 +243,8 @@ static int logdWrite(log_id_t logId, struct timespec *ts, 
                return ret;

            }



            ret = TEMP_FAILURE_RETRY(writev(logdLoggerWrite.context.sock, newVec, i));

            ret = TEMP_FAILURE_RETRY(writev(

                    atomic_load(&logdLoggerWrite.context.sock), newVec, i));

            if (ret < 0) {

                ret = -errno;

            }

liblog/logger.h

+4 −2
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
#ifndef _LIBLOG_LOGGER_H__

#define _LIBLOG_LOGGER_H__



#include <stdatomic.h>

#include <stdbool.h>

#include <log/uio.h>
@@ -31,9 +32,10 @@ __BEGIN_DECLS 
/* Union, sock or fd of zero is not allowed unless static initialized */

union android_log_context {

  void *private;

  int sock;

  int fd;

  atomic_int sock;

  atomic_int fd;

  struct listnode *node;

  atomic_uintptr_t atomic_pointer;

};



struct android_log_transport_write {

liblog/pmsg_reader.c

+33 −13
Original line number Diff line number Diff line
@@ -151,8 +151,8 @@ static int pmsgRead(struct android_log_logger_list *logger_list, 


    memset(log_msg, 0, sizeof(*log_msg));



    if (transp->context.fd <= 0) {

        int fd = open("/sys/fs/pstore/pmsg-ramoops-0", O_RDONLY | O_CLOEXEC);

    if (atomic_load(&transp->context.fd) <= 0) {

        int i, fd = open("/sys/fs/pstore/pmsg-ramoops-0", O_RDONLY | O_CLOEXEC);



        if (fd < 0) {

            return -errno;
@@ -164,13 +164,22 @@ static int pmsgRead(struct android_log_logger_list *logger_list, 
                return -errno;

            }

        }

        transp->context.fd = fd;

        i = atomic_exchange(&transp->context.fd, fd);

        if ((i > 0) && (i != fd)) {

            close(i);

        }

        preread_count = 0;

    }



    while(1) {

        int fd;



        if (preread_count < sizeof(buf)) {

            ret = TEMP_FAILURE_RETRY(read(transp->context.fd,

            fd = atomic_load(&transp->context.fd);

            if (fd <= 0) {

                return -EBADF;

            }

            ret = TEMP_FAILURE_RETRY(read(fd,

                                          &buf.p.magic + preread_count,

                                          sizeof(buf) - preread_count));

            if (ret < 0) {
@@ -212,7 +221,11 @@ static int pmsgRead(struct android_log_logger_list *logger_list, 
                    log_msg->entry_v4.msg :

                    log_msg->entry_v3.msg;

                *msg = buf.prio;

                ret = TEMP_FAILURE_RETRY(read(transp->context.fd,

                fd = atomic_load(&transp->context.fd);

                if (fd <= 0) {

                    return -EBADF;

                }

                ret = TEMP_FAILURE_RETRY(read(fd,

                                              msg + sizeof(buf.prio),

                                              buf.p.len - sizeof(buf)));

                if (ret < 0) {
@@ -239,12 +252,19 @@ static int pmsgRead(struct android_log_logger_list *logger_list, 
            }

        }



        current = TEMP_FAILURE_RETRY(lseek(transp->context.fd,

                                           (off_t)0, SEEK_CUR));

        fd = atomic_load(&transp->context.fd);

        if (fd <= 0) {

            return -EBADF;

        }

        current = TEMP_FAILURE_RETRY(lseek(fd, (off_t)0, SEEK_CUR));

        if (current < 0) {

            return -errno;

        }

        next = TEMP_FAILURE_RETRY(lseek(transp->context.fd,

        fd = atomic_load(&transp->context.fd);

        if (fd <= 0) {

            return -EBADF;

        }

        next = TEMP_FAILURE_RETRY(lseek(fd,

                                        (off_t)(buf.p.len - sizeof(buf)),

                                        SEEK_CUR));

        if (next < 0) {
@@ -258,10 +278,10 @@ static int pmsgRead(struct android_log_logger_list *logger_list, 


static void pmsgClose(struct android_log_logger_list *logger_list __unused,

                      struct android_log_transport_context *transp) {

    if (transp->context.fd > 0) {

        close (transp->context.fd);

    int fd = atomic_exchange(&transp->context.fd, 0);

    if (fd > 0) {

        close (fd);

    }

    transp->context.fd = 0;

}



LIBLOG_ABI_PRIVATE ssize_t __android_log_pmsg_file_read(

liblog/pmsg_writer.c

+42 −21
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ 


#include <errno.h>

#include <fcntl.h>

#include <stdbool.h>

#include <stdlib.h>

#include <string.h>

#include <sys/types.h>
@@ -53,18 +54,25 @@ LIBLOG_HIDDEN struct android_log_transport_write pmsgLoggerWrite = { 


static int pmsgOpen()

{

    if (pmsgLoggerWrite.context.fd < 0) {

        pmsgLoggerWrite.context.fd = TEMP_FAILURE_RETRY(open("/dev/pmsg0", O_WRONLY | O_CLOEXEC));

    int fd = atomic_load(&pmsgLoggerWrite.context.fd);

    if (fd < 0) {

        int i;



        fd = TEMP_FAILURE_RETRY(open("/dev/pmsg0", O_WRONLY | O_CLOEXEC));

        i = atomic_exchange(&pmsgLoggerWrite.context.fd, fd);

        if ((i >= 0) && (i != fd)) {

            close(i);

        }

    }



    return pmsgLoggerWrite.context.fd;

    return fd;

}



static void pmsgClose()

{

    if (pmsgLoggerWrite.context.fd >= 0) {

        close(pmsgLoggerWrite.context.fd);

        pmsgLoggerWrite.context.fd = -1;

    int fd = atomic_exchange(&pmsgLoggerWrite.context.fd, -1);

    if (fd >= 0) {

        close(fd);

    }

}
@@ -78,7 +86,7 @@ static int pmsgAvailable(log_id_t logId) 
            !__android_log_is_debuggable()) {

        return -EINVAL;

    }

    if (pmsgLoggerWrite.context.fd < 0) {

    if (atomic_load(&pmsgLoggerWrite.context.fd) < 0) {

        if (access("/dev/pmsg0", W_OK) == 0) {

            return 0;

        }
@@ -115,7 +123,7 @@ static int pmsgWrite(log_id_t logId, struct timespec *ts, 
        }

    }



    if (pmsgLoggerWrite.context.fd < 0) {

    if (atomic_load(&pmsgLoggerWrite.context.fd) < 0) {

        return -EBADF;

    }
@@ -169,7 +177,8 @@ static int pmsgWrite(log_id_t logId, struct timespec *ts, 
    }

    pmsgHeader.len += payloadSize;



    ret = TEMP_FAILURE_RETRY(writev(pmsgLoggerWrite.context.fd, newVec, i));

    ret = TEMP_FAILURE_RETRY(writev(atomic_load(&pmsgLoggerWrite.context.fd),

                                    newVec, i));

    if (ret < 0) {

        ret = errno ? -errno : -ENOTCONN;

    }
@@ -206,7 +215,7 @@ LIBLOG_ABI_PRIVATE ssize_t __android_log_pmsg_file_write( 
        char prio,

        const char *filename,

        const char *buf, size_t len) {

    int fd;

    bool weOpened;

    size_t length, packet_len;

    const char *tag;

    char *cp, *slash;
@@ -228,16 +237,6 @@ LIBLOG_ABI_PRIVATE ssize_t __android_log_pmsg_file_write( 
        return -ENOMEM;

    }



    fd = pmsgLoggerWrite.context.fd;

    if (fd < 0) {

        __android_log_lock();

        fd = pmsgOpen();

        __android_log_unlock();

        if (fd < 0) {

            return -EBADF;

        }

    }



    tag = cp;

    slash = strrchr(cp, '/');

    if (slash) {
@@ -256,6 +255,7 @@ LIBLOG_ABI_PRIVATE ssize_t __android_log_pmsg_file_write( 
    vec[1].iov_base = (unsigned char *)tag;

    vec[1].iov_len  = length;



    weOpened = false;

    for (ts.tv_nsec = 0, length = len;

            length;

            ts.tv_nsec += ANDROID_LOG_PMSG_FILE_SEQUENCE) {
@@ -279,15 +279,36 @@ LIBLOG_ABI_PRIVATE ssize_t __android_log_pmsg_file_write( 
        vec[2].iov_base = (unsigned char *)buf;

        vec[2].iov_len  = transfer;



        if (atomic_load(&pmsgLoggerWrite.context.fd) < 0) {

            if (!weOpened) { /* Impossible for weOpened = true here */

                __android_log_lock();

            }

            weOpened = atomic_load(&pmsgLoggerWrite.context.fd) < 0;

            if (!weOpened) {

                __android_log_unlock();

            } else if (pmsgOpen() < 0) {

                __android_log_unlock();

                return -EBADF;

            }

        }



        ret = pmsgWrite(logId, &ts, vec, sizeof(vec) / sizeof(vec[0]));



        if (ret <= 0) {

            if (weOpened) {

                pmsgClose();

                __android_log_unlock();

            }

            free(cp);

            return ret;

            return ret ? ret : (len - length);

        }

        length -= transfer;

        buf += transfer;

    }

    if (weOpened) {

        pmsgClose();

        __android_log_unlock();

    }

    free(cp);

    return len;

}