highbank: validate register offset before access
PATCH 1/1 highbank: validate register offset before access
From: Prasad J Pandit <[email protected]>
An 'offset' parameter sent to highbank register r/w functions could be greater than number(NUM_REGS=0x200) of hb registers, leading to an OOB access issue. Add check to avoid it.
--- hw/arm/highbank.c | 7 +++++++ 1 file changed, 7 insertions(+)
hw/arm/highbank.c +7 -0
@@ -117,6 +117,9 @@ static void hb_regs_write(void *opaque, hwaddr offset, } } +if (offset /4 >=NUM_REGS ) { +return ; + } regs[offset/4 ] = value; }
@@ -124,6 +127,10 @@ static uint64_t hb_regs_read(void *opaque, hwaddr offset, unsigned size) {uint32_t *regs = opaque; + +if (offset /4 >=NUM_REGS ) { +return 0 ; + }uint32_t value = regs[offset/4 ];if ((offset ==0x100 ) || (offset ==0x108 ) || (offset ==0x10C )) {
Signature · 1 line
2.13.6
Hi Prasad, Moguofang.
Quoted 17 lines · P J P
From: Prasad J Pandit <[email protected]>
An 'offset' parameter sent to highbank register r/w functions could be greater than
number(NUM_REGS=0x200)of hb registers, leading to an OOB access issue. Add check to avoid it.Reported-by Moguofang (Dennis mo) <[email protected]>Signed-off-by Prasad J Pandit <[email protected]>--- hw/arm/highbank.c | 7 +++++++ 1 file changed, 7 insertions(+)hw/arm/highbank.c +0
-0
hw/arm/highbank.c static void hb_regs_write(void *opaque, hwaddr offset, · line 120
Quoted 4 lines
hw/arm/highbank.c +1
-0@@ -117,6 +117,9 @@ static void hb_regs_write(void *opaque, hwaddr offset, } } +if (offset /4 >=NUM_REGS ) {
I'd report that:
qemu_log_mask(LOG_UNIMP, ...
Cc'ing Shawn & Rob since this might also be a LOG_GUEST_ERROR.
hw/arm/highbank.c static uint64_t hb_regs_read(void *opaque, hwaddr offset, · line 131
Quoted 11 lines
hw/arm/highbank.c +4
-0+return ; + } regs[offset/4 ] = value; }@@ -124,6 +127,10 @@ static uint64_t hb_regs_read(void *opaque, hwaddr offset, unsigned size) {uint32_t *regs = opaque; + +if (offset /4 >=NUM_REGS ) {
Ditto.
hw/arm/highbank.c static uint64_t hb_regs_read(void *opaque, hwaddr offset, · line 133
Quoted 2 lines
hw/arm/highbank.c +2
-0+return 0 ; + }
From CODING_STYLE:
Mixed declarations (interleaving statements and declarations within blocks) are generally not allowed; declarations should be at the beginning of blocks.
hw/arm/highbank.c static uint64_t hb_regs_read(void *opaque, hwaddr offset, · line 136
Quoted 3 lines
hw/arm/highbank.c +0
-0uint32_t value = regs[offset/4 ];if ((offset ==0x100 ) || (offset ==0x108 ) || (offset ==0x10C )) {
Regards,
Phil.
Hello Philippe,
+-- On Fri, 10 Nov 2017, Philippe Mathieu-Daudé wrote --+ | I'd report that: | | qemu_log_mask(LOG_UNIMP, ... | | Cc'ing Shawn & Rob since this might also be a LOG_GUEST_ERROR. | | Mixed declarations (interleaving statements and declarations within | blocks) are generally not allowed; declarations should be at the | beginning of blocks.
Done. Sent a revised patch v1.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F