diff mbox series

[RFC,PATCH-for-7.2,1/4] hw/display/qxl: Have qxl_log_command Return early if no log_cmd handler

Message ID 20221125154030.42108-2-philmd@linaro.org
State Superseded
Headers show
Series hw/display/qxl: Avoid buffer overrun in qxl_phys2virt() | expand

Commit Message

Philippe Mathieu-Daudé Nov. 25, 2022, 3:40 p.m. UTC 
Only 3 command types are logged: no need to call qxl_phys2virt()
for the other types.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/display/qxl-logger.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Marc-André Lureau Nov. 28, 2022, 8:24 a.m. UTC | #1 
Hi

On Fri, Nov 25, 2022 at 7:41 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Only 3 command types are logged: no need to call qxl_phys2virt()
> for the other types.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/display/qxl-logger.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/hw/display/qxl-logger.c b/hw/display/qxl-logger.c
> index 68bfa47568..1bcf803db6 100644
> --- a/hw/display/qxl-logger.c
> +++ b/hw/display/qxl-logger.c
> @@ -247,6 +247,16 @@ int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
>              qxl_name(qxl_type, ext->cmd.type),
>              compat ? "(compat)" : "");
>
> +    switch (ext->cmd.type) {
> +    case QXL_CMD_DRAW:
> +        break;
> +    case QXL_CMD_SURFACE:
> +        break;
> +    case QXL_CMD_CURSOR:
> +        break;
> +    default:
> +        goto out;
> +    }

That's a quite verbose way to repeat the case list below. Furthermore,
it shouldn't hurt to call qxl_phys2virt() next.


>      data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);

But I understand better the change looking at patch 3, where you
introduce the size argument. Maybe mention the coming change in the
commit message?


Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

>      if (!data) {
>          return 1;
> @@ -269,6 +279,7 @@ int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
>          qxl_log_cmd_cursor(qxl, data, ext->group_id);
>          break;
>      }
> +out:
>      fprintf(stderr, "\n");
>      return 0;
>  }
> --
> 2.38.1
>
>


--
Marc-André Lureau
diff mbox series

Patch

diff --git a/hw/display/qxl-logger.c b/hw/display/qxl-logger.c
index 68bfa47568..1bcf803db6 100644
--- a/hw/display/qxl-logger.c
+++ b/hw/display/qxl-logger.c
@@ -247,6 +247,16 @@  int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
             qxl_name(qxl_type, ext->cmd.type),
             compat ? "(compat)" : "");
 
+    switch (ext->cmd.type) {
+    case QXL_CMD_DRAW:
+        break;
+    case QXL_CMD_SURFACE:
+        break;
+    case QXL_CMD_CURSOR:
+        break;
+    default:
+        goto out;
+    }
     data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
     if (!data) {
         return 1;
@@ -269,6 +279,7 @@  int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
         qxl_log_cmd_cursor(qxl, data, ext->group_id);
         break;
     }
+out:
     fprintf(stderr, "\n");
     return 0;
 }