| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFHUOYxSegkbX61ne6TP41oeEZtzxzmyRnbi2UkDFvtyvsNcQw@mail.gmail.com>
Date: Thu, 8 Sep 2016 08:33:07 -0700
From: Hoan Tran <hotran@....com>
To: Guenter Roeck <linux@...ck-us.net>,
Jean Delvare <jdelvare@...e.com>
Cc: linux-hwmon@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>,
Itaru Kitayama <itaru.kitayama@...en.jp>, Loc Ho <lho@....com>,
Duc Dang <dhdang@....com>, Hoan Tran <hotran@....com>
Subject: Re: [PATCH v2] hwmon: xgene: Fix crash when alarm occurs before
driver probe
Hi All,
Please discard this patch. I'll resend another one soon.
Thanks
Hoan
On Thu, Sep 8, 2016 at 8:31 AM, Hoan Tran <hotran@....com> wrote:
> The system crashes during probing xgene-hwmon driver when temperature
> alarm interrupt occurs before.
> It's because
> - xgene_hwmon_probe() requests mailbox channel which also enables
> the mailbox interrupt.
> - As temperature alarm interrupt is pending, ISR runs and crashes when accesses
> into invalid resourse as unmapped PCC shared memory.
>
> This patch fixes this issue by saving this alarm message and scheduling a
> bottom handler after xgene_hwmon_probe() finish.
>
> Signed-off-by: Hoan Tran <hotran@....com>
> Reported-by: Itaru Kitayama <itaru.kitayama@...en.jp>
> ---
> v2
> * Check hwmon_dev and resp_pending to determine the driver is not ready
> * Remove init_flag and rx_pending
> * Replace EBUSY by ENODEV
>
> v1
> * Initial
>
> drivers/hwmon/xgene-hwmon.c | 69 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
> index bc78a5d..ea53537 100644
> --- a/drivers/hwmon/xgene-hwmon.c
> +++ b/drivers/hwmon/xgene-hwmon.c
> @@ -465,13 +465,34 @@ static void xgene_hwmon_evt_work(struct work_struct *work)
> }
> }
>
> +static int xgene_hwmon_rx_ready(struct xgene_hwmon_dev *ctx, void *msg)
> +{
> + if (IS_ERR(ctx->hwmon_dev) && !ctx->resp_pending) {
> + /* Enqueue to the FIFO */
> + kfifo_in_spinlocked(&ctx->async_msg_fifo, msg,
> + sizeof(struct slimpro_resp_msg),
> + &ctx->kfifo_lock);
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> /*
> * This function is called when the SLIMpro Mailbox received a message
> */
> static void xgene_hwmon_rx_cb(struct mbox_client *cl, void *msg)
> {
> struct xgene_hwmon_dev *ctx = to_xgene_hwmon_dev(cl);
> - struct slimpro_resp_msg amsg;
> +
> + /*
> + * While the driver registers with the mailbox framework, an interrupt
> + * can be pending before the probe function completes its
> + * initialization. If such condition occurs, just queue up the message
> + * as the driver is not ready for servicing the callback.
> + */
> + if (xgene_hwmon_rx_ready(ctx, msg) < 0)
> + return;
>
> /*
> * Response message format:
> @@ -500,12 +521,8 @@ static void xgene_hwmon_rx_cb(struct mbox_client *cl, void *msg)
> return;
> }
>
> - amsg.msg = ((u32 *)msg)[0];
> - amsg.param1 = ((u32 *)msg)[1];
> - amsg.param2 = ((u32 *)msg)[2];
> -
> /* Enqueue to the FIFO */
> - kfifo_in_spinlocked(&ctx->async_msg_fifo, &amsg,
> + kfifo_in_spinlocked(&ctx->async_msg_fifo, msg,
> sizeof(struct slimpro_resp_msg), &ctx->kfifo_lock);
> /* Schedule the bottom handler */
> schedule_work(&ctx->workq);
> @@ -520,6 +537,15 @@ static void xgene_hwmon_pcc_rx_cb(struct mbox_client *cl, void *msg)
> struct acpi_pcct_shared_memory *generic_comm_base = ctx->pcc_comm_addr;
> struct slimpro_resp_msg amsg;
>
> + /*
> + * While the driver registers with the mailbox framework, an interrupt
> + * can be pending before the probe function completes its
> + * initialization. If such condition occurs, just queue up the message
> + * as the driver is not ready for servicing the callback.
> + */
> + if (xgene_hwmon_rx_ready(ctx, &amsg) < 0)
> + return;
> +
> msg = generic_comm_base + 1;
> /* Check if platform sends interrupt */
> if (!xgene_word_tst_and_clr(&generic_comm_base->status,
> @@ -596,6 +622,17 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, ctx);
> cl = &ctx->mbox_client;
>
> + spin_lock_init(&ctx->kfifo_lock);
> + mutex_init(&ctx->rd_mutex);
> +
> + rc = kfifo_alloc(&ctx->async_msg_fifo,
> + sizeof(struct slimpro_resp_msg) * ASYNC_MSG_FIFO_SIZE,
> + GFP_KERNEL);
> + if (rc)
> + goto out_mbox_free;
> +
> + INIT_WORK(&ctx->workq, xgene_hwmon_evt_work);
> +
> /* Request mailbox channel */
> cl->dev = &pdev->dev;
> cl->tx_done = xgene_hwmon_tx_done;
> @@ -676,17 +713,6 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
> ctx->usecs_lat = PCC_NUM_RETRIES * cppc_ss->latency;
> }
>
> - spin_lock_init(&ctx->kfifo_lock);
> - mutex_init(&ctx->rd_mutex);
> -
> - rc = kfifo_alloc(&ctx->async_msg_fifo,
> - sizeof(struct slimpro_resp_msg) * ASYNC_MSG_FIFO_SIZE,
> - GFP_KERNEL);
> - if (rc)
> - goto out_mbox_free;
> -
> - INIT_WORK(&ctx->workq, xgene_hwmon_evt_work);
> -
> ctx->hwmon_dev = hwmon_device_register_with_groups(ctx->dev,
> "apm_xgene",
> ctx,
> @@ -697,17 +723,22 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
> goto out;
> }
>
> + /*
> + * Schedule the bottom handler if there is a pending message.
> + */
> + schedule_work(&ctx->workq);
> +
> dev_info(&pdev->dev, "APM X-Gene SoC HW monitor driver registered\n");
>
> return 0;
>
> out:
> - kfifo_free(&ctx->async_msg_fifo);
> -out_mbox_free:
> if (acpi_disabled)
> mbox_free_channel(ctx->mbox_chan);
> else
> pcc_mbox_free_channel(ctx->mbox_chan);
> +out_mbox_free:
> + kfifo_free(&ctx->async_msg_fifo);
>
> return rc;
> }
> --
> 1.9.1
>
Powered by blists - more mailing lists