* [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error
@ 2020-12-10 14:16 Philippe Mathieu-Daudé
2020-12-10 16:39 ` Alistair Francis
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-10 14:16 UTC (permalink / raw)
To: qemu-devel
Cc: Damien Hedde, Peter Maydell, Mauro Matteo Cascella,
Edgar E . Iglesias, Gaoning Pan, Alistair Francis,
Philippe Mathieu-Daudé,
qemu-arm, Alistair Francis, Edgar E. Iglesias, Gaoning Pan
Malicious user can set the feedback divisor for the PLLs
to zero, triggering a floating-point exception (SIGFPE).
As the datasheet [*] is not clear how hardware behaves
when these bits are zeroes, use the maximum divisor
possible (128) to avoid the software FPE.
[*] Zynq-7000 TRM, UG585 (v1.12.2)
B.28 System Level Control Registers (slcr)
-> "Register (slcr) ARM_PLL_CTRL"
25.10.4 PLLs
-> "Software-Controlled PLL Update"
Fixes: 38867cb7ec9 ("hw/misc/zynq_slcr: add clock generation for uarts")
Reported-by: Gaoning Pan <pgn@zju.edu.cn>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Damien Hedde <damien.hedde@greensocs.com>
Cc: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Cc: Alistair Francis <alistair.francis@wdc.com>
Cc: Gaoning Pan <gaoning.pgn@antgroup.com>
Cc: Mauro Matteo Cascella <mcascell@redhat.com>
Alternative is to threat that as PLL disabled and return 0...
---
hw/misc/zynq_slcr.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index a2b28019e3c..66504a9d3ab 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -217,6 +217,11 @@ static uint64_t zynq_slcr_compute_pll(uint64_t input, uint32_t ctrl_reg)
return 0;
}
+ /* Consider zero feedback as maximum divide ratio possible */
+ if (!mult) {
+ mult = 1 << R_xxx_PLL_CTRL_PLL_FPDIV_LENGTH;
+ }
+
/* frequency multiplier -> period division */
return input / mult;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error
2020-12-10 14:16 [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error Philippe Mathieu-Daudé
@ 2020-12-10 16:39 ` Alistair Francis
2020-12-10 20:13 ` Edgar E. Iglesias
2020-12-10 17:21 ` Mauro Matteo Cascella
2020-12-15 13:37 ` Peter Maydell
2 siblings, 1 reply; 6+ messages in thread
From: Alistair Francis @ 2020-12-10 16:39 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Damien Hedde, Peter Maydell, Mauro Matteo Cascella,
Edgar E . Iglesias, Gaoning Pan, Alistair Francis,
qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis,
Edgar E. Iglesias, Gaoning Pan
On Thu, Dec 10, 2020 at 6:27 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Malicious user can set the feedback divisor for the PLLs
> to zero, triggering a floating-point exception (SIGFPE).
>
> As the datasheet [*] is not clear how hardware behaves
> when these bits are zeroes, use the maximum divisor
> possible (128) to avoid the software FPE.
>
> [*] Zynq-7000 TRM, UG585 (v1.12.2)
> B.28 System Level Control Registers (slcr)
> -> "Register (slcr) ARM_PLL_CTRL"
> 25.10.4 PLLs
> -> "Software-Controlled PLL Update"
>
> Fixes: 38867cb7ec9 ("hw/misc/zynq_slcr: add clock generation for uarts")
> Reported-by: Gaoning Pan <pgn@zju.edu.cn>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Damien Hedde <damien.hedde@greensocs.com>
> Cc: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Gaoning Pan <gaoning.pgn@antgroup.com>
> Cc: Mauro Matteo Cascella <mcascell@redhat.com>
>
> Alternative is to threat that as PLL disabled and return 0...
I'm not sure which is better, but this patch now is better then before:
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/misc/zynq_slcr.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> index a2b28019e3c..66504a9d3ab 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -217,6 +217,11 @@ static uint64_t zynq_slcr_compute_pll(uint64_t input, uint32_t ctrl_reg)
> return 0;
> }
>
> + /* Consider zero feedback as maximum divide ratio possible */
> + if (!mult) {
> + mult = 1 << R_xxx_PLL_CTRL_PLL_FPDIV_LENGTH;
> + }
> +
> /* frequency multiplier -> period division */
> return input / mult;
> }
> --
> 2.26.2
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error
2020-12-10 14:16 [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error Philippe Mathieu-Daudé
2020-12-10 16:39 ` Alistair Francis
@ 2020-12-10 17:21 ` Mauro Matteo Cascella
2020-12-15 13:37 ` Peter Maydell
2 siblings, 0 replies; 6+ messages in thread
From: Mauro Matteo Cascella @ 2020-12-10 17:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Damien Hedde, Peter Maydell, Edgar E . Iglesias, Gaoning Pan,
Alistair Francis, QEMU Developers, qemu-arm, Alistair Francis,
Edgar E. Iglesias, Gaoning Pan
On Thu, Dec 10, 2020 at 3:16 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Malicious user can set the feedback divisor for the PLLs
> to zero, triggering a floating-point exception (SIGFPE).
>
> As the datasheet [*] is not clear how hardware behaves
> when these bits are zeroes, use the maximum divisor
> possible (128) to avoid the software FPE.
>
> [*] Zynq-7000 TRM, UG585 (v1.12.2)
> B.28 System Level Control Registers (slcr)
> -> "Register (slcr) ARM_PLL_CTRL"
> 25.10.4 PLLs
> -> "Software-Controlled PLL Update"
>
> Fixes: 38867cb7ec9 ("hw/misc/zynq_slcr: add clock generation for uarts")
> Reported-by: Gaoning Pan <pgn@zju.edu.cn>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Damien Hedde <damien.hedde@greensocs.com>
> Cc: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Gaoning Pan <gaoning.pgn@antgroup.com>
> Cc: Mauro Matteo Cascella <mcascell@redhat.com>
>
> Alternative is to threat that as PLL disabled and return 0...
> ---
> hw/misc/zynq_slcr.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> index a2b28019e3c..66504a9d3ab 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -217,6 +217,11 @@ static uint64_t zynq_slcr_compute_pll(uint64_t input, uint32_t ctrl_reg)
> return 0;
> }
>
> + /* Consider zero feedback as maximum divide ratio possible */
> + if (!mult) {
> + mult = 1 << R_xxx_PLL_CTRL_PLL_FPDIV_LENGTH;
> + }
> +
> /* frequency multiplier -> period division */
> return input / mult;
> }
> --
> 2.26.2
>
This patch fixes RHBZ#1906388:
https://bugzilla.redhat.com/show_bug.cgi?id=1906388
Thank you,
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error
2020-12-10 16:39 ` Alistair Francis
@ 2020-12-10 20:13 ` Edgar E. Iglesias
2020-12-11 15:26 ` Damien Hedde
0 siblings, 1 reply; 6+ messages in thread
From: Edgar E. Iglesias @ 2020-12-10 20:13 UTC (permalink / raw)
To: Alistair Francis
Cc: Damien Hedde, Peter Maydell, Mauro Matteo Cascella, Gaoning Pan,
Alistair Francis, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis,
Edgar E. Iglesias, Gaoning Pan
On Thu, Dec 10, 2020 at 08:39:32AM -0800, Alistair Francis wrote:
> On Thu, Dec 10, 2020 at 6:27 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > Malicious user can set the feedback divisor for the PLLs
> > to zero, triggering a floating-point exception (SIGFPE).
> >
> > As the datasheet [*] is not clear how hardware behaves
> > when these bits are zeroes, use the maximum divisor
> > possible (128) to avoid the software FPE.
> >
> > [*] Zynq-7000 TRM, UG585 (v1.12.2)
> > B.28 System Level Control Registers (slcr)
> > -> "Register (slcr) ARM_PLL_CTRL"
> > 25.10.4 PLLs
> > -> "Software-Controlled PLL Update"
> >
> > Fixes: 38867cb7ec9 ("hw/misc/zynq_slcr: add clock generation for uarts")
> > Reported-by: Gaoning Pan <pgn@zju.edu.cn>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> > Cc: Damien Hedde <damien.hedde@greensocs.com>
> > Cc: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > Cc: Alistair Francis <alistair.francis@wdc.com>
> > Cc: Gaoning Pan <gaoning.pgn@antgroup.com>
> > Cc: Mauro Matteo Cascella <mcascell@redhat.com>
> >
> > Alternative is to threat that as PLL disabled and return 0...
>
> I'm not sure which is better, but this patch now is better then before:
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
I agree with Alistair:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error
2020-12-10 20:13 ` Edgar E. Iglesias
@ 2020-12-11 15:26 ` Damien Hedde
0 siblings, 0 replies; 6+ messages in thread
From: Damien Hedde @ 2020-12-11 15:26 UTC (permalink / raw)
To: Edgar E. Iglesias, Alistair Francis
Cc: Peter Maydell, Mauro Matteo Cascella, Gaoning Pan,
Alistair Francis, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis,
Edgar E. Iglesias, Gaoning Pan
On 12/10/20 9:13 PM, Edgar E. Iglesias wrote:
> On Thu, Dec 10, 2020 at 08:39:32AM -0800, Alistair Francis wrote:
>> On Thu, Dec 10, 2020 at 6:27 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>
>>> Malicious user can set the feedback divisor for the PLLs
>>> to zero, triggering a floating-point exception (SIGFPE).
>>>
>>> As the datasheet [*] is not clear how hardware behaves
>>> when these bits are zeroes, use the maximum divisor
>>> possible (128) to avoid the software FPE.
>>>
>>> [*] Zynq-7000 TRM, UG585 (v1.12.2)
>>> B.28 System Level Control Registers (slcr)
>>> -> "Register (slcr) ARM_PLL_CTRL"
>>> 25.10.4 PLLs
>>> -> "Software-Controlled PLL Update"
>>>
>>> Fixes: 38867cb7ec9 ("hw/misc/zynq_slcr: add clock generation for uarts")
>>> Reported-by: Gaoning Pan <pgn@zju.edu.cn>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> Cc: Damien Hedde <damien.hedde@greensocs.com>
>>> Cc: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>>> Cc: Alistair Francis <alistair.francis@wdc.com>
>>> Cc: Gaoning Pan <gaoning.pgn@antgroup.com>
>>> Cc: Mauro Matteo Cascella <mcascell@redhat.com>
>>>
>>> Alternative is to threat that as PLL disabled and return 0...
>>
>> I'm not sure which is better, but this patch now is better then before:
>>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> I agree with Alistair:
>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error
2020-12-10 14:16 [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error Philippe Mathieu-Daudé
2020-12-10 16:39 ` Alistair Francis
2020-12-10 17:21 ` Mauro Matteo Cascella
@ 2020-12-15 13:37 ` Peter Maydell
2 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2020-12-15 13:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Damien Hedde, Edgar E . Iglesias, Mauro Matteo Cascella,
Gaoning Pan, Alistair Francis, QEMU Developers, qemu-arm,
Alistair Francis, Edgar E. Iglesias, Gaoning Pan
On Thu, 10 Dec 2020 at 14:16, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Malicious user can set the feedback divisor for the PLLs
> to zero, triggering a floating-point exception (SIGFPE).
>
> As the datasheet [*] is not clear how hardware behaves
> when these bits are zeroes, use the maximum divisor
> possible (128) to avoid the software FPE.
>
> [*] Zynq-7000 TRM, UG585 (v1.12.2)
> B.28 System Level Control Registers (slcr)
> -> "Register (slcr) ARM_PLL_CTRL"
> 25.10.4 PLLs
> -> "Software-Controlled PLL Update"
>
> Fixes: 38867cb7ec9 ("hw/misc/zynq_slcr: add clock generation for uarts")
> Reported-by: Gaoning Pan <pgn@zju.edu.cn>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-15 13:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-10 14:16 [RFC PATCH] hw/misc/zynq_slcr: Avoid #DIV/0! error Philippe Mathieu-Daudé
2020-12-10 16:39 ` Alistair Francis
2020-12-10 20:13 ` Edgar E. Iglesias
2020-12-11 15:26 ` Damien Hedde
2020-12-10 17:21 ` Mauro Matteo Cascella
2020-12-15 13:37 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).