1
The following changes since commit e3debd5e7d0ce031356024878a0a18b9d109354a:
1
It's been quiet on the arm front this week, so all I have is
2
these coverity fixes I posted a while back...
2
3
3
Merge tag 'pull-request-2023-03-24' of https://gitlab.com/thuth/qemu into staging (2023-03-24 16:08:46 +0000)
4
-- PMM
5
6
The following changes since commit 853546f8128476eefb701d4a55b2781bb3a46faa:
7
8
Merge tag 'pull-loongarch-20240322' of https://gitlab.com/gaosong/qemu into staging (2024-03-22 10:59:57 +0000)
4
9
5
are available in the Git repository at:
10
are available in the Git repository at:
6
11
7
https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20230328
12
https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20240325
8
13
9
for you to fetch changes up to 46e3b237c52e0c48bfd81bce020b51fbe300b23a:
14
for you to fetch changes up to 55c79639d553c1b7a82b4cde781ad5f316f45b0e:
10
15
11
target/arm/gdbstub: Only advertise M-profile features if TCG available (2023-03-28 10:53:40 +0100)
16
tests/qtest/libqtest.c: Check for g_setenv() failure (2024-03-25 10:41:01 +0000)
12
17
13
----------------------------------------------------------------
18
----------------------------------------------------------------
14
target-arm queue:
19
target-arm queue:
15
* fix part of the "TCG-disabled builds are broken" issue
20
* Fixes for seven minor coverity issues
16
21
17
----------------------------------------------------------------
22
----------------------------------------------------------------
18
Philippe Mathieu-Daudé (1):
23
Peter Maydell (7):
19
target/arm/gdbstub: Only advertise M-profile features if TCG available
24
tests/qtest/npcm7xx_emc_test: Don't leak cmd_line
25
tests/unit/socket-helpers: Don't close(-1)
26
net/af-xdp.c: Don't leak sock_fds array in net_init_af_xdp()
27
hw/misc/pca9554: Correct error check bounds in get/set pin functions
28
hw/nvram/mac_nvram: Report failure to write data
29
tests/unit/test-throttle: Avoid unintended integer division
30
tests/qtest/libqtest.c: Check for g_setenv() failure
20
31
21
target/arm/gdbstub.c | 5 +++--
32
hw/misc/pca9554.c | 4 ++--
22
1 file changed, 3 insertions(+), 2 deletions(-)
33
hw/nvram/mac_nvram.c | 5 ++++-
23
34
net/af-xdp.c | 3 +--
35
tests/qtest/libqtest.c | 6 +++++-
36
tests/qtest/npcm7xx_emc-test.c | 4 ++--
37
tests/unit/socket-helpers.c | 4 +++-
38
tests/unit/test-throttle.c | 4 ++--
39
7 files changed, 19 insertions(+), 11 deletions(-)
diff view generated by jsdifflib
New patch
1
In test_rx() and test_tx() we allocate a GString *cmd_line
2
but never free it. This is pretty harmless in a test case, but
3
Coverity spotted it.
1
4
5
Resolves: Coverity CID 1507122
6
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
7
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
8
Reviewed-by: Thomas Huth <thuth@redhat.com>
9
Message-id: 20240312183810.557768-2-peter.maydell@linaro.org
10
---
11
tests/qtest/npcm7xx_emc-test.c | 4 ++--
12
1 file changed, 2 insertions(+), 2 deletions(-)
13
14
diff --git a/tests/qtest/npcm7xx_emc-test.c b/tests/qtest/npcm7xx_emc-test.c
15
index XXXXXXX..XXXXXXX 100644
16
--- a/tests/qtest/npcm7xx_emc-test.c
17
+++ b/tests/qtest/npcm7xx_emc-test.c
18
@@ -XXX,XX +XXX,XX @@ static void emc_test_ptle(QTestState *qts, const EMCModule *mod, int fd)
19
static void test_tx(gconstpointer test_data)
20
{
21
const TestData *td = test_data;
22
- GString *cmd_line = g_string_new("-machine quanta-gsj");
23
+ g_autoptr(GString) cmd_line = g_string_new("-machine quanta-gsj");
24
int *test_sockets = packet_test_init(emc_module_index(td->module),
25
cmd_line);
26
QTestState *qts = qtest_init(cmd_line->str);
27
@@ -XXX,XX +XXX,XX @@ static void test_tx(gconstpointer test_data)
28
static void test_rx(gconstpointer test_data)
29
{
30
const TestData *td = test_data;
31
- GString *cmd_line = g_string_new("-machine quanta-gsj");
32
+ g_autoptr(GString) cmd_line = g_string_new("-machine quanta-gsj");
33
int *test_sockets = packet_test_init(emc_module_index(td->module),
34
cmd_line);
35
QTestState *qts = qtest_init(cmd_line->str);
36
--
37
2.34.1
diff view generated by jsdifflib
New patch
1
In socket_check_afunix_support() we call socket(PF_UNIX, SOCK_STREAM, 0)
2
to see if it works, but we call close() on the result whether it
3
worked or not. Only close the fd if the socket() call succeeded.
4
Spotted by Coverity.
1
5
6
Resolves: Coverity CID 1497481
7
8
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
9
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
10
Reviewed-by: Thomas Huth <thuth@redhat.com>
11
Message-id: 20240312183810.557768-3-peter.maydell@linaro.org
12
---
13
tests/unit/socket-helpers.c | 4 +++-
14
1 file changed, 3 insertions(+), 1 deletion(-)
15
16
diff --git a/tests/unit/socket-helpers.c b/tests/unit/socket-helpers.c
17
index XXXXXXX..XXXXXXX 100644
18
--- a/tests/unit/socket-helpers.c
19
+++ b/tests/unit/socket-helpers.c
20
@@ -XXX,XX +XXX,XX @@ void socket_check_afunix_support(bool *has_afunix)
21
int fd;
22
23
fd = socket(PF_UNIX, SOCK_STREAM, 0);
24
- close(fd);
25
26
#ifdef _WIN32
27
*has_afunix = (fd != (int)INVALID_SOCKET);
28
@@ -XXX,XX +XXX,XX @@ void socket_check_afunix_support(bool *has_afunix)
29
*has_afunix = (fd >= 0);
30
#endif
31
32
+ if (*has_afunix) {
33
+ close(fd);
34
+ }
35
return;
36
}
37
--
38
2.34.1
diff view generated by jsdifflib
New patch
1
In net_init_af_xdp() we parse the arguments and allocate
2
a buffer of ints into sock_fds. However, although we
3
free this in the error exit path, we don't ever free it
4
in the successful return path. Coverity spots this leak.
1
5
6
Switch to g_autofree so we don't need to manually free the
7
array.
8
9
Resolves: Coverity CID 1534906
10
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
11
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
12
Reviewed-by: Thomas Huth <thuth@redhat.com>
13
Message-id: 20240312183810.557768-4-peter.maydell@linaro.org
14
---
15
net/af-xdp.c | 3 +--
16
1 file changed, 1 insertion(+), 2 deletions(-)
17
18
diff --git a/net/af-xdp.c b/net/af-xdp.c
19
index XXXXXXX..XXXXXXX 100644
20
--- a/net/af-xdp.c
21
+++ b/net/af-xdp.c
22
@@ -XXX,XX +XXX,XX @@ int net_init_af_xdp(const Netdev *netdev,
23
NetClientState *nc, *nc0 = NULL;
24
unsigned int ifindex;
25
uint32_t prog_id = 0;
26
- int *sock_fds = NULL;
27
+ g_autofree int *sock_fds = NULL;
28
int64_t i, queues;
29
Error *err = NULL;
30
AFXDPState *s;
31
@@ -XXX,XX +XXX,XX @@ int net_init_af_xdp(const Netdev *netdev,
32
return 0;
33
34
err:
35
- g_free(sock_fds);
36
if (nc0) {
37
qemu_del_net_client(nc0);
38
}
39
--
40
2.34.1
diff view generated by jsdifflib
New patch
1
In pca9554_get_pin() and pca9554_set_pin(), we try to detect an
2
incorrect pin value, but we get the condition wrong, using ">"
3
when ">=" was intended.
1
4
5
This has no actual effect, because in pca9554_initfn() we
6
use the correct test when creating the properties and so
7
we'll never be called with an out of range value. However,
8
Coverity complains about the mismatch between the check and
9
the later use of the pin value in a shift operation.
10
11
Use the correct condition.
12
13
Resolves: Coverity CID 1534917
14
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
15
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
16
Reviewed-by: Thomas Huth <thuth@redhat.com>
17
Message-id: 20240312183810.557768-5-peter.maydell@linaro.org
18
---
19
hw/misc/pca9554.c | 4 ++--
20
1 file changed, 2 insertions(+), 2 deletions(-)
21
22
diff --git a/hw/misc/pca9554.c b/hw/misc/pca9554.c
23
index XXXXXXX..XXXXXXX 100644
24
--- a/hw/misc/pca9554.c
25
+++ b/hw/misc/pca9554.c
26
@@ -XXX,XX +XXX,XX @@ static void pca9554_get_pin(Object *obj, Visitor *v, const char *name,
27
error_setg(errp, "%s: error reading %s", __func__, name);
28
return;
29
}
30
- if (pin < 0 || pin > PCA9554_PIN_COUNT) {
31
+ if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
32
error_setg(errp, "%s invalid pin %s", __func__, name);
33
return;
34
}
35
@@ -XXX,XX +XXX,XX @@ static void pca9554_set_pin(Object *obj, Visitor *v, const char *name,
36
error_setg(errp, "%s: error reading %s", __func__, name);
37
return;
38
}
39
- if (pin < 0 || pin > PCA9554_PIN_COUNT) {
40
+ if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
41
error_setg(errp, "%s invalid pin %s", __func__, name);
42
return;
43
}
44
--
45
2.34.1
diff view generated by jsdifflib
New patch
1
There's no way for the macio_nvram device to report failure to write
2
data, but we can at least report it to the user with error_report()
3
as we do in other devices like xlnx-efuse.
1
4
5
Spotted by Coverity.
6
7
Resolves: Coverity CID 1507628
8
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
9
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
10
Reviewed-by: Thomas Huth <thuth@redhat.com>
11
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
12
Message-id: 20240312183810.557768-6-peter.maydell@linaro.org
13
---
14
hw/nvram/mac_nvram.c | 5 ++++-
15
1 file changed, 4 insertions(+), 1 deletion(-)
16
17
diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c
18
index XXXXXXX..XXXXXXX 100644
19
--- a/hw/nvram/mac_nvram.c
20
+++ b/hw/nvram/mac_nvram.c
21
@@ -XXX,XX +XXX,XX @@ static void macio_nvram_writeb(void *opaque, hwaddr addr,
22
trace_macio_nvram_write(addr, value);
23
s->data[addr] = value;
24
if (s->blk) {
25
- blk_pwrite(s->blk, addr, 1, &s->data[addr], 0);
26
+ if (blk_pwrite(s->blk, addr, 1, &s->data[addr], 0) < 0) {
27
+ error_report("%s: write of NVRAM data to backing store failed",
28
+ blk_name(s->blk));
29
+ }
30
}
31
}
32
33
--
34
2.34.1
35
36
diff view generated by jsdifflib
New patch
1
In test_compute_wait() we do
2
double units = bkt.max / 10;
3
which does an integer division and then assigns it to a double variable,
4
and similarly later on in the expression for an assertion.
1
5
6
Use 10.0 so that we do a floating point division and calculate the
7
exact value, rather than doing an integer division.
8
9
Spotted by Coverity.
10
11
Resolves: Coverity CID 1432564
12
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
13
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
14
Reviewed-by: Thomas Huth <thuth@redhat.com>
15
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
16
Message-id: 20240312183810.557768-7-peter.maydell@linaro.org
17
---
18
tests/unit/test-throttle.c | 4 ++--
19
1 file changed, 2 insertions(+), 2 deletions(-)
20
21
diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
22
index XXXXXXX..XXXXXXX 100644
23
--- a/tests/unit/test-throttle.c
24
+++ b/tests/unit/test-throttle.c
25
@@ -XXX,XX +XXX,XX @@ static void test_compute_wait(void)
26
bkt.avg = 10;
27
bkt.max = 200;
28
for (i = 0; i < 22; i++) {
29
- double units = bkt.max / 10;
30
+ double units = bkt.max / 10.0;
31
bkt.level += units;
32
bkt.burst_level += units;
33
throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 10);
34
wait = throttle_compute_wait(&bkt);
35
g_assert(double_cmp(bkt.burst_level, 0));
36
- g_assert(double_cmp(bkt.level, (i + 1) * (bkt.max - bkt.avg) / 10));
37
+ g_assert(double_cmp(bkt.level, (i + 1) * (bkt.max - bkt.avg) / 10.0));
38
/* We can do bursts for the 2 seconds we have configured in
39
* burst_length. We have 100 extra milliseconds of burst
40
* because bkt.level has been leaking during this time.
41
--
42
2.34.1
43
44
diff view generated by jsdifflib
1
From: Philippe Mathieu-Daudé <philmd@linaro.org>
1
Coverity points out that g_setenv() can fail and we don't
2
check for this in qtest_inproc_init(). In practice this will
3
only fail if a memory allocation failed in setenv() or if
4
the caller passed an invalid architecture name (e.g. one
5
with an '=' in it), so rather than requiring the callsite
6
to check for failure, make g_setenv() failure fatal here,
7
similarly to what we did in commit aca68d95c515.
2
8
3
Cortex-M profile is only emulable from TCG accelerator. Restrict
9
Resolves: Coverity CID 1497485
4
the GDBstub features to its availability in order to avoid a link
10
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
5
error when TCG is not enabled:
11
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
12
Reviewed-by: Thomas Huth <thuth@redhat.com>
13
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
14
Message-id: 20240312183810.557768-8-peter.maydell@linaro.org
15
---
16
tests/qtest/libqtest.c | 6 +++++-
17
1 file changed, 5 insertions(+), 1 deletion(-)
6
18
7
Undefined symbols for architecture arm64:
19
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
8
"_arm_v7m_get_sp_ptr", referenced from:
9
_m_sysreg_get in target_arm_gdbstub.c.o
10
"_arm_v7m_mrs_control", referenced from:
11
_arm_gdb_get_m_systemreg in target_arm_gdbstub.c.o
12
ld: symbol(s) not found for architecture arm64
13
clang: error: linker command failed with exit code 1 (use -v to see invocation)
14
15
Fixes: 7d8b28b8b5 ("target/arm: Implement gdbstub m-profile systemreg and secext")
16
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
17
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
18
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
19
Message-id: 20230322142902.69511-3-philmd@linaro.org
20
[PMM: add #include since I cherry-picked this patch from the series]
21
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
22
---
23
target/arm/gdbstub.c | 5 +++--
24
1 file changed, 3 insertions(+), 2 deletions(-)
25
26
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
27
index XXXXXXX..XXXXXXX 100644
20
index XXXXXXX..XXXXXXX 100644
28
--- a/target/arm/gdbstub.c
21
--- a/tests/qtest/libqtest.c
29
+++ b/target/arm/gdbstub.c
22
+++ b/tests/qtest/libqtest.c
30
@@ -XXX,XX +XXX,XX @@
23
@@ -XXX,XX +XXX,XX @@ QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
31
#include "cpu.h"
24
* way, qtest_get_arch works for inproc qtest.
32
#include "exec/gdbstub.h"
25
*/
33
#include "gdbstub/helpers.h"
26
gchar *bin_path = g_strconcat("/qemu-system-", arch, NULL);
34
+#include "sysemu/tcg.h"
27
- g_setenv("QTEST_QEMU_BINARY", bin_path, 0);
35
#include "internals.h"
28
+ if (!g_setenv("QTEST_QEMU_BINARY", bin_path, 0)) {
36
#include "cpregs.h"
29
+ fprintf(stderr,
37
30
+ "Could not set environment variable QTEST_QEMU_BINARY\n");
38
@@ -XXX,XX +XXX,XX @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
31
+ exit(1);
39
2, "arm-vfp-sysregs.xml", 0);
32
+ }
40
}
33
g_free(bin_path);
41
}
34
42
- if (cpu_isar_feature(aa32_mve, cpu)) {
35
return qts;
43
+ if (cpu_isar_feature(aa32_mve, cpu) && tcg_enabled()) {
44
gdb_register_coprocessor(cs, mve_gdb_get_reg, mve_gdb_set_reg,
45
1, "arm-m-profile-mve.xml", 0);
46
}
47
@@ -XXX,XX +XXX,XX @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
48
arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
49
"system-registers.xml", 0);
50
51
- if (arm_feature(env, ARM_FEATURE_M)) {
52
+ if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
53
gdb_register_coprocessor(cs,
54
arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg,
55
arm_gen_dynamic_m_systemreg_xml(cs, cs->gdb_num_regs),
56
--
36
--
57
2.34.1
37
2.34.1
58
38
59
39
diff view generated by jsdifflib