[Skiboot] [PATCH] fsp-elog: Add various NULL checks

[Benjamin Herrenschmidt]

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 at kernel.crashing.org>
---

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;
 		}