From 775e281fb6c19350f5c65e83e7c4b768c67bbe44 Mon Sep 17 00:00:00 2001 From: Jack Pham Date: Thu, 30 Mar 2017 09:53:53 -0700 Subject: trace: ipc_logging: Avoid buffer overflow in ipc_log_string() In ipc_log_string() the return value from vsnprintf(), data_size, is used to increment ectxt.offset. However, this length could actually be much larger than that of ectxt.buff itself. This is a typical mistake of [v]snprintf() usage [1], in that it returns not the number of characters written but how many characters *would* have been written regardless of whether it was truncated. The result is that even though ectxt.buff itself is not overrun, the incorrect size in ectxt.offset will be later used as the length parameter when memcpy()'ing to the ipc_log_page's data, overflowing that memory and beyond. The write_page's write_offset would also indicate an out-of-bounds (greater than PAGE_SIZE) length. The fix is simple: use vscnprintf() instead of vsnprintf(). [1] https://lwn.net/Articles/69419/ Change-Id: I2e9d44e74f5f30a009732e31a554d82e31946999 Signed-off-by: Jack Pham --- kernel/trace/ipc_logging.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/trace/ipc_logging.c b/kernel/trace/ipc_logging.c index 2c3e0998d400..ed29c38cd7fb 100644 --- a/kernel/trace/ipc_logging.c +++ b/kernel/trace/ipc_logging.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2012-2016, The Linux Foundation. All rights reserved. +/* Copyright (c) 2012-2017, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -507,8 +507,8 @@ int ipc_log_string(void *ilctxt, const char *fmt, ...) tsv_qtimer_write(&ectxt); avail_size = (MAX_MSG_SIZE - (ectxt.offset + hdr_size)); va_start(arg_list, fmt); - data_size = vsnprintf((ectxt.buff + ectxt.offset + hdr_size), - avail_size, fmt, arg_list); + data_size = vscnprintf((ectxt.buff + ectxt.offset + hdr_size), + avail_size, fmt, arg_list); va_end(arg_list); tsv_write_header(&ectxt, TSV_TYPE_BYTE_ARRAY, data_size); ectxt.offset += data_size; -- cgit v1.2.3