From f2a006fd98045066bdf429cc142d033e9feb0a8f Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Wed, 28 Jul 2010 09:31:09 -0700 Subject: [PATCH] Make ao_log_data re-entrant as it is used for both sensor and GPS logs Because ao_log_data is called from two different threads, failing to make it re-entrant would cause the 'log' pointer parameter to get overwritten if another thread asked to log data while the eeprom was busy writing out a block. This would cause the second thread to re-writing data from the first thread's address, but without re-checksumming the data as the checksum is computed before the log mutex is taken. The bug can be seen by log blocks with invalid checksums. Here's what happens with the ao_gps_tracking_report and ao_log threads: ao_gps_tracking_report ao_log Writes a bunch of records *blocks* in the eeprom flush sets ao_log_data 'log' to global 'log' computes checksum for 'log' block *blocks* on ao_log_mutex Wakes up sets ao_log_data 'log' to 'gps_log' writes remaining records 'gps_log' is left with svid = 0 *blocks* on ao_gps_tracking_data writes data, reading from the current ao_log_data 'log' pointer which points at 'gps_log' Making ao_log_data re-entrant fixes this by ensuring that the 'ao_log' thread has its own copy of the ao_log_data 'log' parameter. I made this function take an __xdata restricted pointer so that it could be passed in the dptr register instead of needing to go on the stack. Signed-off-by: Keith Packard --- src/ao.h | 2 +- src/ao_log.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ao.h b/src/ao.h index dfff8a8d..5dd756da 100644 --- a/src/ao.h +++ b/src/ao.h @@ -562,7 +562,7 @@ struct ao_log_record { /* Write a record to the eeprom log */ void -ao_log_data(struct ao_log_record *log); +ao_log_data(__xdata struct ao_log_record *log) __reentrant; /* Flush the log */ void diff --git a/src/ao_log.c b/src/ao_log.c index 44ce90e0..d550d408 100644 --- a/src/ao_log.c +++ b/src/ao_log.c @@ -23,7 +23,7 @@ static __xdata uint8_t ao_log_running; static __xdata uint8_t ao_log_mutex; static uint8_t -ao_log_csum(uint8_t *b) +ao_log_csum(__xdata uint8_t *b) __reentrant { uint8_t sum = 0x5a; uint8_t i; @@ -34,11 +34,11 @@ ao_log_csum(uint8_t *b) } void -ao_log_data(struct ao_log_record *log) +ao_log_data(__xdata struct ao_log_record *log) __reentrant { /* set checksum */ log->csum = 0; - log->csum = ao_log_csum((uint8_t *) log); + log->csum = ao_log_csum((__xdata uint8_t *) log); ao_mutex_get(&ao_log_mutex); { if (ao_log_running) { ao_ee_write(ao_log_current_pos, -- 2.30.2