From: Michal Bloch Date: Tue, 13 Mar 2018 13:34:04 +0000 (+0100) Subject: Handle payload sizes correctly X-Git-Tag: accepted/tizen/unified/20180315.061335^0 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4634037472cf1ad0862e6f658969cd854439d19e;p=platform%2Fcore%2Fsystem%2Fdlog.git Handle payload sizes correctly Change-Id: Ie809ace9b5463025caeefdbbda9d8d237bb318b9 Signed-off-by: Michal Bloch --- diff --git a/include/logcommon.h b/include/logcommon.h index 524f646..6f3f753 100644 --- a/include/logcommon.h +++ b/include/logcommon.h @@ -19,8 +19,7 @@ #ifndef _LOGCOMMON_H #define _LOGCOMMON_H -#define LOG_MAX_PAYLOAD_SIZE 4076 -#define LOG_MAX_SIZE (LOG_MAX_PAYLOAD_SIZE + sizeof(struct logger_entry)) +#define LOG_MAX_PAYLOAD_SIZE 4076 // has to be at least the value in the kernel: drivers/staging/android/logger.h #include #include diff --git a/src/libdlog/log.c b/src/libdlog/log.c index 65d62e7..89ba2a8 100644 --- a/src/libdlog/log.c +++ b/src/libdlog/log.c @@ -242,7 +242,7 @@ static int dlog_should_log(log_id_t log_id, const char* tag, int prio) static int __write_to_log(log_id_t log_id, int prio, const char *tag, const char *fmt, va_list ap, bool check_should_log) { - char buf[LOG_MAX_SIZE]; + char buf[LOG_MAX_PAYLOAD_SIZE]; __dlog_init(); @@ -250,7 +250,7 @@ static int __write_to_log(log_id_t log_id, int prio, const char *tag, const char if (ret < 0) return ret; - vsnprintf(buf, LOG_MAX_SIZE, fmt, ap); + vsnprintf(buf, sizeof buf, fmt, ap); return write_to_log(log_id, prio, tag, buf); } diff --git a/src/libdlog/log_pipe.c b/src/libdlog/log_pipe.c index 86929d9..b43878a 100644 --- a/src/libdlog/log_pipe.c +++ b/src/libdlog/log_pipe.c @@ -107,8 +107,8 @@ static int __reconnect_pipe(log_id_t log_id) static int __write_to_log_pipe(log_id_t log_id, log_priority prio, const char *tag, const char *msg) { ssize_t ret; - char buf[LOG_MAX_SIZE]; - struct pipe_logger_entry* ple = (struct pipe_logger_entry*)buf; + char buf[LOG_MAX_PAYLOAD_SIZE + sizeof(struct pipe_logger_entry)]; + struct pipe_logger_entry* ple = (struct pipe_logger_entry *)buf; if (!tag) tag = ""; @@ -117,7 +117,7 @@ static int __write_to_log_pipe(log_id_t log_id, log_priority prio, const char *t prio < DLOG_VERBOSE || prio >= DLOG_PRIO_MAX || !msg || - strlen(tag) + strlen(msg) + sizeof(struct pipe_logger_entry) + 2 >= LOG_MAX_SIZE) + 1 /* priority */ + strlen(tag) + 1 /* NULL delimiter */ + strlen(msg) + 1 /* NULL terminator */ >= LOG_MAX_PAYLOAD_SIZE) return DLOG_ERROR_INVALID_PARAMETER; create_pipe_message(buf, prio, tag, msg); diff --git a/src/logger/logger.c b/src/logger/logger.c index d140e16..ac585c8 100755 --- a/src/logger/logger.c +++ b/src/logger/logger.c @@ -125,14 +125,16 @@ typedef int (*service_socket_t)(struct logger *server, struct writer *wr, struct */ typedef int (*service_reader_t)(struct logger *server, struct reader *reader); +#define LARGEST_STRUCT(a, b) (sizeof(struct a) > sizeof(struct b) ? sizeof(struct a) : sizeof(struct b)) struct writer { struct fd_entity fd_entity; struct log_buffer* buf_ptr; unsigned readed; service_writer_t service_writer; service_socket_t service_socket; - char buffer[LOG_MAX_SIZE]; + char buffer[LOG_MAX_PAYLOAD_SIZE + LARGEST_STRUCT(android_logger_entry, pipe_logger_entry)]; }; +#undef LARGEST_STRUCT /** * @brief Service reader @@ -153,7 +155,7 @@ struct reader { int dumpcount; struct timespec last_read_time; int partial_log_size; - char partial_log[LOG_MAX_SIZE]; + char partial_log[sizeof(struct logger_entry_with_msg)]; service_reader_t service_reader; }; @@ -868,8 +870,7 @@ static int print_out_logs(struct logger *server, struct reader *reader) assert(reader->buf_ptr); int r, ret = 0; - struct logger_entry* ple; - char tmp[LOG_MAX_SIZE]; + struct logger_entry_with_msg lem; char * tag; unsigned from = reader->current; log_entry entry; @@ -891,35 +892,34 @@ static int print_out_logs(struct logger *server, struct reader *reader) } while (from != reader->buf_ptr->tail && (!reader->dumpcount || (reader->bytes_to_read > 0))) { - typeof(ple->len) length; + typeof(lem.header.len) length; copy_from_buffer(&length, from, sizeof length, reader->buf_ptr); - copy_from_buffer(tmp, from, length, reader->buf_ptr); - ple = (struct logger_entry*)tmp; + copy_from_buffer(&lem, from, length, reader->buf_ptr); - from += ple->len; + from += lem.header.len; from %= reader->buf_ptr->size; if (reader->dumpcount) { - if (reader->bytes_to_read > ple->len) - reader->bytes_to_read -= ple->len; + if (reader->bytes_to_read > lem.header.len) + reader->bytes_to_read -= lem.header.len; else reader->bytes_to_read = 0; } if (plaintext) { - logfile_write_with_rotation(ple, &reader->file); + logfile_write_with_rotation(&lem.header, &reader->file); continue; } - tag = ple->msg + 1; + tag = lem.msg + 1; if (!strlen(tag)) continue; - if (log_process_log_buffer(ple, &entry) || !log_should_print_line(reader->file.format, &entry)) + if (log_process_log_buffer(&lem.header, &entry) || !log_should_print_line(reader->file.format, &entry)) continue; - r = write(reader->file.path ? reader->file.fd : reader->fd_entity.fd, ple, ple->len); + r = write(reader->file.path ? reader->file.fd : reader->fd_entity.fd, &lem, lem.header.len); if (r < 0) { if (errno == EPIPE) ret = 1; @@ -927,9 +927,9 @@ static int print_out_logs(struct logger *server, struct reader *reader) } reader->file.size += r; - if (r < ple->len) { - reader->partial_log_size = ple->len - r; - memcpy(reader->partial_log, ple + r, reader->partial_log_size); + if (r < lem.header.len) { + reader->partial_log_size = lem.header.len - r; + memcpy(reader->partial_log, ((char *)&lem) + r, reader->partial_log_size); goto cleanup; } else if (logfile_rotate_needed(&reader->file) > 0) { logfile_do_rotate(&reader->file); @@ -989,8 +989,8 @@ static int service_reader_file(struct logger * server, struct reader* reader) { assert(reader); - static char buffer[LOG_MAX_SIZE + 1]; struct logger_entry_with_msg entry; + static char buffer[sizeof entry + 1]; buffer[sizeof buffer - 1] = '\0'; /* The devices for both KMSG and Android Logger only return one log per read(). @@ -1501,7 +1501,7 @@ static int service_writer_socket(struct logger* server, struct writer* wr, struc assert(event); if (event->events & EPOLLIN) - r = read(wr->fd_entity.fd, wr->buffer + wr->readed, LOG_MAX_SIZE - wr->readed); + r = read(wr->fd_entity.fd, wr->buffer + wr->readed, sizeof wr->buffer - wr->readed); if ((r == 0 || r == -1) && event->events & EPOLLHUP) return -EINVAL; @@ -1521,7 +1521,7 @@ static int service_writer_socket(struct logger* server, struct writer* wr, struc if (r <= 0) return r; - r = read(wr->fd_entity.fd, wr->buffer + wr->readed, LOG_MAX_SIZE - wr->readed); + r = read(wr->fd_entity.fd, wr->buffer + wr->readed, sizeof wr->buffer - wr->readed); } while (r > 0 || ((wr->readed >= sizeof(msg->length) && wr->readed >= msg->length))); return (r >= 0 || errno == EAGAIN) ? 0 : r; @@ -1538,7 +1538,7 @@ static int service_writer_socket(struct logger* server, struct writer* wr, struc static int service_writer_pipe(struct logger *server, struct writer *wr, struct epoll_event *event) { if (event->events & EPOLLIN) { - int r = read(wr->fd_entity.fd, wr->buffer + wr->readed, LOG_MAX_SIZE - wr->readed); + int r = read(wr->fd_entity.fd, wr->buffer + wr->readed, sizeof wr->buffer - wr->readed); if (r == -1 && errno == EAGAIN) return 0; @@ -1551,7 +1551,8 @@ static int service_writer_pipe(struct logger *server, struct writer *wr, struct struct pipe_logger_entry *const ple = (struct pipe_logger_entry *const)wr->buffer; while ((wr->readed >= sizeof(ple->len)) && (ple->len <= wr->readed)) { - if (ple->len < sizeof(struct pipe_logger_entry) || ple->len > LOG_MAX_SIZE) + const int payload_size = ple->len - sizeof *ple; + if (payload_size < 0 || payload_size > LOG_MAX_PAYLOAD_SIZE) return -EINVAL; struct logger_entry_with_msg lem; @@ -1559,7 +1560,7 @@ static int service_writer_pipe(struct logger *server, struct writer *wr, struct add_recv_timestamp(&lem.header); buffer_append(&lem.header, wr->buf_ptr); wr->readed -= ple->len; - memmove(wr->buffer, wr->buffer + ple->len, LOG_MAX_SIZE - ple->len); + memmove(wr->buffer, wr->buffer + ple->len, sizeof wr->buffer - ple->len); } } else if (event->events & EPOLLHUP) return -EBADF; @@ -1582,7 +1583,7 @@ static int service_writer_kmsg(struct logger* server, struct writer* wr, struct if (!(event->events & EPOLLIN)) return 0; - int r = read(wr->fd_entity.fd, wr->buffer, LOG_MAX_SIZE - 1); + int r = read(wr->fd_entity.fd, wr->buffer, sizeof wr->buffer - 1); if (r == -1 && (errno == EAGAIN || errno == EPIPE)) return 0; @@ -1673,7 +1674,7 @@ static int service_writer_syslog(struct logger* server, struct writer* wr, struc if (!(event->events & EPOLLIN)) return 0; - int r = read(wr->fd_entity.fd, wr->buffer, LOG_MAX_SIZE - 1); + int r = read(wr->fd_entity.fd, wr->buffer, sizeof wr->buffer - 1); if (r == -1 && (errno == EAGAIN || errno == EPIPE)) return 0; @@ -1686,15 +1687,14 @@ static int service_writer_syslog(struct logger* server, struct writer* wr, struc wr->buffer[r] = '\0'; - char buffer[LOG_MAX_SIZE]; - if (parse_syslog_datagram(wr->buffer, r + 1, (struct logger_entry *) buffer)) { + struct logger_entry_with_msg lem; + if (parse_syslog_datagram(wr->buffer, r + 1, &lem.header)) { /* don't return parse error as writers are removed then */ return 0; } - struct logger_entry * entry = (struct logger_entry*) buffer; - add_recv_timestamp(entry); - buffer_append(entry, wr->buf_ptr); + add_recv_timestamp(&lem.header); + buffer_append(&lem.header, wr->buf_ptr); return 0; } diff --git a/src/logutil/fdi_logger.c b/src/logutil/fdi_logger.c index 43648f9..03dc1f1 100644 --- a/src/logutil/fdi_logger.c +++ b/src/logutil/fdi_logger.c @@ -151,7 +151,7 @@ static int logger_read(struct fd_info *fdi) assert(lpd); assert(!lpd->entry); - char buff[LOG_MAX_SIZE]; + char buff[LOG_MAX_PAYLOAD_SIZE + sizeof(struct android_logger_entry)]; int r = read(fdi->fd, buff, sizeof buff); if (r < 0) return -errno; diff --git a/src/shared/queued_entry.c b/src/shared/queued_entry.c index e328b52..4c3e0e5 100644 --- a/src/shared/queued_entry.c +++ b/src/shared/queued_entry.c @@ -47,7 +47,7 @@ void create_pipe_message(void *buf, int prio, char const *tag, char const *msg) assert(tag); assert(prio >= DLOG_VERBOSE); assert(prio < DLOG_PRIO_MAX); - assert(strlen(tag) + strlen(msg) + sizeof(struct pipe_logger_entry) + 2 < LOG_MAX_SIZE); + assert(1 /* priority */ + strlen(tag) + 1 /* NULL delimiter */ + strlen(msg) + 1 /* NULL terminator */ <= LOG_MAX_PAYLOAD_SIZE); struct pipe_logger_entry *ple = (struct pipe_logger_entry *) buf; struct timespec ts; @@ -79,7 +79,7 @@ void create_pipe_message(void *buf, int prio, char const *tag, char const *msg) */ int parse_kmsg_message(char * buffer, int buffer_size) { - static char buffer_temp[LOG_MAX_SIZE]; + static char buffer_temp[LOG_MAX_PAYLOAD_SIZE]; struct logger_entry le; unsigned long long timestamp; char * tag_begin, * pri_begin, * msg_begin; @@ -143,8 +143,8 @@ int parse_kmsg_message(char * buffer, int buffer_size) prio = strtol(pri_begin, NULL, 10); struct logger_entry * buf_le = (struct logger_entry *) buffer; - buf_le->len = 1 + snprintf(buffer_temp, LOG_MAX_SIZE - 1, "%c%s", prio, tag_begin); - buf_le->len += 1 + snprintf(buffer_temp + buf_le->len, LOG_MAX_SIZE - 1 - buf_le->len, "%s", msg_begin); + buf_le->len = 1 + snprintf(buffer_temp, sizeof buffer_temp - 1, "%c%s", prio, tag_begin); + buf_le->len += 1 + snprintf(buffer_temp + buf_le->len, sizeof buffer_temp - 1 - buf_le->len, "%s", msg_begin); memcpy(buf_le->msg, buffer_temp, buf_le->len); buf_le->len += sizeof(struct logger_entry); @@ -166,6 +166,7 @@ int parse_kmsg_message(char * buffer, int buffer_size) void parse_androidlogger_message(struct android_logger_entry *ale, struct logger_entry *le, size_t dgram_size) { const size_t payload_size = dgram_size - sizeof *ale; + assert(payload_size <= LOG_MAX_PAYLOAD_SIZE); le->len = sizeof *le + payload_size; le->pid = ale->pid; diff --git a/src/tests/pipe_message.c b/src/tests/pipe_message.c index 4d9aeb4..515b1b6 100644 --- a/src/tests/pipe_message.c +++ b/src/tests/pipe_message.c @@ -21,8 +21,8 @@ int main() int prio = 5; char tag[] = "TEST_TAG"; char msg[] = "Test message with a newline \n and some special characters \e[31m in colour"; - struct pipe_logger_entry *ple = alloca(LOG_MAX_SIZE); - struct logger_entry *le = alloca(LOG_MAX_SIZE); + struct pipe_logger_entry *ple = alloca(LOG_MAX_PAYLOAD_SIZE + sizeof *ple); + struct logger_entry *le = alloca(LOG_MAX_PAYLOAD_SIZE + sizeof *le); struct timespec ts_pre, ts_post; clock_gettime(CLOCK_MONOTONIC, &ts_pre); @@ -38,7 +38,7 @@ int main() assert(time_cmp(ts_pre, le->sec, le->nsec) <= 0); assert(time_cmp(ts_post, le->sec, le->nsec) >= 0); - static const int SAFE_SIZE = (LOG_MAX_SIZE - sizeof(struct logger_entry) - 2) / 2; + static const int SAFE_SIZE = ((LOG_MAX_PAYLOAD_SIZE - 1 /* prio */) / 2 /* split over tag and msg */) - 1 /* null delimiter */; char random_tag[SAFE_SIZE]; char random_msg[SAFE_SIZE]; random_tag[SAFE_SIZE - 1] = random_msg[SAFE_SIZE - 1] = '\0'; diff --git a/tests/test_libdlog.c b/tests/test_libdlog.c index 7e25fcc..3c6da55 100644 --- a/tests/test_libdlog.c +++ b/tests/test_libdlog.c @@ -21,21 +21,20 @@ void *func(void *data) int main(int argc, char **argv) { if (argc == 1) { - char huge_buffer[LOG_MAX_SIZE * 3]; - char buffer[LOG_MAX_SIZE]; + char huge_buffer[LOG_MAX_PAYLOAD_SIZE * 3]; + char buffer[LOG_MAX_PAYLOAD_SIZE - 20 /* leeway for tag, prio etc */]; int i; pthread_t threads[50]; // check buffer overflow - memset(huge_buffer, 'a', LOG_MAX_SIZE); - memset(huge_buffer + LOG_MAX_SIZE, 'b', LOG_MAX_SIZE); - memset(huge_buffer + LOG_MAX_SIZE * 2, 'c', LOG_MAX_SIZE); - huge_buffer[LOG_MAX_SIZE * 3 - 1] = '\0'; + for (int i = 0; i < 3; ++i) + memset(huge_buffer + i * LOG_MAX_PAYLOAD_SIZE, 'a' + i, LOG_MAX_PAYLOAD_SIZE); + huge_buffer[sizeof huge_buffer - 1] = '\0'; LOGE("%s", huge_buffer); // check garbage data - for (i = 0; i < LOG_MAX_SIZE; ++i) + for (i = 0; i < sizeof buffer; ++i) buffer[i] = rand() & 255; LOGE("%s", buffer);