diff options
author | Benjamin Herrenschmidt <benh@kernel.crashing.org> | 2014-12-09 13:45:39 +1100 |
---|---|---|
committer | Stewart Smith <stewart@linux.vnet.ibm.com> | 2014-12-10 11:50:33 +1100 |
commit | 92d638c6eb622e8417582eab4ef2136ee04560a6 (patch) | |
tree | 6e6308bd9772d3dc50a869a3bd6b3f06f7f83394 | |
parent | 55a59b944a913832e54b4818b41046bda0b6be66 (diff) | |
download | skiboot-92d638c6eb622e8417582eab4ef2136ee04560a6.zip skiboot-92d638c6eb622e8417582eab4ef2136ee04560a6.tar.gz skiboot-92d638c6eb622e8417582eab4ef2136ee04560a6.tar.bz2 |
fsp-elog: Add various NULL checks
Recent gcc 4.9 will silently add conditional traps for NULL checks in
cases where it thinks we may dereference a NULL pointer. It seems to
be pretty keen on doing so when we dereference the result of list_top.
Most of the time, these aren't bugs because we have some other state
variable that tells us whether our list contains something or not but
we hit a bug in the FSP code recently where that was getting out of
sync, and the result of a trap in real mode isn't pretty ....
So this adds explicit NULL checks in a number of place where gcc added
trap instructions. With this patch, the current tree doesn't generate
any. I didn't find a way to make gcc warn unfortunately.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
-rw-r--r-- | hw/fsp/fsp-elog-read.c | 32 | ||||
-rw-r--r-- | hw/fsp/fsp-elog-write.c | 25 |
2 files changed, 46 insertions, 11 deletions
diff --git a/hw/fsp/fsp-elog-read.c b/hw/fsp/fsp-elog-read.c index ede958c..435612e 100644 --- a/hw/fsp/fsp-elog-read.c +++ b/hw/fsp/fsp-elog-read.c @@ -173,10 +173,16 @@ static void fsp_elog_fetch_failure(uint8_t fsp_status) /* read top list and delete the node */ log_data = list_top(&elog_read_pending, struct fsp_log_entry, link); - list_del(&log_data->link); - list_add(&elog_read_free, &log_data->link); - prerror("ELOG: received invalid data: %x FSP status: 0x%x\n", - log_data->log_id, fsp_status); + if (!log_data) { + prlog(PR_ERR, "%s: Inconsistent internal list state !\n", + __func__); + } else { + list_del(&log_data->link); + list_add(&elog_read_free, &log_data->link); + prerror("ELOG: received invalid data: %x FSP status: 0x%x\n", + log_data->log_id, fsp_status); + + } fsp_elog_set_head_state(ELOG_STATE_NONE); } @@ -228,6 +234,12 @@ static void fsp_elog_queue_fetch(void) struct fsp_log_entry *entry; entry = list_top(&elog_read_pending, struct fsp_log_entry, link); + if (!entry) { + prlog(PR_ERR, "%s: Inconsistent internal list state !\n", + __func__); + fsp_elog_set_head_state(ELOG_STATE_NONE); + return; + } fsp_elog_set_head_state(ELOG_STATE_FETCHING); elog_head_id = entry->log_id; elog_head_size = entry->log_size; @@ -260,6 +272,12 @@ static int64_t fsp_opal_elog_info(uint64_t *opal_elog_id, return OPAL_WRONG_STATE; } log_data = list_top(&elog_read_pending, struct fsp_log_entry, link); + if (!log_data) { + prlog(PR_ERR, "%s: Inconsistent internal list state !\n", + __func__); + unlock(&elog_read_lock); + return OPAL_WRONG_STATE; + } *opal_elog_id = log_data->log_id; *opal_elog_size = log_data->log_size; unlock(&elog_read_lock); @@ -288,6 +306,12 @@ static int64_t fsp_opal_elog_read(uint64_t *buffer, uint64_t opal_elog_size, } log_data = list_top(&elog_read_pending, struct fsp_log_entry, link); + if (!log_data) { + prlog(PR_ERR, "%s: Inconsistent internal list state !\n", + __func__); + unlock(&elog_read_lock); + return OPAL_WRONG_STATE; + } /* Check log ID and then read log from buffer */ if (opal_elog_id != log_data->log_id) { diff --git a/hw/fsp/fsp-elog-write.c b/hw/fsp/fsp-elog-write.c index 3a9adda..dd71712 100644 --- a/hw/fsp/fsp-elog-write.c +++ b/hw/fsp/fsp-elog-write.c @@ -134,10 +134,17 @@ bool opal_elog_info(uint64_t *opal_elog_id, uint64_t *opal_elog_size) if (elog_write_to_host_head_state == ELOG_STATE_FETCHED_DATA) { head = list_top(&elog_write_to_host_pending, struct errorlog, link); - *opal_elog_id = head->plid; - *opal_elog_size = head->log_size; - elog_write_to_host_head_state = ELOG_STATE_FETCHED_INFO; - rc = true; + if (!head) { + prlog(PR_ERR, + "%s: Inconsistent internal list state !\n", + __func__); + elog_write_to_host_head_state = ELOG_STATE_NONE; + } else { + *opal_elog_id = head->plid; + *opal_elog_size = head->log_size; + elog_write_to_host_head_state = ELOG_STATE_FETCHED_INFO; + rc = true; + } } unlock(&elog_write_to_host_lock); return rc; @@ -165,7 +172,7 @@ static void opal_commit_elog_in_host(void) bool opal_elog_read(uint64_t *buffer, uint64_t opal_elog_size, - uint64_t opal_elog_id) + uint64_t opal_elog_id) { struct errorlog *log_data; bool rc = false; @@ -174,9 +181,13 @@ bool opal_elog_read(uint64_t *buffer, uint64_t opal_elog_size, if (elog_write_to_host_head_state == ELOG_STATE_FETCHED_INFO) { log_data = list_top(&elog_write_to_host_pending, struct errorlog, link); - + if (!log_data) { + elog_write_to_host_head_state = ELOG_STATE_NONE; + unlock(&elog_write_to_host_lock); + return rc; + } if ((opal_elog_id != log_data->plid) && - (opal_elog_size != log_data->log_size)) { + (opal_elog_size != log_data->log_size)) { unlock(&elog_write_to_host_lock); return rc; } |