| Message ID | 20230530131214.373524-1-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/2] target/ppc: Fix decrementer time underflow and infinite timer loop | expand |
On 5/30/23 10:12, Nicholas Piggin wrote: > It is possible to store a very large value to the decrementer that it > does not raise the decrementer exception so the timer is scheduled, but > the next time value wraps and is treated as in the past. > > This can occur if (u64)-1 is stored on a zero-triggered exception, or > (u64)-1 is stored twice on an underflow-triggered exception, for > example. > > If such a value is set in DECAR, it gets stored to the decrementer by > the timer function, which then immediately causes another timer, which > hangs QEMU. > > Clamp the decrementer to the implemented width, and use that as the > value for the timer calculation, effectively preventing this overflow. > > Reported-by: sdicaro@DDCI.com > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> And queued. Daniel > sdicaro@DDCI.com debugged and reported this, I just changed their fix > to extract variable bits so it works with large decrementer. So most > of the credit goes to them. > > Thanks, > Nick > > hw/ppc/ppc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c > index 4e816c68c7..d80b0adc6c 100644 > --- a/hw/ppc/ppc.c > +++ b/hw/ppc/ppc.c > @@ -798,6 +798,8 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp, > int64_t signed_decr; > > /* Truncate value to decr_width and sign extend for simplicity */ > + value = extract64(value, 0, nr_bits); > + decr = extract64(decr, 0, nr_bits); > signed_value = sextract64(value, 0, nr_bits); > signed_decr = sextract64(decr, 0, nr_bits); >
30.05.2023 16:12, Nicholas Piggin wrote: > It is possible to store a very large value to the decrementer that it > does not raise the decrementer exception so the timer is scheduled, but > the next time value wraps and is treated as in the past. > > This can occur if (u64)-1 is stored on a zero-triggered exception, or > (u64)-1 is stored twice on an underflow-triggered exception, for > example. > > If such a value is set in DECAR, it gets stored to the decrementer by > the timer function, which then immediately causes another timer, which > hangs QEMU. > > Clamp the decrementer to the implemented width, and use that as the > value for the timer calculation, effectively preventing this overflow. > > Reported-by: sdicaro@DDCI.com > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > sdicaro@DDCI.com debugged and reported this, I just changed their fix > to extract variable bits so it works with large decrementer. So most > of the credit goes to them. > > Thanks, > Nick > > hw/ppc/ppc.c | 2 ++ > 1 file changed, 2 insertions(+) Is it a -stable material? From the description it smells like it is. Thanks, /mjt
On 6/7/23 06:26, Michael Tokarev wrote: > 30.05.2023 16:12, Nicholas Piggin wrote: >> It is possible to store a very large value to the decrementer that it >> does not raise the decrementer exception so the timer is scheduled, but >> the next time value wraps and is treated as in the past. >> >> This can occur if (u64)-1 is stored on a zero-triggered exception, or >> (u64)-1 is stored twice on an underflow-triggered exception, for >> example. >> >> If such a value is set in DECAR, it gets stored to the decrementer by >> the timer function, which then immediately causes another timer, which >> hangs QEMU. >> >> Clamp the decrementer to the implemented width, and use that as the >> value for the timer calculation, effectively preventing this overflow. >> >> Reported-by: sdicaro@DDCI.com >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> sdicaro@DDCI.com debugged and reported this, I just changed their fix >> to extract variable bits so it works with large decrementer. So most >> of the credit goes to them. >> >> Thanks, >> Nick >> >> hw/ppc/ppc.c | 2 ++ >> 1 file changed, 2 insertions(+) > > Is it a -stable material? From the description it smells like it is. Feel free to pick it for -stable. Thanks, Daniel > > Thanks, > > /mjt >
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c index 4e816c68c7..d80b0adc6c 100644 --- a/hw/ppc/ppc.c +++ b/hw/ppc/ppc.c @@ -798,6 +798,8 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp, int64_t signed_decr; /* Truncate value to decr_width and sign extend for simplicity */ + value = extract64(value, 0, nr_bits); + decr = extract64(decr, 0, nr_bits); signed_value = sextract64(value, 0, nr_bits); signed_decr = sextract64(decr, 0, nr_bits);
It is possible to store a very large value to the decrementer that it does not raise the decrementer exception so the timer is scheduled, but the next time value wraps and is treated as in the past. This can occur if (u64)-1 is stored on a zero-triggered exception, or (u64)-1 is stored twice on an underflow-triggered exception, for example. If such a value is set in DECAR, it gets stored to the decrementer by the timer function, which then immediately causes another timer, which hangs QEMU. Clamp the decrementer to the implemented width, and use that as the value for the timer calculation, effectively preventing this overflow. Reported-by: sdicaro@DDCI.com Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- sdicaro@DDCI.com debugged and reported this, I just changed their fix to extract variable bits so it works with large decrementer. So most of the credit goes to them. Thanks, Nick hw/ppc/ppc.c | 2 ++ 1 file changed, 2 insertions(+)