From: Yonghua Huang <yonghua.huang@...>

  this patch fix several issues that NULL pointers
  maybe be dereferenced in display module.

Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
 devicemodel/hw/pci/virtio/virtio_gpu.c | 61 ++++++++++++++++++++++--
--
 devicemodel/hw/vdisplay_sdl.c          | 13 ++++--
 devicemodel/hw/vga.c                   | 10 ++++-
 3 files changed, 71 insertions(+), 13 deletions(-)

diff --git a/devicemodel/hw/pci/virtio/virtio_gpu.c
b/devicemodel/hw/pci/virtio/virtio_gpu.c
index 8b2399b35..6e1ba248b 100644
--- a/devicemodel/hw/pci/virtio/virtio_gpu.c
+++ b/devicemodel/hw/pci/virtio/virtio_gpu.c
@@ -701,6 +701,12 @@ virtio_gpu_cmd_resource_create_2d(struct
virtio_gpu_command *cmd)
        }
        r2d = (struct virtio_gpu_resource_2d*)calloc(1, \
                        sizeof(struct virtio_gpu_resource_2d));
+       if (!r2d) {
+               pr_err("%s: memory allocation for r2d failed.\n",
__func__);
+               resp.type = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+               goto response;
+       }
+
        r2d->resource_id = req.resource_id;
        r2d->width = req.width;
        r2d->height = req.height;
@@ -773,15 +779,27 @@ virtio_gpu_cmd_resource_attach_backing(struct
virtio_gpu_command *cmd)
        struct virtio_gpu_ctrl_hdr resp;
        int i;
        uint8_t *pbuf;
+       struct iovec *iov;
 
        memcpy(&req, cmd->iov[0].iov_base, sizeof(req));
        memset(&resp, 0, sizeof(resp));
 
        r2d = virtio_gpu_find_resource_2d(cmd->gpu, req.resource_id);
        if (r2d) {
-               r2d->iov = malloc(req.nr_entries * sizeof(struct
iovec));
+               iov = malloc(req.nr_entries * sizeof(struct iovec));
+               if (!iov) {
+                       resp.type =
VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+                       goto exit;
+               }
+
+               r2d->iov = iov;
                r2d->iovcnt = req.nr_entries;
                entries = malloc(req.nr_entries * sizeof(struct
virtio_gpu_mem_entry));
+               if (!entries) {
+                       free(iov);
+                       resp.type =
VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+                       goto exit;
+               }
                pbuf = (uint8_t*)entries;
                for (i = 1; i < (cmd->iovcnt - 1); i++) {
                        memcpy(pbuf, cmd->iov[i].iov_base, cmd-

iov[i].iov_len);

@@ -800,8 +818,10 @@ virtio_gpu_cmd_resource_attach_backing(struct
virtio_gpu_command *cmd)
                resp.type = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
        }
 
-       cmd->iolen = sizeof(resp);
        resp.type = VIRTIO_GPU_RESP_OK_NODATA;
+
+exit:
+       cmd->iolen = sizeof(resp);
        virtio_gpu_update_resp_fence(&cmd->hdr, &resp);
        memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
 }
@@ -1165,6 +1185,7 @@ virtio_gpu_cmd_create_blob(struct
virtio_gpu_command *cmd)
        struct virtio_gpu_ctrl_hdr resp;
        int i;
        uint8_t *pbuf;
+       struct iovec *iov;
 
        memcpy(&req, cmd->iov[0].iov_base, sizeof(req));
        cmd->iolen = sizeof(resp);
@@ -1199,9 +1220,23 @@ virtio_gpu_cmd_create_blob(struct
virtio_gpu_command *cmd)
 
        r2d = (struct virtio_gpu_resource_2d *)calloc(1,
                        sizeof(struct virtio_gpu_resource_2d));
+       if (!r2d) {
+               pr_err("%s : memory allocation for r2d failed.\n",
__func__);
+               resp.type = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+               memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
+               return;
+       }
+
        r2d->resource_id = req.resource_id;
 
        entries = malloc(req.nr_entries * sizeof(struct
virtio_gpu_mem_entry));
+       if (!entries) {
+               pr_err("%s : memory allocation for entries
failed.\n", __func__);
+               free(r2d);
+               resp.type = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+               memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
+               return;
+       }
        pbuf = (uint8_t *)entries;
        for (i = 1; i < (cmd->iovcnt - 1); i++) {
                memcpy(pbuf, cmd->iov[i].iov_base, cmd-

iov[i].iov_len);

@@ -1229,7 +1264,16 @@ virtio_gpu_cmd_create_blob(struct
virtio_gpu_command *cmd)
                r2d->image = pixman_image_create_bits(
                                r2d->format, r2d->width, r2d->height,
NULL, 0);
 
-               r2d->iov = malloc(req.nr_entries * sizeof(struct
iovec));
+               iov = malloc(req.nr_entries * sizeof(struct iovec));
+               if (!iov) {
+                       free(entries);
+                       free(r2d);
+                       resp.type =
VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+                       memcpy(cmd->iov[cmd->iovcnt - 1].iov_base,
&resp, sizeof(resp));
+                       return;
+               }
+               r2d->iov = iov;
+
                r2d->iovcnt = req.nr_entries;
                for (i = 0; i < req.nr_entries; i++) {
                        r2d->iov[i].iov_base = paddr_guest2host(
@@ -1551,7 +1595,7 @@ virtio_gpu_vga_render(void *param)
        gpu->vga.surf.stride = 0;
        /* The below logic needs to be refined */
        while(gpu->vga.enable) {
-               if(gpu->vga.gc->gc_image->vgamode) {
+               if((gpu->vga.gc->gc_image->vgamode) && (gpu->vga.dev
!= NULL)) {
                        vga_render(gpu->vga.gc, gpu->vga.dev);
                        break;
                }
@@ -1796,6 +1840,9 @@ virtio_gpu_deinit(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)
        int i;
 
        gpu = (struct virtio_gpu *)dev->arg;
+       if (!gpu)
+               return;
+
        gpu->vga.enable = false;
 
        pthread_mutex_lock(&gpu->vga_thread_mtx);
@@ -1855,10 +1902,8 @@ virtio_gpu_deinit(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)
 
        vdpy_deinit(gpu->vdpy_handle);
 
-       if (gpu) {
-               pthread_mutex_destroy(&gpu->mtx);
-               free(gpu);
-       }
+       pthread_mutex_destroy(&gpu->mtx);
+       free(gpu);
        virtio_gpu_device_cnt--;
 }
 
diff --git a/devicemodel/hw/vdisplay_sdl.c
b/devicemodel/hw/vdisplay_sdl.c
index 9bb2f7360..f83b92e6a 100644
--- a/devicemodel/hw/vdisplay_sdl.c
+++ b/devicemodel/hw/vdisplay_sdl.c
@@ -1485,14 +1485,19 @@ int vdpy_parse_cmd_option(const char *opts)
        struct vscreen *vscr;
 
        error = 0;
-       vdpy.vscrs = calloc(VSCREEN_MAX_NUM, sizeof(struct vscreen));
+       vscr = calloc(VSCREEN_MAX_NUM, sizeof(struct vscreen));
+       if (!vscr) {
+               pr_err("%s, memory allocation for vscrs failed.",
__func__);
+               return -1;
+       }
+       vdpy.vscrs = vscr;
        vdpy.vscrs_num = 0;
        vscr = vdpy.vscrs + vdpy.vscrs_num;
 
        stropts = strdup(opts);
        while ((str = strsep(&stropts, ",")) != NULL) {
                tmp = strcasestr(str, "geometry=");
-               if (str && strcasestr(str, "geometry=fullscreen")) {
+               if (tmp && str && strcasestr(str,
"geometry=fullscreen")) {
                        vscr = vdpy.vscrs + vdpy.vscrs_num;
                        snum = sscanf(tmp, "geometry=fullscreen:%d",
&vscr->pscreen_id);
                        if (snum != 1) {
@@ -1506,7 +1511,7 @@ int vdpy_parse_cmd_option(const char *opts)
                        pr_info("virtual display: fullscreen on
monitor %d\n",
                                        vscr->pscreen_id);
                        vdpy.vscrs_num++;
-               } else if (str && strcasestr(str, "geometry=")) {
+               } else if (tmp && str && strcasestr(str,
"geometry=")) {
                        vscr = vdpy.vscrs + vdpy.vscrs_num;
                        snum = sscanf(tmp, "geometry=%dx%d+%d+%d",
                                        &vscr->guest_width, &vscr-

guest_height,

@@ -1524,7 +1529,7 @@ int vdpy_parse_cmd_option(const char *opts)
                }
 
                tmp = strcasestr(str, "rotation=");
-               if (str && strcasestr(str, "rotation=")) {
+               if (tmp && str && strcasestr(str, "rotation=")) {
                        snum = sscanf(tmp, "rotation=%d", &vscr-

rotation);

                        if (snum != 1) {
                                vscr->rotation = 0;
diff --git a/devicemodel/hw/vga.c b/devicemodel/hw/vga.c
index 19cc06428..1776cfb05 100644
--- a/devicemodel/hw/vga.c
+++ b/devicemodel/hw/vga.c
@@ -1235,8 +1235,11 @@ vga_init(struct gfx_ctx *gc, int io_only)
        struct inout_port iop;
        struct vga_vdev *vd;
        int port, error;
+       uint8_t *vga_ram;
 
        vd = calloc(1, sizeof(struct vga_vdev));
+       if (!vd)
+               return NULL;
 
        bzero(&iop, sizeof(struct inout_port));
        iop.name = "VGA";
@@ -1272,7 +1275,12 @@ vga_init(struct gfx_ctx *gc, int io_only)
                return NULL;
        }
 
-       vd->vga_ram = malloc(256 * KB);
+       vga_ram = malloc(256 * KB);
+       if (!vga_ram) {
+               free(vd);
+               return NULL;
+       }
+       vd->vga_ram = vga_ram;
        memset(vd->vga_ram, 0, 256 * KB);
 
        {

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of Sun, Peng
Sent: Tuesday, November 15, 2022 11:35
To: acrn-dev@...
Subject: Re: [acrn-dev] [PATCH v2 2/3] dm:ivi:fix NULL pointer dereference risk
in vdisplay

LGTM.
Reviewed-by: Sun, Peng <peng.p.sun@...>

On Tue, 2022-11-15 at 11:15 +0800, Yonghua Huang wrote:

From: Yonghua Huang <yonghua.huang@...>

  this patch fix several issues that NULL pointers
  maybe be dereferenced in display module.

Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
 devicemodel/hw/pci/virtio/virtio_gpu.c | 61 ++++++++++++++++++++++--
--
 devicemodel/hw/vdisplay_sdl.c          | 13 ++++--
 devicemodel/hw/vga.c                   | 10 ++++-
 3 files changed, 71 insertions(+), 13 deletions(-)

diff --git a/devicemodel/hw/pci/virtio/virtio_gpu.c
b/devicemodel/hw/pci/virtio/virtio_gpu.c
index 8b2399b35..6e1ba248b 100644
--- a/devicemodel/hw/pci/virtio/virtio_gpu.c
+++ b/devicemodel/hw/pci/virtio/virtio_gpu.c
@@ -701,6 +701,12 @@ virtio_gpu_cmd_resource_create_2d(struct
virtio_gpu_command *cmd)
        }
        r2d = (struct virtio_gpu_resource_2d*)calloc(1, \
                        sizeof(struct virtio_gpu_resource_2d));
+       if (!r2d) {
+               pr_err("%s: memory allocation for r2d failed.\n",
__func__);
+               resp.type = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+               goto response;
+       }
+
        r2d->resource_id = req.resource_id;
        r2d->width = req.width;
        r2d->height = req.height;
@@ -773,15 +779,27 @@ virtio_gpu_cmd_resource_attach_backing(struct
virtio_gpu_command *cmd)
        struct virtio_gpu_ctrl_hdr resp;
        int i;
        uint8_t *pbuf;
+       struct iovec *iov;

        memcpy(&req, cmd->iov[0].iov_base, sizeof(req));
        memset(&resp, 0, sizeof(resp));

        r2d = virtio_gpu_find_resource_2d(cmd->gpu, req.resource_id);
        if (r2d) {
-               r2d->iov = malloc(req.nr_entries * sizeof(struct
iovec));
+               iov = malloc(req.nr_entries * sizeof(struct iovec));
+               if (!iov) {
+                       resp.type =
VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+                       goto exit;
+               }
+
+               r2d->iov = iov;
                r2d->iovcnt = req.nr_entries;
                entries = malloc(req.nr_entries * sizeof(struct
virtio_gpu_mem_entry));
+               if (!entries) {
+                       free(iov);
+                       resp.type =
VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+                       goto exit;
+               }
                pbuf = (uint8_t*)entries;
                for (i = 1; i < (cmd->iovcnt - 1); i++) {
                        memcpy(pbuf, cmd->iov[i].iov_base, cmd-

iov[i].iov_len);

@@ -800,8 +818,10 @@ virtio_gpu_cmd_resource_attach_backing(struct
virtio_gpu_command *cmd)
                resp.type = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
        }

-       cmd->iolen = sizeof(resp);
        resp.type = VIRTIO_GPU_RESP_OK_NODATA;
+
+exit:
+       cmd->iolen = sizeof(resp);
        virtio_gpu_update_resp_fence(&cmd->hdr, &resp);
        memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
 }
@@ -1165,6 +1185,7 @@ virtio_gpu_cmd_create_blob(struct
virtio_gpu_command *cmd)
        struct virtio_gpu_ctrl_hdr resp;
        int i;
        uint8_t *pbuf;
+       struct iovec *iov;

        memcpy(&req, cmd->iov[0].iov_base, sizeof(req));
        cmd->iolen = sizeof(resp);
@@ -1199,9 +1220,23 @@ virtio_gpu_cmd_create_blob(struct
virtio_gpu_command *cmd)

        r2d = (struct virtio_gpu_resource_2d *)calloc(1,
                        sizeof(struct virtio_gpu_resource_2d));
+       if (!r2d) {
+               pr_err("%s : memory allocation for r2d failed.\n",
__func__);
+               resp.type = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+               memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
+               return;
+       }
+
        r2d->resource_id = req.resource_id;

        entries = malloc(req.nr_entries * sizeof(struct
virtio_gpu_mem_entry));
+       if (!entries) {
+               pr_err("%s : memory allocation for entries
failed.\n", __func__);
+               free(r2d);
+               resp.type = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+               memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
+               return;
+       }
        pbuf = (uint8_t *)entries;
        for (i = 1; i < (cmd->iovcnt - 1); i++) {
                memcpy(pbuf, cmd->iov[i].iov_base, cmd-

iov[i].iov_len);

@@ -1229,7 +1264,16 @@ virtio_gpu_cmd_create_blob(struct
virtio_gpu_command *cmd)
                r2d->image = pixman_image_create_bits(
                                r2d->format, r2d->width, r2d->height,
NULL, 0);

-               r2d->iov = malloc(req.nr_entries * sizeof(struct
iovec));
+               iov = malloc(req.nr_entries * sizeof(struct iovec));
+               if (!iov) {
+                       free(entries);
+                       free(r2d);
+                       resp.type =
VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+                       memcpy(cmd->iov[cmd->iovcnt - 1].iov_base,
&resp, sizeof(resp));
+                       return;
+               }
+               r2d->iov = iov;
+
                r2d->iovcnt = req.nr_entries;
                for (i = 0; i < req.nr_entries; i++) {
                        r2d->iov[i].iov_base = paddr_guest2host( @@
-1551,7 +1595,7 @@ virtio_gpu_vga_render(void *param)
        gpu->vga.surf.stride = 0;
        /* The below logic needs to be refined */
        while(gpu->vga.enable) {
-               if(gpu->vga.gc->gc_image->vgamode) {
+               if((gpu->vga.gc->gc_image->vgamode) && (gpu->vga.dev
!= NULL)) {
                        vga_render(gpu->vga.gc, gpu->vga.dev);
                        break;
                }
@@ -1796,6 +1840,9 @@ virtio_gpu_deinit(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)
        int i;

        gpu = (struct virtio_gpu *)dev->arg;
+       if (!gpu)
+               return;
+
        gpu->vga.enable = false;

        pthread_mutex_lock(&gpu->vga_thread_mtx);
@@ -1855,10 +1902,8 @@ virtio_gpu_deinit(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)

        vdpy_deinit(gpu->vdpy_handle);

-       if (gpu) {
-               pthread_mutex_destroy(&gpu->mtx);
-               free(gpu);
-       }
+       pthread_mutex_destroy(&gpu->mtx);
+       free(gpu);
        virtio_gpu_device_cnt--;
 }

diff --git a/devicemodel/hw/vdisplay_sdl.c
b/devicemodel/hw/vdisplay_sdl.c index 9bb2f7360..f83b92e6a 100644
--- a/devicemodel/hw/vdisplay_sdl.c
+++ b/devicemodel/hw/vdisplay_sdl.c
@@ -1485,14 +1485,19 @@ int vdpy_parse_cmd_option(const char *opts)
        struct vscreen *vscr;

        error = 0;
-       vdpy.vscrs = calloc(VSCREEN_MAX_NUM, sizeof(struct vscreen));
+       vscr = calloc(VSCREEN_MAX_NUM, sizeof(struct vscreen));
+       if (!vscr) {
+               pr_err("%s, memory allocation for vscrs failed.",
__func__);
+               return -1;
+       }
+       vdpy.vscrs = vscr;
        vdpy.vscrs_num = 0;
        vscr = vdpy.vscrs + vdpy.vscrs_num;

        stropts = strdup(opts);
        while ((str = strsep(&stropts, ",")) != NULL) {
                tmp = strcasestr(str, "geometry=");
-               if (str && strcasestr(str, "geometry=fullscreen")) {
+               if (tmp && str && strcasestr(str,
"geometry=fullscreen")) {
                        vscr = vdpy.vscrs + vdpy.vscrs_num;
                        snum = sscanf(tmp, "geometry=fullscreen:%d",
&vscr->pscreen_id);
                        if (snum != 1) { @@ -1506,7 +1511,7 @@ int
vdpy_parse_cmd_option(const char *opts)
                        pr_info("virtual display: fullscreen on
monitor %d\n",
                                        vscr->pscreen_id);
                        vdpy.vscrs_num++;
-               } else if (str && strcasestr(str, "geometry=")) {
+               } else if (tmp && str && strcasestr(str,
"geometry=")) {
                        vscr = vdpy.vscrs + vdpy.vscrs_num;
                        snum = sscanf(tmp, "geometry=%dx%d+%d+%d",
                                        &vscr->guest_width, &vscr-

guest_height,

@@ -1524,7 +1529,7 @@ int vdpy_parse_cmd_option(const char *opts)
                }

                tmp = strcasestr(str, "rotation=");
-               if (str && strcasestr(str, "rotation=")) {
+               if (tmp && str && strcasestr(str, "rotation=")) {
                        snum = sscanf(tmp, "rotation=%d", &vscr-

rotation);

                        if (snum != 1) {
                                vscr->rotation = 0; diff --git
a/devicemodel/hw/vga.c b/devicemodel/hw/vga.c index
19cc06428..1776cfb05 100644
--- a/devicemodel/hw/vga.c
+++ b/devicemodel/hw/vga.c
@@ -1235,8 +1235,11 @@ vga_init(struct gfx_ctx *gc, int io_only)
        struct inout_port iop;
        struct vga_vdev *vd;
        int port, error;
+       uint8_t *vga_ram;

        vd = calloc(1, sizeof(struct vga_vdev));
+       if (!vd)
+               return NULL;

        bzero(&iop, sizeof(struct inout_port));
        iop.name = "VGA";
@@ -1272,7 +1275,12 @@ vga_init(struct gfx_ctx *gc, int io_only)
                return NULL;
        }

-       vd->vga_ram = malloc(256 * KB);
+       vga_ram = malloc(256 * KB);
+       if (!vga_ram) {
+               free(vd);
+               return NULL;
+       }
+       vd->vga_ram = vga_ram;
        memset(vd->vga_ram, 0, 256 * KB);

        {

--
Sun Peng <peng.p.sun@...>
SSE/ACRN Upstream