On 2/19/21 11:32 PM, Peter Maydell wrote:
> The read_tcnt() function calculates the TCNT register values for the
> two channels of the timer module; it sets these up in the local
> tcnt[] array, and eventually returns either one or both of them,
> depending on whether the access is 8 or 16 bits. However, not all of
> the code paths through this function set both elements of this array:
> if the guest has programmed the TCCR.CSS register fields to values
> which are either documented as not to be used or which QEMU does not
> implement, then the function will return uninitialized data. (This
> was spotted by Coverity.)
>
> Add the missing CSS cases to this code, so that we return a
> consistent value instead of uninitialized data, and so the code
> structure indicates what's happening.
>
> Fixes: CID 1429976
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> hw/timer/renesas_tmr.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c
> index 22260aaaba5..eed39917fec 100644
> --- a/hw/timer/renesas_tmr.c
> +++ b/hw/timer/renesas_tmr.c
> @@ -46,7 +46,9 @@ REG8(TCCR, 10)
> FIELD(TCCR, CSS, 3, 2)
> FIELD(TCCR, TMRIS, 7, 1)
>
> +#define CSS_EXTERNAL 0x00
> #define CSS_INTERNAL 0x01
> +#define CSS_INVALID 0x02
> #define CSS_CASCADING 0x03
> #define CCLR_A 0x01
> #define CCLR_B 0x02
> @@ -130,13 +132,20 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned
> size, int ch)
> if (delta > 0) {
> tmr->tick = now;
>
> - if (FIELD_EX8(tmr->tccr[1], TCCR, CSS) == CSS_INTERNAL) {
> + switch (FIELD_EX8(tmr->tccr[1], TCCR, CSS)) {
> + case CSS_INTERNAL:
> /* timer1 count update */
> elapsed = elapsed_time(tmr, 1, delta);
> if (elapsed >= 0x100) {
> ovf = elapsed >> 8;
> }
> tcnt[1] = tmr->tcnt[1] + (elapsed & 0xff);
> + break;
> + case CSS_INVALID: /* guest error to have set this */
> + case CSS_EXTERNAL: /* QEMU doesn't implement these */
> + case CSS_CASCADING:
> + tcnt[1] = tmr->tcnt[1];
> + break;
> }
> switch (FIELD_EX8(tmr->tccr[0], TCCR, CSS)) {
> case CSS_INTERNAL:
> @@ -144,9 +153,11 @@ static uint16_t read_tcnt(RTMRState *tmr, unsigned size,
> int ch)
> tcnt[0] = tmr->tcnt[0] + elapsed;
> break;
> case CSS_CASCADING:
> - if (ovf > 0) {
> - tcnt[0] = tmr->tcnt[0] + ovf;
> - }
> + tcnt[0] = tmr->tcnt[0] + ovf;
> + break;
> + case CSS_INVALID: /* guest error to have set this */
> + case CSS_EXTERNAL: /* QEMU doesn't implement this */
> + tcnt[0] = tmr->tcnt[0];
> break;
> }
Elegant nice fix :)
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
