From 1c0cf16e49544a849b5382cc0622dc7cd01f0f36 Mon Sep 17 00:00:00 2001 From: William Henderson Date: Mon, 18 Sep 2023 14:30:04 +0100 Subject: [PATCH] fix: minor memory bugs #784 Fixes the following Coverity reports: ________________________________________________________________________________________________________ *** CID 417161: Memory - corruptions (ARRAY_VS_SINGLETON) /samples/server.c: 438 in migration_write_data() 432 } 433 434 /* write to bar0, if any */ 435 if (write_end > server_data->bar1_size) { 436 length_in_bar0 = write_end - write_start; 437 write_start -= server_data->bar1_size; CID 417161: Memory - corruptions (ARRAY_VS_SINGLETON) Using "&server_data->bar0" as an array. This might corrupt or misinterpret adjacent memory locations. 438 memcpy(&server_data->bar0 + write_start, buf + length_in_bar1, 439 length_in_bar0); 440 } 441 442 server_data->migration.bytes_transferred += bytes_written; 443 ________________________________________________________________________________________________________ *** CID 417160: Memory - corruptions (ARRAY_VS_SINGLETON) /samples/server.c: 394 in migration_read_data() 388 } 389 390 /* read bar0, if any */ 391 if (read_end > server_data->bar1_size) { 392 length_in_bar0 = read_end - read_start; 393 read_start -= server_data->bar1_size; CID 417160: Memory - corruptions (ARRAY_VS_SINGLETON) Using "&server_data->bar0" as an array. This might corrupt or misinterpret adjacent memory locations. 394 memcpy(buf + length_in_bar1, &server_data->bar0 + read_start, 395 length_in_bar0); 396 } 397 398 server_data->migration.bytes_transferred += bytes_read; 399 ________________________________________________________________________________________________________ *** CID 417159: Possible Control flow issues (DEADCODE) /lib/libvfio-user.c: 121 in dev_get_caps() 115 116 header = (struct vfio_info_cap_header*)(vfio_reg + 1); 117 118 if (vfu_reg->mmap_areas != NULL) { 119 int i, nr_mmap_areas = vfu_reg->nr_mmap_areas; 120 if (type != NULL) { CID 417159: Possible Control flow issues (DEADCODE) Execution cannot reach this statement: "type->header.next = vfio_re...". 121 type->header.next = vfio_reg->cap_offset + sizeof(struct vfio_region_info_cap_type); 122 sparse = (struct vfio_region_info_cap_sparse_mmap*)(type + 1); 123 } else { 124 vfio_reg->cap_offset = sizeof(struct vfio_region_info); 125 sparse = (struct vfio_region_info_cap_sparse_mmap*)header; 126 } Signed-off-by: William Henderson --- lib/libvfio-user.c | 10 ++-------- samples/server.c | 4 ++-- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index 81b00108..47ae5226 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -105,7 +105,6 @@ dev_get_caps(vfu_ctx_t *vfu_ctx, vfu_reg_info_t *vfu_reg, struct vfio_region_info *vfio_reg, int **fds, size_t *nr_fds) { struct vfio_info_cap_header *header; - struct vfio_region_info_cap_type *type = NULL; struct vfio_region_info_cap_sparse_mmap *sparse = NULL; assert(vfu_ctx != NULL); @@ -117,13 +116,8 @@ dev_get_caps(vfu_ctx_t *vfu_ctx, vfu_reg_info_t *vfu_reg, if (vfu_reg->mmap_areas != NULL) { int i, nr_mmap_areas = vfu_reg->nr_mmap_areas; - if (type != NULL) { - type->header.next = vfio_reg->cap_offset + sizeof(struct vfio_region_info_cap_type); - sparse = (struct vfio_region_info_cap_sparse_mmap*)(type + 1); - } else { - vfio_reg->cap_offset = sizeof(struct vfio_region_info); - sparse = (struct vfio_region_info_cap_sparse_mmap*)header; - } + vfio_reg->cap_offset = sizeof(struct vfio_region_info); + sparse = (struct vfio_region_info_cap_sparse_mmap *)header; *fds = malloc(nr_mmap_areas * sizeof(int)); if (*fds == NULL) { diff --git a/samples/server.c b/samples/server.c index 5edf6746..d0707f3d 100644 --- a/samples/server.c +++ b/samples/server.c @@ -391,7 +391,7 @@ migration_read_data(vfu_ctx_t *vfu_ctx, void *buf, uint64_t size) if (read_end > server_data->bar1_size) { length_in_bar0 = read_end - read_start; read_start -= server_data->bar1_size; - memcpy(buf + length_in_bar1, &server_data->bar0 + read_start, + memcpy(buf + length_in_bar1, (char *)&server_data->bar0 + read_start, length_in_bar0); } @@ -435,7 +435,7 @@ migration_write_data(vfu_ctx_t *vfu_ctx, void *data, uint64_t size) if (write_end > server_data->bar1_size) { length_in_bar0 = write_end - write_start; write_start -= server_data->bar1_size; - memcpy(&server_data->bar0 + write_start, buf + length_in_bar1, + memcpy((char *)&server_data->bar0 + write_start, buf + length_in_bar1, length_in_bar0); }