| Message ID | 20200713143716.9881-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-5.1] hw/arm/armsse: Assert info->num_cpus is in-bounds in armsse_realize() | expand |
On 7/13/20 4:37 PM, Peter Maydell wrote: > In armsse_realize() we have a loop over [0, info->num_cpus), which > indexes into various fixed-size arrays in the ARMSSE struct. This > confuses Coverity, which warns that we might overrun those arrays > (CID 1430326, 1430337, 1430371, 1430414, 1430430). This can't > actually happen, because the info struct is always one of the entries > in the armsse_variants[] array and num_cpus is either 1 or 2; we also > already assert in armsse_init() that num_cpus is not too large. > However, adding an assert to armsse_realize() like the one in > armsse_init() should help Coverity figure out that these code paths > aren't possible. Similar to commit 1db889c71f ("hw/openrisc/openrisc_sim: Add assertion to silence GCC warning"). Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/arm/armsse.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c > index 64fcab895f7..dcbff9bd8f4 100644 > --- a/hw/arm/armsse.c > +++ b/hw/arm/armsse.c > @@ -452,6 +452,8 @@ static void armsse_realize(DeviceState *dev, Error **errp) > return; > } > > + assert(info->num_cpus <= SSE_MAX_CPUS); > + > /* max SRAM_ADDR_WIDTH: 24 - log2(SRAM_NUM_BANK) */ > assert(is_power_of_2(info->sram_banks)); > addr_width_max = 24 - ctz32(info->sram_banks); >
Patchew URL: https://patchew.org/QEMU/20200713143716.9881-1-peter.maydell@linaro.org/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
TEST check-unit: tests/test-crypto-secret
TEST check-unit: tests/test-char
**
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should not be NULL
ERROR test-char - Bail out! ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should not be NULL
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs....
TEST iotest-qcow2: 017
TEST iotest-qcow2: 018
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=a72d62fc12f242e5a79799d2742f6b37', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-s6rziv2y/src/docker-src.2020-07-13-11.15.06.18606:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=a72d62fc12f242e5a79799d2742f6b37
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-s6rziv2y/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 15m51.537s
user 0m9.092s
The full log is available at
http://patchew.org/logs/20200713143716.9881-1-peter.maydell@linaro.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index 64fcab895f7..dcbff9bd8f4 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -452,6 +452,8 @@ static void armsse_realize(DeviceState *dev, Error **errp) return; } + assert(info->num_cpus <= SSE_MAX_CPUS); + /* max SRAM_ADDR_WIDTH: 24 - log2(SRAM_NUM_BANK) */ assert(is_power_of_2(info->sram_banks)); addr_width_max = 24 - ctz32(info->sram_banks);
In armsse_realize() we have a loop over [0, info->num_cpus), which indexes into various fixed-size arrays in the ARMSSE struct. This confuses Coverity, which warns that we might overrun those arrays (CID 1430326, 1430337, 1430371, 1430414, 1430430). This can't actually happen, because the info struct is always one of the entries in the armsse_variants[] array and num_cpus is either 1 or 2; we also already assert in armsse_init() that num_cpus is not too large. However, adding an assert to armsse_realize() like the one in armsse_init() should help Coverity figure out that these code paths aren't possible. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/armsse.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.20.1