aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBenjamin Herrenschmidt <benh@kernel.crashing.org>2014-12-09 13:45:39 +1100
committerStewart Smith <stewart@linux.vnet.ibm.com>2014-12-10 11:50:33 +1100
commit92d638c6eb622e8417582eab4ef2136ee04560a6 (patch)
tree6e6308bd9772d3dc50a869a3bd6b3f06f7f83394
parent55a59b944a913832e54b4818b41046bda0b6be66 (diff)
downloadskiboot-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.c32
-rw-r--r--hw/fsp/fsp-elog-write.c25
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;
}