| Message ID | be0c5e442b5c0d29c136e802e8f4552d85955c32.1585173194.git.leonard.crestez@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] firmware: imx: scu: Fix corruption of header | expand |
> From: Leonard Crestez <leonard.crestez@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 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> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > Cc: stable@vger.kernel.org Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com> Regards Aisheng > > --- > Changes since v1: > * Add my signed-off-by as requested: > Link to v1: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw > ork.kernel.org%2Fpatch%2F11394401%2F&data=02%7C01%7Caisheng.d > ong%40nxp.com%7C67d37a0e46c4473ec6f808d7d107e780%7C686ea1d3bc > 2b4c6fa92cd99c5c301635%7C0%7C0%7C637207704182589926&sdata > =Qzj2kFXpN%2FfO%2BgclQ1RcQDmwlLUTuLdg78WxWgTy4cc%3D&reser > ved=0 > > There is another longer series attempting a fix but that probably won't make it > into stable: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw > ork.kernel.org%2Fcover%2F11446661%2F&data=02%7C01%7Caisheng.d > ong%40nxp.com%7C67d37a0e46c4473ec6f808d7d107e780%7C686ea1d3bc > 2b4c6fa92cd99c5c301635%7C0%7C0%7C637207704182599918&sdata > =mBJ8fX4DzU0JArPobbCf1GRvupEo0uL9hV4Htf6kN3I%3D&reserved=0 > > That series implements a different imx mailbox type but still maintains the old > path for DT compatibility so this fix is worth including. It fixes real boot hangs. > > drivers/firmware/imx/imx-scu.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c > index f71eaa5bf52d..7119228a8059 100644 > --- a/drivers/firmware/imx/imx-scu.c > +++ b/drivers/firmware/imx/imx-scu.c > @@ -141,24 +141,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]; > > /* > * SCU requires that all messages words are written > * sequentially but linux MU driver implements multiple > -- > 2.17.1
On Thu, Mar 26, 2020 at 12:00:05AM +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 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> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > Cc: stable@vger.kernel.org Applied, thanks.
diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c index f71eaa5bf52d..7119228a8059 100644 --- a/drivers/firmware/imx/imx-scu.c +++ b/drivers/firmware/imx/imx-scu.c @@ -141,24 +141,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]; /* * SCU requires that all messages words are written * sequentially but linux MU driver implements multiple