| Message ID | 3178ff7651948270b714daa4adad48b94eaca9ba.1634309856.git.michal.simek@xilinx.com |
|---|---|
| State | Accepted |
| Commit | 53f5d1688e33f4c9c1e68ba132d50f8aca06fc3b |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | [1/2] firmware: zynqmp: Handle errors from ipi_req properly | expand |
On 15.10.2021 16:57, Michal Simek wrote: > When a caller is not interested in the returned message, the ret_payload > pointer is set to NULL in the u-boot-sources. In this case, under EL3, the > memory from address 0x0 would be overwritten by ipi_req() with the returned > IPI message, damaging the original data under this address. The patch, in > case ret_payload is NULL, assigns the pointer to the array holding the IPI > message being sent. > > Signed-off-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com> > Signed-off-by: Michal Simek <michal.simek@xilinx.com> Reviewed-by: Adrian Fiergolski <Adrian.Fiergolski@fastree3d.com> Thanks, Adrian > --- > > Based on origin series from Adrian. That's why also adding his SoB line. > https://lore.kernel.org/r/20211014124349.1429696-1-adrian.fiergolski@fastree3d.com > > Adrian: The patch is just suggestion how we could avoid that NULL pointer > writes but done in ipi_req() > > --- > drivers/firmware/firmware-zynqmp.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c > index 1391aab0a160..7e498d8169e8 100644 > --- a/drivers/firmware/firmware-zynqmp.c > +++ b/drivers/firmware/firmware-zynqmp.c > @@ -30,6 +30,10 @@ static int ipi_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen) > { > struct zynqmp_ipi_msg msg; > int ret; > + u32 buffer[PAYLOAD_ARG_CNT]; > + > + if (!res) > + res = buffer; > > if (req_len > PMUFW_PAYLOAD_ARG_CNT || > res_maxlen > PMUFW_PAYLOAD_ARG_CNT)
On 10/15/21 16:57, Michal Simek wrote: > When a caller is not interested in the returned message, the ret_payload > pointer is set to NULL in the u-boot-sources. In this case, under EL3, the > memory from address 0x0 would be overwritten by ipi_req() with the returned > IPI message, damaging the original data under this address. The patch, in > case ret_payload is NULL, assigns the pointer to the array holding the IPI > message being sent. > > Signed-off-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com> > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > Based on origin series from Adrian. That's why also adding his SoB line. > https://lore.kernel.org/r/20211014124349.1429696-1-adrian.fiergolski@fastree3d.com > > Adrian: The patch is just suggestion how we could avoid that NULL pointer > writes but done in ipi_req() > > --- > drivers/firmware/firmware-zynqmp.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c > index 1391aab0a160..7e498d8169e8 100644 > --- a/drivers/firmware/firmware-zynqmp.c > +++ b/drivers/firmware/firmware-zynqmp.c > @@ -30,6 +30,10 @@ static int ipi_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen) > { > struct zynqmp_ipi_msg msg; > int ret; > + u32 buffer[PAYLOAD_ARG_CNT]; > + > + if (!res) > + res = buffer; > > if (req_len > PMUFW_PAYLOAD_ARG_CNT || > res_maxlen > PMUFW_PAYLOAD_ARG_CNT) > Applied. M
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 1391aab0a160..7e498d8169e8 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -30,6 +30,10 @@ static int ipi_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen) { struct zynqmp_ipi_msg msg; int ret; + u32 buffer[PAYLOAD_ARG_CNT]; + + if (!res) + res = buffer; if (req_len > PMUFW_PAYLOAD_ARG_CNT || res_maxlen > PMUFW_PAYLOAD_ARG_CNT)