[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH-for-7.2 3/4] hw/display/qxl: Pass requested buffer size t
From: |
Marc-André Lureau |
Subject: |
Re: [RFC PATCH-for-7.2 3/4] hw/display/qxl: Pass requested buffer size to qxl_phys2virt() |
Date: |
Mon, 28 Nov 2022 12:22:46 +0400 |
On Fri, Nov 25, 2022 at 9:35 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Currently qxl_phys2virt() doesn't check for buffer overrun.
> In order to do so in the next commit, pass the buffer size
> as argument.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: Please double-check qxl_render_update_area_unlocked()
> ---
> hw/display/qxl-logger.c | 11 ++++++++---
> hw/display/qxl-render.c | 11 +++++++----
> hw/display/qxl.c | 14 +++++++++-----
> hw/display/qxl.h | 4 +++-
> 4 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/hw/display/qxl-logger.c b/hw/display/qxl-logger.c
> index 1bcf803db6..35c38f6252 100644
> --- a/hw/display/qxl-logger.c
> +++ b/hw/display/qxl-logger.c
> @@ -106,7 +106,7 @@ static int qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL
> addr, int group_id)
> QXLImage *image;
> QXLImageDescriptor *desc;
>
> - image = qxl_phys2virt(qxl, addr, group_id);
> + image = qxl_phys2virt(qxl, addr, group_id, sizeof(QXLImage));
> if (!image) {
> return 1;
> }
> @@ -214,7 +214,8 @@ int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd
> *cmd, int group_id)
> cmd->u.set.position.y,
> cmd->u.set.visible ? "yes" : "no",
> cmd->u.set.shape);
> - cursor = qxl_phys2virt(qxl, cmd->u.set.shape, group_id);
> + cursor = qxl_phys2virt(qxl, cmd->u.set.shape, group_id,
> + sizeof(QXLCursor));
> if (!cursor) {
> return 1;
> }
> @@ -236,6 +237,7 @@ int qxl_log_command(PCIQXLDevice *qxl, const char *ring,
> QXLCommandExt *ext)
> {
> bool compat = ext->flags & QXL_COMMAND_FLAG_COMPAT;
> void *data;
> + size_t datasz;
> int ret;
>
> if (!qxl->cmdlog) {
> @@ -249,15 +251,18 @@ int qxl_log_command(PCIQXLDevice *qxl, const char
> *ring, QXLCommandExt *ext)
>
> switch (ext->cmd.type) {
> case QXL_CMD_DRAW:
> + datasz = compat ? sizeof(QXLCompatDrawable) : sizeof(QXLDrawable);
> break;
> case QXL_CMD_SURFACE:
> + datasz = sizeof(QXLSurfaceCmd);
> break;
> case QXL_CMD_CURSOR:
> + datasz = sizeof(QXLCursorCmd);
> break;
> default:
> goto out;
> }
> - data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
> + data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id, datasz);
> if (!data) {
> return 1;
> }
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index ca217004bf..1b0a50c1aa 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -107,7 +107,8 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice
> *qxl)
> qxl->guest_primary.resized = 0;
> qxl->guest_primary.data = qxl_phys2virt(qxl,
>
> qxl->guest_primary.surface.mem,
> - MEMSLOT_GROUP_GUEST);
> + MEMSLOT_GROUP_GUEST,
> + sizeof(uint32_t) * width *
> height);
It looks wrong, I think it should be:
qxl->guest_primary.abs_stride * height * qxl->guest_primary.bytes_pp
> if (!qxl->guest_primary.data) {
> goto end;
> }
> @@ -228,7 +229,7 @@ static void qxl_unpack_chunks(void *dest, size_t size,
> PCIQXLDevice *qxl,
> if (offset == size) {
> return;
> }
> - chunk = qxl_phys2virt(qxl, chunk->next_chunk, group_id);
> + chunk = qxl_phys2virt(qxl, chunk->next_chunk, group_id, bytes);
> if (!chunk) {
> return;
> }
> @@ -295,7 +296,8 @@ fail:
> /* called from spice server thread context only */
> int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
> {
> - QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
> + QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id,
> + sizeof(QXLCursorCmd));
> QXLCursor *cursor;
> QEMUCursor *c;
>
> @@ -314,7 +316,8 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt
> *ext)
> }
> switch (cmd->type) {
> case QXL_CURSOR_SET:
> - cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id);
> + cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id,
> + sizeof(QXLCursor));
> if (!cursor) {
> return 1;
> }
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 5b10f697f1..231d733250 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -274,7 +274,8 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice
> *qxl, int replay)
> QXL_IO_MONITORS_CONFIG_ASYNC));
> }
>
> - cfg = qxl_phys2virt(qxl, qxl->guest_monitors_config,
> MEMSLOT_GROUP_GUEST);
> + cfg = qxl_phys2virt(qxl, qxl->guest_monitors_config, MEMSLOT_GROUP_GUEST,
> + sizeof(QXLMonitorsConfig));
> if (cfg != NULL && cfg->count == 1) {
> qxl->guest_primary.resized = 1;
> qxl->guest_head0_width = cfg->heads[0].width;
> @@ -459,7 +460,8 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct
> QXLCommandExt *ext)
> switch (le32_to_cpu(ext->cmd.type)) {
> case QXL_CMD_SURFACE:
> {
> - QXLSurfaceCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data,
> ext->group_id);
> + QXLSurfaceCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id,
> + sizeof(QXLSurfaceCmd));
>
> if (!cmd) {
> return 1;
> @@ -494,7 +496,8 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct
> QXLCommandExt *ext)
> }
> case QXL_CMD_CURSOR:
> {
> - QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
> + QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id,
> + sizeof(QXLCursorCmd));
>
> if (!cmd) {
> return 1;
> @@ -1456,7 +1459,8 @@ static bool qxl_get_check_slot_offset(PCIQXLDevice
> *qxl, QXLPHYSICAL pqxl,
> }
>
> /* can be also called from spice server thread context */
> -void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
> +void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id,
> + size_t size)
> {
> uint64_t offset;
> uint32_t slot;
> @@ -1964,7 +1968,7 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl)
> }
>
> cmd = qxl_phys2virt(qxl, qxl->guest_surfaces.cmds[i],
> - MEMSLOT_GROUP_GUEST);
> + MEMSLOT_GROUP_GUEST, sizeof(QXLSurfaceCmd));
> assert(cmd);
> assert(cmd->type == QXL_SURFACE_CMD_CREATE);
> qxl_dirty_one_surface(qxl, cmd->u.surface_create.data,
> diff --git a/hw/display/qxl.h b/hw/display/qxl.h
> index 78b3a6c9ba..bf03138ab4 100644
> --- a/hw/display/qxl.h
> +++ b/hw/display/qxl.h
> @@ -153,6 +153,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PCIQXLDevice, PCI_QXL)
> * @qxl: QXL device
> * @phys: physical offset of buffer within the VRAM
> * @group_id: memory slot group
> + * @size: size of the buffer
> *
> * Returns a host pointer to a buffer placed at offset @phys within the
> * active slot @group_id of the PCI VGA RAM memory region associated with
> @@ -166,7 +167,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(PCIQXLDevice, PCI_QXL)
> * the incoming ram_addr_t.
> *
> */
> -void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
> +void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id,
> + size_t size);
> void qxl_set_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
> G_GNUC_PRINTF(2, 3);
>
> --
> 2.38.1
>
>
--
Marc-André Lureau