From patchwork Sat Jan 14 22:42:24 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-Patchwork-Submitter: Michael Neuling <mikey@neuling.org>
X-Patchwork-Id: 715395
Return-Path: <skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>
X-Original-To: incoming@patchwork.ozlabs.org
Delivered-To: patchwork-incoming@bilbo.ozlabs.org
Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])
	(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by ozlabs.org (Postfix) with ESMTPS id 3v1Dys60zdz9t0P
	for <incoming@patchwork.ozlabs.org>;
	Sun, 15 Jan 2017 09:42:29 +1100 (AEDT)
Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])
	by lists.ozlabs.org (Postfix) with ESMTP id 3v1Dys4HC1zDqXd
	for <incoming@patchwork.ozlabs.org>;
	Sun, 15 Jan 2017 09:42:29 +1100 (AEDT)
X-Original-To: skiboot@lists.ozlabs.org
Delivered-To: skiboot@lists.ozlabs.org
Received: from ozlabs.org (ozlabs.org [IPv6:2401:3900:2:1::2])
	(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by lists.ozlabs.org (Postfix) with ESMTPS id 3v1Dyn6ZMZzDqVQ
	for <skiboot@lists.ozlabs.org>; Sun, 15 Jan 2017 09:42:25 +1100 (AEDT)
Received: from localhost.localdomain (localhost [127.0.0.1])
	by ozlabs.org (Postfix) with ESMTP id 3v1Dyn3WHJz9t0P;
	Sun, 15 Jan 2017 09:42:25 +1100 (AEDT)
Received: by localhost.localdomain (Postfix, from userid 1000)
	id C1893EEE620; Sat, 14 Jan 2017 16:42:24 -0600 (CST)
Message-ID: <1484433744.12241.99.camel@neuling.org>
From: Michael Neuling <mikey@neuling.org>
To: Cyril Bur <cyril.bur@au1.ibm.com>, skiboot@lists.ozlabs.org
Date: Sat, 14 Jan 2017 16:42:24 -0600
In-Reply-To: <20170112052619.16670-2-cyril.bur@au1.ibm.com>
References: <20170112052619.16670-1-cyril.bur@au1.ibm.com>
	<20170112052619.16670-2-cyril.bur@au1.ibm.com>
X-Mailer: Evolution 3.22.3-0ubuntu0.1 
Mime-Version: 1.0
Subject: Re: [Skiboot] [RFC PATCH 1/3] hw/lpc-mbox: Add skiboot drivers for
	the BMC mbox regs
X-BeenThere: skiboot@lists.ozlabs.org
X-Mailman-Version: 2.1.23
Precedence: list
List-Id: Mailing list for skiboot development <skiboot.lists.ozlabs.org>
List-Unsubscribe: <https://lists.ozlabs.org/options/skiboot>,
	<mailto:skiboot-request@lists.ozlabs.org?subject=unsubscribe>
List-Archive: <http://lists.ozlabs.org/pipermail/skiboot/>
List-Post: <mailto:skiboot@lists.ozlabs.org>
List-Help: <mailto:skiboot-request@lists.ozlabs.org?subject=help>
List-Subscribe: <https://lists.ozlabs.org/listinfo/skiboot>,
	<mailto:skiboot-request@lists.ozlabs.org?subject=subscribe>
Cc: alistair@popple.id.au
Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org
Sender: "Skiboot"
	<skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>

> +int bmc_mbox_enqueue(struct bmc_mbox_msg *msg)
> +{
> +	int rc = 0;

Can you add a check here to ensure mbox.base is set?  

I've hit a few cases in testing new platforms where bmc_mbox_enqueue() get
called from mbox_flash_init() but mbox_init() hasn't been run.  I end up with a
very odd crash when mbox.base gets de-referenced and starts corrupting random
bits of memory.

Something like:

diff --git a/hw/lpc-mbox.c b/hw/lpc-mbox.c
index 0ffc5e97d6..7818943864 100644
--- a/hw/lpc-mbox.c
+++ b/hw/lpc-mbox.c
@@ -118,6 +118,11 @@ static void bmc_mbox_send_message(struct bmc_mbox_msg *msg)
 int bmc_mbox_enqueue(struct bmc_mbox_msg *msg)
 {
        int rc = 0;
+
+       /* In case mbox is not inited */
+       if (!mbox.base)
+               return -1;
+
        lock(&mbox.lock);
        if (mbox.queue_len == MBOX_MAX_QUEUE_LEN) {
                rc = -1;