OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

hw/misc: Fix arith overflow in NPCM7XX PWM module

Browse Source 
Fix potential overflow problem when calculating pwm_duty.
1. Ensure p->cmr and p->cnr to be from [0,65535], according to the
   hardware specification.
2. Changed duty to uint32_t. However, since MAX_DUTY * (p->cmr+1)
   can excceed UINT32_MAX, we convert them to uint64_t in computation
   and converted them back to uint32_t.
   (duty is guaranteed to be <= MAX_DUTY so it won't overflow.)

Fixes: CID 1442342
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Doug Evans <dje@google.com>
Signed-off-by: Hao Wu <wuhaotsh@google.com>
Message-id: 20210127011142.2122790-1-wuhaotsh@google.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Hao Wu 2021-01-26 17:11:42 -08:00 committed by Peter Maydell
parent daa726d926
commit 1e5ce6e10a
2 changed files with 21 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
hw/misc/npcm7xx_pwm.c
View File

@ -58,6 +58,9 @@ REG32(NPCM7XX_PWM_PWDR3, 0x50);
#define NPCM7XX_CH_INV BIT(2) #define NPCM7XX_CH_INV BIT(2)
#define NPCM7XX_CH_MOD BIT(3) #define NPCM7XX_CH_MOD BIT(3)
#define NPCM7XX_MAX_CMR 65535
#define NPCM7XX_MAX_CNR 65535
/* Offset of each PWM channel's prescaler in the PPR register. */ /* Offset of each PWM channel's prescaler in the PPR register. */
static const int npcm7xx_ppr_base[] = { 0, 0, 8, 8 }; static const int npcm7xx_ppr_base[] = { 0, 0, 8, 8 };
/* Offset of each PWM channel's clock selector in the CSR register. */ /* Offset of each PWM channel's clock selector in the CSR register. */
@ -96,7 +99,7 @@ static uint32_t npcm7xx_pwm_calculate_freq(NPCM7xxPWM *p)
static uint32_t npcm7xx_pwm_calculate_duty(NPCM7xxPWM *p) static uint32_t npcm7xx_pwm_calculate_duty(NPCM7xxPWM *p)
{ {
uint64_t duty; uint32_t duty;
if (p->running) { if (p->running) {
if (p->cnr == 0) { if (p->cnr == 0) {
@ -104,7 +107,7 @@ static uint32_t npcm7xx_pwm_calculate_duty(NPCM7xxPWM *p)
} else if (p->cmr >= p->cnr) { } else if (p->cmr >= p->cnr) {
duty = NPCM7XX_PWM_MAX_DUTY; duty = NPCM7XX_PWM_MAX_DUTY;
} else { } else {
duty = NPCM7XX_PWM_MAX_DUTY * (p->cmr + 1) / (p->cnr + 1); duty = (uint64_t)NPCM7XX_PWM_MAX_DUTY * (p->cmr + 1) / (p->cnr + 1);
} }
} else { } else {
duty = 0; duty = 0;
@ -357,7 +360,13 @@ static void npcm7xx_pwm_write(void *opaque, hwaddr offset,
case A_NPCM7XX_PWM_CNR2: case A_NPCM7XX_PWM_CNR2:
case A_NPCM7XX_PWM_CNR3: case A_NPCM7XX_PWM_CNR3:
p = &s->pwm[npcm7xx_cnr_index(offset)]; p = &s->pwm[npcm7xx_cnr_index(offset)];
p->cnr = value; if (value > NPCM7XX_MAX_CNR) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid cnr value: %u", __func__, value);
p->cnr = NPCM7XX_MAX_CNR;
} else {
p->cnr = value;
}
npcm7xx_pwm_update_output(p); npcm7xx_pwm_update_output(p);
break; break;
@ -366,7 +375,13 @@ static void npcm7xx_pwm_write(void *opaque, hwaddr offset,
case A_NPCM7XX_PWM_CMR2: case A_NPCM7XX_PWM_CMR2:
case A_NPCM7XX_PWM_CMR3: case A_NPCM7XX_PWM_CMR3:
p = &s->pwm[npcm7xx_cmr_index(offset)]; p = &s->pwm[npcm7xx_cmr_index(offset)];
p->cmr = value; if (value > NPCM7XX_MAX_CMR) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: invalid cmr value: %u", __func__, value);
p->cmr = NPCM7XX_MAX_CMR;
} else {
p->cmr = value;
}
npcm7xx_pwm_update_output(p); npcm7xx_pwm_update_output(p);
break; break;

4
tests/qtest/npcm7xx_pwm-test.c
View File

@ -272,7 +272,7 @@ static uint64_t pwm_compute_freq(QTestState *qts, uint32_t ppr, uint32_t csr,
static uint64_t pwm_compute_duty(uint32_t cnr, uint32_t cmr, bool inverted) static uint64_t pwm_compute_duty(uint32_t cnr, uint32_t cmr, bool inverted)
{ {
uint64_t duty; uint32_t duty;
if (cnr == 0) { if (cnr == 0) {
/* PWM is stopped. */ /* PWM is stopped. */
@ -280,7 +280,7 @@ static uint64_t pwm_compute_duty(uint32_t cnr, uint32_t cmr, bool inverted)
} else if (cmr >= cnr) { } else if (cmr >= cnr) {
duty = MAX_DUTY; duty = MAX_DUTY;
} else { } else {
duty = MAX_DUTY * (cmr + 1) / (cnr + 1); duty = (uint64_t)MAX_DUTY * (cmr + 1) / (cnr + 1);
} }
if (inverted) { if (inverted) {