[PATCH] firmware: imx: scu: Fix corruption of header

[PATCH] firmware: imx: scu: Fix corruption of header

[Leonard Crestez]

From: Franck LENORMAND <franck.lenormand@nxp.com>

The header of the message to send can be changed if the
response is longer than the request:
 - 1st word, the header is sent
 - the remaining words of the message are sent
 - the response is received asynchronously during the
   execution of the loop, changing the size field in
   the header
 - the for loop test the termination condition using
   the corrupted header

It is the case for the API build_info which has just a
header as request but 3 words in response.

This issue is fixed by storing the header locally instead of
using a pointer on it.

Fixes: edbee095fafb (firmware: imx: add SCU firmware driver support)
Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Cc: stable@vger.kernel.org
---
 drivers/firmware/imx/imx-scu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

This can manifest as random crashes so Cc: stable

diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
index 03b43b7a6d1d..2cf09f8a075c 100644
--- a/drivers/firmware/imx/imx-scu.c
+++ b/drivers/firmware/imx/imx-scu.c
@@ -132,24 +132,24 @@ static void imx_scu_rx_callback(struct mbox_client *c, void *msg)
 		complete(&sc_ipc->done);
 }
 
 static int imx_scu_ipc_write(struct imx_sc_ipc *sc_ipc, void *msg)
 {
-	struct imx_sc_rpc_msg *hdr = msg;
+	struct imx_sc_rpc_msg hdr = *(struct imx_sc_rpc_msg *)msg;
 	struct imx_sc_chan *sc_chan;
 	u32 *data = msg;
 	int ret;
 	int i;
 
 	/* Check size */
-	if (hdr->size > IMX_SC_RPC_MAX_MSG)
+	if (hdr.size > IMX_SC_RPC_MAX_MSG)
 		return -EINVAL;
 
-	dev_dbg(sc_ipc->dev, "RPC SVC %u FUNC %u SIZE %u\n", hdr->svc,
-		hdr->func, hdr->size);
+	dev_dbg(sc_ipc->dev, "RPC SVC %u FUNC %u SIZE %u\n", hdr.svc,
+		hdr.func, hdr.size);
 
-	for (i = 0; i < hdr->size; i++) {
+	for (i = 0; i < hdr.size; i++) {
 		sc_chan = &sc_ipc->chans[i % 4];
 		ret = mbox_send_message(sc_chan->ch, &data[i]);
 		if (ret < 0)
 			return ret;
 	}
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] firmware: imx: scu: Fix corruption of header

[Shawn Guo]

On Thu, Feb 20, 2020 at 06:12:22PM +0200, Leonard Crestez wrote:
> From: Franck LENORMAND <franck.lenormand@nxp.com>
> 
> The header of the message to send can be changed if the
> response is longer than the request:
>  - 1st word, the header is sent
>  - the remaining words of the message are sent
>  - the response is received asynchronously during the
>    execution of the loop, changing the size field in
>    the header
>  - the for loop test the termination condition using
>    the corrupted header
> 
> It is the case for the API build_info which has just a
> header as request but 3 words in response.
> 
> This issue is fixed by storing the header locally instead of
> using a pointer on it.
> 
> Fixes: edbee095fafb (firmware: imx: add SCU firmware driver support)
> Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>

When you forward a patch from someone else, it should have your SoB.

Shawn

> Cc: stable@vger.kernel.org
> ---
>  drivers/firmware/imx/imx-scu.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> This can manifest as random crashes so Cc: stable
> 
> diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
> index 03b43b7a6d1d..2cf09f8a075c 100644
> --- a/drivers/firmware/imx/imx-scu.c
> +++ b/drivers/firmware/imx/imx-scu.c
> @@ -132,24 +132,24 @@ static void imx_scu_rx_callback(struct mbox_client *c, void *msg)
>  		complete(&sc_ipc->done);
>  }
>  
>  static int imx_scu_ipc_write(struct imx_sc_ipc *sc_ipc, void *msg)
>  {
> -	struct imx_sc_rpc_msg *hdr = msg;
> +	struct imx_sc_rpc_msg hdr = *(struct imx_sc_rpc_msg *)msg;
>  	struct imx_sc_chan *sc_chan;
>  	u32 *data = msg;
>  	int ret;
>  	int i;
>  
>  	/* Check size */
> -	if (hdr->size > IMX_SC_RPC_MAX_MSG)
> +	if (hdr.size > IMX_SC_RPC_MAX_MSG)
>  		return -EINVAL;
>  
> -	dev_dbg(sc_ipc->dev, "RPC SVC %u FUNC %u SIZE %u\n", hdr->svc,
> -		hdr->func, hdr->size);
> +	dev_dbg(sc_ipc->dev, "RPC SVC %u FUNC %u SIZE %u\n", hdr.svc,
> +		hdr.func, hdr.size);
>  
> -	for (i = 0; i < hdr->size; i++) {
> +	for (i = 0; i < hdr.size; i++) {
>  		sc_chan = &sc_ipc->chans[i % 4];
>  		ret = mbox_send_message(sc_chan->ch, &data[i]);
>  		if (ret < 0)
>  			return ret;
>  	}
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel