* [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly
@ 2021-10-15 14:57 Michal Simek
2021-10-15 14:57 ` [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req() Michal Simek
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Michal Simek @ 2021-10-15 14:57 UTC (permalink / raw)
To: u-boot, git, adrian.fiergolski; +Cc: Michal Simek, Simon Glass
There are multiple errors what can happen in ipi_req but they are not
propagated properly. That's why propage all error properly.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/firmware/firmware-zynqmp.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index c22bdca282fc..1391aab0a160 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -165,6 +165,7 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
* firmware API is limited by the SMC call size
*/
u32 regs[] = {api_id, arg0, arg1, arg2, arg3};
+ int ret;
if (api_id == PM_FPGA_LOAD) {
/* Swap addr_hi/low because of incompatibility */
@@ -174,7 +175,10 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
regs[2] = temp;
}
- ipi_req(regs, PAYLOAD_ARG_CNT, ret_payload, PAYLOAD_ARG_CNT);
+ ret = ipi_req(regs, PAYLOAD_ARG_CNT, ret_payload,
+ PAYLOAD_ARG_CNT);
+ if (ret)
+ return ret;
#else
return -EPERM;
#endif
--
2.33.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req()
2021-10-15 14:57 [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly Michal Simek
@ 2021-10-15 14:57 ` Michal Simek
2021-10-18 9:34 ` Adrian Fiergolski
2021-10-21 6:55 ` Michal Simek
2021-10-18 9:33 ` [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly Adrian Fiergolski
2021-10-21 6:55 ` Michal Simek
2 siblings, 2 replies; 6+ messages in thread
From: Michal Simek @ 2021-10-15 14:57 UTC (permalink / raw)
To: u-boot, git, adrian.fiergolski; +Cc: Michal Simek, Simon Glass
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)
--
2.33.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly
2021-10-15 14:57 [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly Michal Simek
2021-10-15 14:57 ` [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req() Michal Simek
@ 2021-10-18 9:33 ` Adrian Fiergolski
2021-10-21 6:55 ` Michal Simek
2 siblings, 0 replies; 6+ messages in thread
From: Adrian Fiergolski @ 2021-10-18 9:33 UTC (permalink / raw)
To: Michal Simek, u-boot, git; +Cc: Michal Simek, Simon Glass
On 15.10.2021 16:57, Michal Simek wrote:
> There are multiple errors what can happen in ipi_req but they are not
> propagated properly. That's why propage all error properly.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Adrian Fiergolski <Adrian.Fiergolski@fastree3d.com>
Thanks,
Adrian
> ---
>
> drivers/firmware/firmware-zynqmp.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
> index c22bdca282fc..1391aab0a160 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -165,6 +165,7 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
> * firmware API is limited by the SMC call size
> */
> u32 regs[] = {api_id, arg0, arg1, arg2, arg3};
> + int ret;
>
> if (api_id == PM_FPGA_LOAD) {
> /* Swap addr_hi/low because of incompatibility */
> @@ -174,7 +175,10 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
> regs[2] = temp;
> }
>
> - ipi_req(regs, PAYLOAD_ARG_CNT, ret_payload, PAYLOAD_ARG_CNT);
> + ret = ipi_req(regs, PAYLOAD_ARG_CNT, ret_payload,
> + PAYLOAD_ARG_CNT);
> + if (ret)
> + return ret;
> #else
> return -EPERM;
> #endif
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req()
2021-10-15 14:57 ` [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req() Michal Simek
@ 2021-10-18 9:34 ` Adrian Fiergolski
2021-10-21 6:55 ` Michal Simek
1 sibling, 0 replies; 6+ messages in thread
From: Adrian Fiergolski @ 2021-10-18 9:34 UTC (permalink / raw)
To: Michal Simek, u-boot, git; +Cc: Michal Simek, Simon Glass
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly
2021-10-15 14:57 [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly Michal Simek
2021-10-15 14:57 ` [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req() Michal Simek
2021-10-18 9:33 ` [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly Adrian Fiergolski
@ 2021-10-21 6:55 ` Michal Simek
2 siblings, 0 replies; 6+ messages in thread
From: Michal Simek @ 2021-10-21 6:55 UTC (permalink / raw)
To: Michal Simek, u-boot, git, adrian.fiergolski; +Cc: Simon Glass
On 10/15/21 16:57, Michal Simek wrote:
> There are multiple errors what can happen in ipi_req but they are not
> propagated properly. That's why propage all error properly.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> drivers/firmware/firmware-zynqmp.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
> index c22bdca282fc..1391aab0a160 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -165,6 +165,7 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
> * firmware API is limited by the SMC call size
> */
> u32 regs[] = {api_id, arg0, arg1, arg2, arg3};
> + int ret;
>
> if (api_id == PM_FPGA_LOAD) {
> /* Swap addr_hi/low because of incompatibility */
> @@ -174,7 +175,10 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
> regs[2] = temp;
> }
>
> - ipi_req(regs, PAYLOAD_ARG_CNT, ret_payload, PAYLOAD_ARG_CNT);
> + ret = ipi_req(regs, PAYLOAD_ARG_CNT, ret_payload,
> + PAYLOAD_ARG_CNT);
> + if (ret)
> + return ret;
> #else
> return -EPERM;
> #endif
>
Applied.
M
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req()
2021-10-15 14:57 ` [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req() Michal Simek
2021-10-18 9:34 ` Adrian Fiergolski
@ 2021-10-21 6:55 ` Michal Simek
1 sibling, 0 replies; 6+ messages in thread
From: Michal Simek @ 2021-10-21 6:55 UTC (permalink / raw)
To: Michal Simek, u-boot, git, adrian.fiergolski; +Cc: Simon Glass
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
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-21 6:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 14:57 [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly Michal Simek
2021-10-15 14:57 ` [PATCH 2/2] firmware: zynqmp: fix write to an uninitialised pointer in ipi_req() Michal Simek
2021-10-18 9:34 ` Adrian Fiergolski
2021-10-21 6:55 ` Michal Simek
2021-10-18 9:33 ` [PATCH 1/2] firmware: zynqmp: Handle errors from ipi_req properly Adrian Fiergolski
2021-10-21 6:55 ` Michal Simek
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.