[PATCH] hw/net/virtio-net.c: fix crash in iov_copy()

2024-05-27 Thread Dmitry Frolov
A crash found while fuzzing device virtio-net-socket-check-used.
Assertion "offset == 0" in iov_copy() fails if less than guest_hdr_len bytes
were transmited.

Signed-off-by: Dmitry Frolov 
---
 hw/net/virtio-net.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 24e5e7d347..603b80a50a 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2783,6 +2783,12 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
  */
 assert(n->host_hdr_len <= n->guest_hdr_len);
 if (n->host_hdr_len != n->guest_hdr_len) {
+if (iov_size(out_sg, out_num) < n->guest_hdr_len) {
+virtio_error(vdev, "virtio-net header is invalid");
+virtqueue_detach_element(q->tx_vq, elem, 0);
+g_free(elem);
+return -EINVAL;
+}
 unsigned sg_num = iov_copy(sg, ARRAY_SIZE(sg),
out_sg, out_num,
0, n->host_hdr_len);
-- 
2.43.0




[PATCH] tests/qtest/fuzz/virtio_net_fuzz.c: fix virtio_net_fuzz_multi

2024-05-23 Thread Dmitry Frolov
If QTestState was already CLOSED due to error, calling qtest_clock_step()
afterwards makes no sense and only raises false-crash with message:
"assertion timer != NULL failed".

Signed-off-by: Dmitry Frolov 
---
 tests/qtest/fuzz/virtio_net_fuzz.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c 
b/tests/qtest/fuzz/virtio_net_fuzz.c
index e239875e3b..2f57a8ddd8 100644
--- a/tests/qtest/fuzz/virtio_net_fuzz.c
+++ b/tests/qtest/fuzz/virtio_net_fuzz.c
@@ -81,6 +81,9 @@ static void virtio_net_fuzz_multi(QTestState *s,
 /* Run the main loop */
 qtest_clock_step(s, 100);
 flush_events(s);
+if (!qtest_probe_child(s)) {
+return;
+}
 
 /* Wait on used descriptors */
 if (check_used && !vqa.rx) {
-- 
2.43.0




[PATCH] tests/qtest/fuzz: fix memleak in qos_fuzz.c

2024-05-21 Thread Dmitry Frolov
Found with fuzzing for qemu-8.2, but also relevant for master

Signed-off-by: Dmitry Frolov 
---
 tests/qtest/fuzz/qos_fuzz.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index b71e945c5f..d3839bf999 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -180,6 +180,7 @@ static void walk_path(QOSGraphNode *orig_path, int len)
 
 fuzz_path_vec = path_vec;
 } else {
+g_string_free(cmd_line, true);
 g_free(path_vec);
 }
 
-- 
2.43.0




[PATCH] hw/virtio: remove meaningless NULL-check

2023-12-13 Thread Dmitry Frolov
vdev is being dereferenced in the first line of the function.
The following NULL-check makes no sense.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Frolov 
---
 hw/virtio/virtio-bus.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 896feb37a1..0436f545b8 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -119,10 +119,8 @@ void virtio_bus_device_unplugged(VirtIODevice *vdev)
 
 DPRINTF("%s: remove device.\n", qbus->name);
 
-if (vdev != NULL) {
-if (klass->device_unplugged != NULL) {
-klass->device_unplugged(qbus->parent);
-}
+if (klass->device_unplugged != NULL) {
+klass->device_unplugged(qbus->parent);
 }
 }
 
-- 
2.34.1




[PATCH] ui: fix DIV_BY_ZERO in tightvnc

2023-12-12 Thread Dmitry Frolov
Division by zero may occur in rare constellation of conditions if:
1. not TrueColor mode on the client side
   tight_detect_smooth_image16() and tight_detect_smooth_image32(),
   defined by macro DEFINE_DETECT_FUNCTION()2, are affected.
2. if all pixels on the screen are equal, then pixels == stats[0]

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Frolov 
---
 ui/vnc-enc-tight.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 41f559eb83..f1249ab136 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -284,6 +284,9 @@ tight_detect_smooth_image24(VncState *vs, int w, int h)
 for (; c < 256; c++) {  \
 errors += stats[c] * (c * c);   \
 }   \
+if (pixels == stats[0]) {   \
+return 0;   \
+}   \
 errors /= (pixels - stats[0]);  \
 \
 return errors;  \
-- 
2.34.1




[RFC PATCH] hw/virtio: fix confusing comment

2023-11-24 Thread Dmitry Frolov
It seems that comments to transitional/non-transitional devices are
mixed up.

Signed-off-by: Dmitry Frolov 
---
 include/hw/virtio/virtio-pci.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 5a3f182f99..45be10d694 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -229,13 +229,13 @@ typedef struct VirtioPCIDeviceTypeInfo {
 /*
  * The transitional device type.  Optional.
  *
- * Implements both INTERFACE_PCIE_DEVICE and 
INTERFACE_CONVENTIONAL_PCI_DEVICE.
+ * Implements INTERFACE_CONVENTIONAL_PCI_DEVICE only.
  */
 const char *transitional_name;
 /*
  * The non-transitional device type.  Optional.
  *
- * Implements INTERFACE_CONVENTIONAL_PCI_DEVICE only.
+ * Implements both INTERFACE_PCIE_DEVICE and 
INTERFACE_CONVENTIONAL_PCI_DEVICE.
  */
 const char *non_transitional_name;
 
-- 
2.34.1




[PATCH] block/monitor: blk_bs() return value check

2023-11-24 Thread Dmitry Frolov
blk_bs() may return NULL, which will be dereferenced without a check in
bdrv_commit().

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Frolov 
---
 block/monitor/block-hmp-cmds.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index c729cbf1eb..ade627bc27 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -221,7 +221,13 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
 return;
 }
 
-bs = bdrv_skip_implicit_filters(blk_bs(blk));
+bs = blk_bs(blk);
+if (!bs) {
+error_report("Device '%s' is invalid", device);
+return;
+}
+
+bs = bdrv_skip_implicit_filters(bs);
 aio_context = bdrv_get_aio_context(bs);
 aio_context_acquire(aio_context);
 
-- 
2.34.1




[PATCH v1] migration: fix RAMBlock add NULL check

2023-10-10 Thread Dmitry Frolov
qemu_ram_block_from_host() may return NULL, which will be dereferenced w/o
check. Usualy return value is checked for this function.
Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: c7c0e72408df5e7821c0e995122fb2fe0ac001f1 ("migration/ram: Handle RAM 
block resizes during precopy")
Signed-off-by: Dmitry Frolov 
---
 migration/ram.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index e4bfd39f08..bd4b7574e1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4281,6 +4281,11 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier 
*n, void *host,
 RAMBlock *rb = qemu_ram_block_from_host(host, false, );
 Error *err = NULL;
 
+if (!rb) {
+error_report("RAM block not found");
+return;
+}
+
 if (migrate_ram_is_ignored(rb)) {
 return;
 }
-- 
2.34.1




[PATCH v3] hw/cxl: Fix out of bound array access

2023-09-14 Thread Dmitry Frolov
According to cxl_interleave_ways_enc(), fw->num_targets is allowed to be up
to 16. This also corresponds to CXL specs. So, the fw->target_hbs[] array
is iterated from 0 to 15. But it is statically declared of length 8. Thus,
out of bound array access may occur.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

v2: assert added
v3: assert removed

Fixes: c28db9e000 ("hw/pci-bridge: Make PCIe and CXL PXB Devices inherit from 
TYPE_PXB_DEV")
Signed-off-by: Dmitry Frolov 
---
 include/hw/cxl/cxl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index 56c9e7676e..4944725849 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -29,7 +29,7 @@ typedef struct PXBCXLDev PXBCXLDev;
 typedef struct CXLFixedWindow {
 uint64_t size;
 char **targets;
-PXBCXLDev *target_hbs[8];
+PXBCXLDev *target_hbs[16];
 uint8_t num_targets;
 uint8_t enc_int_ways;
 uint8_t enc_int_gran;
-- 
2.34.1




[PATCH v2] hw/cxl: Fix out of bound array access

2023-09-13 Thread Dmitry Frolov
According to cxl_interleave_ways_enc(),
fw->num_targets is allowed to be up to 16.
This also corresponds to CXL specs.
So, the fw->target_hbs[] array is iterated from 0 to 15.
But it is staticaly declared of length 8.
Thus, out of bound array access may occur.

Fixes: c28db9e000 ("hw/pci-bridge: Make PCIe and CXL PXB Devices inherit from 
TYPE_PXB_DEV")

Signed-off-by: Dmitry Frolov 
---
v2: assert added
---
 hw/cxl/cxl-host.c| 1 +
 include/hw/cxl/cxl.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 034c7805b3..fe9143409b 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -33,6 +33,7 @@ static void cxl_fixed_memory_window_config(CXLState 
*cxl_state,
 for (target = object->targets; target; target = target->next) {
 fw->num_targets++;
 }
+assert(fw->num_targets <= ARRAY_SIZE(fw->target_hbs));
 
 fw->enc_int_ways = cxl_interleave_ways_enc(fw->num_targets, errp);
 if (*errp) {
diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index 56c9e7676e..4944725849 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -29,7 +29,7 @@ typedef struct PXBCXLDev PXBCXLDev;
 typedef struct CXLFixedWindow {
 uint64_t size;
 char **targets;
-PXBCXLDev *target_hbs[8];
+PXBCXLDev *target_hbs[16];
 uint8_t num_targets;
 uint8_t enc_int_ways;
 uint8_t enc_int_gran;
-- 
2.34.1




[PATCH] hw/cxl: Fix out of bound array access

2023-09-13 Thread Dmitry Frolov
According to cxl_interleave_ways_enc(),
fw->num_targets is allowed to be up to 16.
This also corresponds to CXL specs.
So, the fw->target_hbs[] array is iterated from 0 to 15.
But it is staticaly declared of length 8.
Thus, out of bound array access may occur.

Fixes: c28db9e000 ("hw/pci-bridge: Make PCIe and CXL PXB Devices inherit from 
TYPE_PXB_DEV")

Signed-off-by: Dmitry Frolov 
---
 include/hw/cxl/cxl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index 56c9e7676e..4944725849 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -29,7 +29,7 @@ typedef struct PXBCXLDev PXBCXLDev;
 typedef struct CXLFixedWindow {
 uint64_t size;
 char **targets;
-PXBCXLDev *target_hbs[8];
+PXBCXLDev *target_hbs[16];
 uint8_t num_targets;
 uint8_t enc_int_ways;
 uint8_t enc_int_gran;
-- 
2.34.1




[PATCH] fix bdrv_open_child return value check

2023-08-31 Thread Dmitry Frolov
bdrv_open_child() may return NULL.
Usually return value is checked for this function.
Check for return value is more reliable.

Fixes: 24bc15d1f6 ("vmdk: Use BdrvChild instead of BDS for references to 
extents")

Signed-off-by: Dmitry Frolov 
---
 block/vmdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 70066c2b01..58ce290e9c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1207,7 +1207,7 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
   bs, _of_bds, extent_role, false,
   _err);
 g_free(extent_path);
-if (local_err) {
+if (!extent_file) {
 error_propagate(errp, local_err);
 ret = -EINVAL;
 goto out;
-- 
2.34.1




[PATCH] fix leaks found wtih fuzzing

2023-08-25 Thread Dmitry Frolov
It is true, that there is no problem during runtime
from the first sight, because the memory is lost just
before qemu exits. Nevertheless, this change is necessary,
because AddressSanitizer is not able to recognize this
situation and produces crash-report (which is
false-positive in fact). Lots of False-Positive warnings
are davaluing problems, found with fuzzing, and thus the
whole methodology of dynamic analysis.
This patch eliminates such False-Positive reports,
and makes every problem, found with fuzzing, more valuable.

Fixes: 060ab76356 ("gtk: don't exit early in case gtk init fails")

Signed-off-by: Dmitry Frolov 
---
v2: Moved declarations in the beginning.
v3: Fixed errors in v2.

 ui/gtk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 8ba41c8f13..7db972732b 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2360,7 +2360,7 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
 {
 VirtualConsole *vc;
 
-GtkDisplayState *s = g_malloc0(sizeof(*s));
+GtkDisplayState *s;
 GdkDisplay *window_display;
 GtkIconTheme *theme;
 char *dir;
@@ -2370,6 +2370,7 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
 exit(1);
 }
 assert(opts->type == DISPLAY_TYPE_GTK);
+s = g_malloc0(sizeof(*s));
 s->opts = opts;
 
 theme = gtk_icon_theme_get_default();
-- 
2.34.1




[PATCH] fix leaks found wtih fuzzing

2023-08-25 Thread Dmitry Frolov
It is true, that there is no problem during runtime
from the first sight, because the memmory is lost just
before qemu exits. Nevertheless, this change is necessary,
because AddressSanitizer is not able to recognize this
situation and produces crash-report (which is
false-positive in fact). Lots of False-Positive warnings
are davaluing problems, found with fuzzing, and thus the
whole methodology of dynamic analysis.
This patch eliminates such False-Positive reports,
and makes every problem, found with fuzzing, more valuable.

Fixes: 060ab76356 ("gtk: don't exit early in case gtk init fails")

Signed-off-by: Dmitry Frolov 
---
v2: Moved declarations in the beginning.

 ui/gtk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 8ba41c8f13..23a78787df 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2360,7 +2360,7 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
 {
 VirtualConsole *vc;
 
-GtkDisplayState *s = g_malloc0(sizeof(*s));
+GtkDisplayState *s;
 GdkDisplay *window_display;
 GtkIconTheme *theme;
 char *dir;
@@ -2372,6 +2372,7 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
 assert(opts->type == DISPLAY_TYPE_GTK);
 s->opts = opts;
 
+*s = g_malloc0(sizeof(*s));
 theme = gtk_icon_theme_get_default();
 dir = get_relocated_path(CONFIG_QEMU_ICONDIR);
 gtk_icon_theme_prepend_search_path(theme, dir);
-- 
2.34.1




[PATCH] fix leaks found wtih fuzzing

2023-08-24 Thread Dmitry Frolov
Fuzzing causes thousands of identical crashes with message:
"AddressSanitizer: 3744 byte(s) leaked in 1 allocation(s)"

Fixes: 060ab76356 ("gtk: don't exit early in case gtk init fails")

Signed-off-by: Dmitry Frolov 
---
 ui/gtk.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 8ba41c8f13..996ca7949d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2358,6 +2358,10 @@ static gboolean gtkinit;
 
 static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
 {
+if (!gtkinit) {
+fprintf(stderr, "gtk initialization failed\n");
+exit(1);
+}
 VirtualConsole *vc;
 
 GtkDisplayState *s = g_malloc0(sizeof(*s));
@@ -2365,10 +2369,6 @@ static void gtk_display_init(DisplayState *ds, 
DisplayOptions *opts)
 GtkIconTheme *theme;
 char *dir;
 
-if (!gtkinit) {
-fprintf(stderr, "gtk initialization failed\n");
-exit(1);
-}
 assert(opts->type == DISPLAY_TYPE_GTK);
 s->opts = opts;
 
-- 
2.34.1