From 835d0c737a7e04f30b3990249ce461999ea2cf3a Mon Sep 17 00:00:00 2001 From: Solomon Peachy Date: Fri, 6 Aug 2021 09:56:22 -0400 Subject: [PATCH] logf: Fix two issues with logf_panic_dump() * It had a (read) buffer overflow when dumping the stuff on the back half of the buffer * a highly questionable code construct was nuked Change-Id: I7f6f119524fc2095f788fc9b3d356459955d3ace --- firmware/export/logf.h | 2 +- firmware/logf.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/firmware/export/logf.h b/firmware/export/logf.h index c8aaad06b4..7fbe5976a4 100644 --- a/firmware/export/logf.h +++ b/firmware/export/logf.h @@ -31,7 +31,7 @@ #define MAX_LOGF_SIZE 16384 -extern unsigned char logfbuffer[MAX_LOGF_SIZE]; +extern unsigned char logfbuffer[MAX_LOGF_SIZE + 1]; extern int logfindex; extern bool logfwrap; extern bool logfenabled; diff --git a/firmware/logf.c b/firmware/logf.c index 11ffa4e15c..e71425f2c3 100644 --- a/firmware/logf.c +++ b/firmware/logf.c @@ -61,7 +61,7 @@ static int logdiskfindex; #ifdef ROCKBOX_HAS_LOGF #ifndef __PCTOOL__ -unsigned char logfbuffer[MAX_LOGF_SIZE]; +unsigned char logfbuffer[MAX_LOGF_SIZE + 1]; int logfindex; bool logfwrap; bool logfenabled = true; @@ -272,10 +272,14 @@ void logf_panic_dump(int *y) return; } + /* Explicitly null-terminate our buffer */ + logfbuffer[MAX_LOGF_SIZE] = 0; + lcd_puts(1, (*y)++, "start of logf data"); lcd_update(); - i = logfindex - 2; /* The last actual characer (i.e. not '\0') */ + /* The intent is to dump the newest log entries first! */ + i = logfindex - 2; /* The last actual characer (i.e. not '\0') */ while(i >= 0) { while(logfbuffer[i] != 0 && i>=0) @@ -300,12 +304,13 @@ void logf_panic_dump(int *y) } if(strlen( &logfbuffer[i + 1]) > 0) { - lcd_putsf(1, (*y)++, "%*s", (MAX_LOGF_SIZE-i) &logfbuffer[i + 1]); + lcd_putsf(1, (*y)++, "%*s", &logfbuffer[i + 1]); lcd_update(); } } i--; } + lcd_puts(1, (*y)++, "end of logf data"); lcd_update(); }