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
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 > >
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>
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>
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
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
© 2016 - 2024 Red Hat, Inc.