[PATCH 0/5] Various UI/security-related fixes

[PATCH 0/5] Various UI/security-related fixes

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
Marc-André Lureau (5):
      i386/tdx: fix uninitialized variable warning in tdx_check_features
      ui/vnc: fix OOB write in vnc_refresh_lossy_rect
      ui/vnc: validate color shifts in SetPixelFormat
      ui/vnc: use RFB wire types for client message handlers
      ui/input-barrier: fix off-by-one in keycode bounds check

 target/i386/kvm/tdx.c |  2 +-
 ui/input-barrier.c    |  2 +-
 ui/vnc.c              | 56 ++++++++++++++++++++++++++++++---------------------
 3 files changed, 35 insertions(+), 25 deletions(-)
---
base-commit: 4ee536fac748b70e6f3d8568ddd20cfbaa9cf7bf
change-id: 20260706-fix-cca462e358c9

Best regards,
--  
Marc-André Lureau <marcandre.lureau@redhat.com>

[PATCH 1/5] i386/tdx: fix uninitialized variable warning in tdx_check_features

[Marc-André Lureau]

tdx_fetch_cpuid() only sets the output ret parameter on the error
path. GCC cannot prove that r is always initialized before use in
the caller, triggering -Werror=maybe-uninitialized.

Initialize r to -1 to silence the warning.

Fixes: 228e40f33048 ("i386/tdx: Fetch and validate CPUID of TD guest")
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
 target/i386/kvm/tdx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index c9c3b05d5fe..8294dbde3aa 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -887,7 +887,7 @@ static int tdx_check_features(X86ConfidentialGuest *cg, CPUState *cs)
     FeatureWordInfo *wi;
     FeatureWord w;
     bool mismatch = false;
-    int r;
+    int r = -1;
 
     fetch_cpuid = tdx_fetch_cpuid(cs, &r);
     if (!fetch_cpuid) {

-- 
2.55.0

[PATCH 2/5] ui/vnc: fix OOB write in vnc_refresh_lossy_rect

[Marc-André Lureau]

vnc_refresh_lossy_rect() always marks a full VNC_STAT_RECT (64) rows
as dirty when refreshing a lossy tile. When the display height is not
a multiple of VNC_STAT_RECT (e.g. VNC_MAX_HEIGHT = 2160), the bottom
tile is partial -- the last tile at y=2112 has only 48 valid rows.
The unclamped loop writes to vs->dirty[2160..2175], past the end of
the VNC_MAX_HEIGHT-sized array.

Clamp the row count to the actual surface height so partial bottom
tiles only mark valid dirty bitmap entries.

Fixes: CVE-2026-48002
Fixes: 7d964c9d2fc6 ("vnc: refresh lossy rect after a given timeout")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3950
Reported-by: huntr bubble
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
 ui/vnc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index b2b69923b75..559d3954b87 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3004,10 +3004,18 @@ static int vnc_refresh_lossy_rect(VncDisplay *vd, int x, int y)
     int sty = y / VNC_STAT_RECT;
     int stx = x / VNC_STAT_RECT;
     int has_dirty = 0;
+    int height = MIN(pixman_image_get_height(vd->guest.fb),
+                     pixman_image_get_height(vd->server));
+    int rows;
 
     y = QEMU_ALIGN_DOWN(y, VNC_STAT_RECT);
     x = QEMU_ALIGN_DOWN(x, VNC_STAT_RECT);
 
+    rows = MIN(VNC_STAT_RECT, height - y);
+    if (rows <= 0) {
+        return 0;
+    }
+
     QTAILQ_FOREACH(vs, &vd->clients, next) {
         VncConnection *vc = container_of(vs, VncConnection, vs);
         int j;
@@ -3022,7 +3030,7 @@ static int vnc_refresh_lossy_rect(VncDisplay *vd, int x, int y)
         }
 
         vc->worker.lossy_rect[sty][stx] = 0;
-        for (j = 0; j < VNC_STAT_RECT; ++j) {
+        for (j = 0; j < rows; ++j) {
             bitmap_set(vs->dirty[y + j],
                        x / VNC_DIRTY_PIXELS_PER_BIT,
                        VNC_STAT_RECT / VNC_DIRTY_PIXELS_PER_BIT);

-- 
2.55.0

[PATCH 3/5] ui/vnc: validate color shifts in SetPixelFormat

[Marc-André Lureau]

A malicious VNC client can send a SetPixelFormat message with shift
values >= 32, causing UB mask computation
(e.g. red_max << red_shift where red_shift is 255). Apparently, this is
not covered by -fwrapv.

Reject color shifts >= bits_per_pixel before computing masks, and cast
the max values to uint32_t to avoid signed integer overflow when a
valid shift (e.g. 24) would set bit 31 of a signed int.

Fixes: 9f64916da20 ("pixman/vnc: use pixman images in vnc.")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3948
Reported-by: huntr bubble
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
 ui/vnc.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 559d3954b87..94a38242c56 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2276,18 +2276,25 @@ static void set_pixel_format(VncState *vs, int bits_per_pixel,
         return;
     }
 
+    if (red_shift >= bits_per_pixel ||
+        green_shift >= bits_per_pixel ||
+        blue_shift >= bits_per_pixel) {
+        vnc_client_error(vs);
+        return;
+    }
+
     vs->client_pf.rmax = red_max ? red_max : 0xFF;
     vs->client_pf.rbits = ctpopl(red_max);
     vs->client_pf.rshift = red_shift;
-    vs->client_pf.rmask = red_max << red_shift;
+    vs->client_pf.rmask = (uint32_t)red_max << red_shift;
     vs->client_pf.gmax = green_max ? green_max : 0xFF;
     vs->client_pf.gbits = ctpopl(green_max);
     vs->client_pf.gshift = green_shift;
-    vs->client_pf.gmask = green_max << green_shift;
+    vs->client_pf.gmask = (uint32_t)green_max << green_shift;
     vs->client_pf.bmax = blue_max ? blue_max : 0xFF;
     vs->client_pf.bbits = ctpopl(blue_max);
     vs->client_pf.bshift = blue_shift;
-    vs->client_pf.bmask = blue_max << blue_shift;
+    vs->client_pf.bmask = (uint32_t)blue_max << blue_shift;
     vs->client_pf.bits_per_pixel = bits_per_pixel;
     vs->client_pf.bytes_per_pixel = bits_per_pixel / 8;
     vs->client_pf.depth = bits_per_pixel == 32 ? 24 : bits_per_pixel;

-- 
2.55.0

[PATCH 4/5] ui/vnc: use RFB wire types for client message handlers

[Marc-André Lureau]

Use exact-width unsigned types for the static functions that process
RFB client messages, matching the types returned by read_u8(),
read_u16(), and read_u32():

 - set_pixel_format: uint8_t/uint16_t for pixel format fields
 - pointer_event: uint8_t button_mask, uint16_t x/y
 - key_event/ext_key_event: bool down, uint32_t sym/keycode
 - do_key_event: uint32_t sym
 - framebuffer_update_request: uint8_t incremental, uint16_t x/y/w/h

Drop needless declarations.

Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
 ui/vnc.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 94a38242c56..14ce2816907 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -608,15 +608,7 @@ bool vnc_display_reload_certs(const char *id, Error **errp)
    3) resolutions > 1024
 */
 
-static int vnc_update_client(VncState *vs, int has_dirty);
-static void vnc_disconnect_start(VncState *vs);
-
 static void vnc_colordepth(VncState *vs);
-static void framebuffer_update_request(VncState *vs, int incremental,
-                                       int x_position, int y_position,
-                                       int w, int h);
-static void vnc_refresh(DisplayChangeListener *dcl);
-static int vnc_refresh_server_surface(VncDisplay *vd);
 
 static int vnc_width(VncDisplay *vd)
 {
@@ -1763,7 +1755,8 @@ static void check_pointer_type_change(Notifier *notifier, void *data)
     vs->absolute = absolute;
 }
 
-static void pointer_event(VncState *vs, int button_mask, int x, int y)
+static void pointer_event(VncState *vs, uint8_t button_mask,
+                          uint16_t x, uint16_t y)
 {
     static uint32_t bmap[INPUT_BUTTON__MAX] = {
         [INPUT_BUTTON_LEFT]       = 0x01,
@@ -1841,7 +1834,7 @@ static void kbd_leds(Notifier *notifier, void *data)
     }
 }
 
-static void do_key_event(VncState *vs, int down, int keycode, int sym)
+static void do_key_event(VncState *vs, int down, int keycode, uint32_t sym)
 {
     unsigned int lnx = qemu_input_key_number_to_linux(keycode);
 
@@ -2019,7 +2012,7 @@ static const char *code2name(int keycode)
     return QKeyCode_str(qemu_input_key_number_to_qcode(keycode));
 }
 
-static void key_event(VncState *vs, int down, uint32_t sym)
+static void key_event(VncState *vs, bool down, uint32_t sym)
 {
     int keycode;
     int lsym = sym;
@@ -2034,8 +2027,8 @@ static void key_event(VncState *vs, int down, uint32_t sym)
     do_key_event(vs, down, keycode, sym);
 }
 
-static void ext_key_event(VncState *vs, int down,
-                          uint32_t sym, uint16_t keycode)
+static void ext_key_event(VncState *vs, bool down,
+                          uint32_t sym, uint32_t keycode)
 {
     /* if the user specifies a keyboard layout, always use it */
     if (keyboard_layout) {
@@ -2046,8 +2039,9 @@ static void ext_key_event(VncState *vs, int down,
     }
 }
 
-static void framebuffer_update_request(VncState *vs, int incremental,
-                                       int x, int y, int w, int h)
+static void framebuffer_update_request(VncState *vs, uint8_t incremental,
+                                       uint16_t x, uint16_t y,
+                                       uint16_t w, uint16_t h)
 {
     if (incremental) {
         if (vs->update != VNC_STATE_UPDATE_FORCE) {
@@ -2250,10 +2244,11 @@ static void send_color_map(VncState *vs)
     vnc_unlock_output(vs);
 }
 
-static void set_pixel_format(VncState *vs, int bits_per_pixel,
-                             int big_endian_flag, int true_color_flag,
-                             int red_max, int green_max, int blue_max,
-                             int red_shift, int green_shift, int blue_shift)
+static void set_pixel_format(VncState *vs, uint8_t bits_per_pixel,
+                             uint8_t big_endian_flag, uint8_t true_color_flag,
+                             uint16_t red_max, uint16_t green_max,
+                             uint16_t blue_max, uint8_t red_shift,
+                             uint8_t green_shift, uint8_t blue_shift)
 {
     if (!true_color_flag) {
         /* Expose a reasonable default 256 color map */

-- 
2.55.0

[PATCH 5/5] ui/input-barrier: fix off-by-one in keycode bounds check

[Marc-André Lureau]

Use strict "<" to fix the off-by-one.

Fixes: 6105683da35b ("ui: add an embedded Barrier client")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3951
Reported-by: huntr bubble
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
 ui/input-barrier.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/input-barrier.c b/ui/input-barrier.c
index d07027114af..0a238d4010b 100644
--- a/ui/input-barrier.c
+++ b/ui/input-barrier.c
@@ -87,7 +87,7 @@ static kbd_layout_t *kbd_layout;
 static unsigned int input_barrier_to_linux(uint16_t keyid, uint16_t keycode)
 {
     /* keycode is optional, if it is not provided use keyid */
-    if (keycode && keycode <= qemu_input_map_xorgkbd_to_linux_len) {
+    if (keycode && keycode < qemu_input_map_xorgkbd_to_linux_len) {
         return qemu_input_map_xorgkbd_to_linux[keycode];
     }
 

-- 
2.55.0

Re: [PATCH 5/5] ui/input-barrier: fix off-by-one in keycode bounds check

[Laurent Vivier]

On 7/6/26 14:53, Marc-André Lureau wrote:
> Use strict "<" to fix the off-by-one.
> 
> Fixes: 6105683da35b ("ui: add an embedded Barrier client")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3951
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/input-barrier.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ui/input-barrier.c b/ui/input-barrier.c
> index d07027114af..0a238d4010b 100644
> --- a/ui/input-barrier.c
> +++ b/ui/input-barrier.c
> @@ -87,7 +87,7 @@ static kbd_layout_t *kbd_layout;
>   static unsigned int input_barrier_to_linux(uint16_t keyid, uint16_t keycode)
>   {
>       /* keycode is optional, if it is not provided use keyid */
> -    if (keycode && keycode <= qemu_input_map_xorgkbd_to_linux_len) {
> +    if (keycode && keycode < qemu_input_map_xorgkbd_to_linux_len) {
>           return qemu_input_map_xorgkbd_to_linux[keycode];
>       }
>   
> 

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Re: [PATCH 1/5] i386/tdx: fix uninitialized variable warning in tdx_check_features

[Philippe Mathieu-Daudé]

On 6/7/26 14:53, Marc-André Lureau wrote:
> tdx_fetch_cpuid() only sets the output ret parameter on the error
> path. GCC cannot prove that r is always initialized before use in
> the caller, triggering -Werror=maybe-uninitialized.
> 
> Initialize r to -1 to silence the warning.
> 
> Fixes: 228e40f33048 ("i386/tdx: Fetch and validate CPUID of TD guest")
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   target/i386/kvm/tdx.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index c9c3b05d5fe..8294dbde3aa 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -887,7 +887,7 @@ static int tdx_check_features(X86ConfidentialGuest *cg, CPUState *cs)
>       FeatureWordInfo *wi;
>       FeatureWord w;
>       bool mismatch = false;
> -    int r;
> +    int r = -1;
>   
>       fetch_cpuid = tdx_fetch_cpuid(cs, &r);
>       if (!fetch_cpuid) {

Alternatively, 'return -1;' here.

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 5/5] ui/input-barrier: fix off-by-one in keycode bounds check

[Philippe Mathieu-Daudé]

On 6/7/26 14:53, Marc-André Lureau wrote:
> Use strict "<" to fix the off-by-one.
> 
> Fixes: 6105683da35b ("ui: add an embedded Barrier client")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3951
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/input-barrier.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 3/5] ui/vnc: validate color shifts in SetPixelFormat

[Philippe Mathieu-Daudé]

On 6/7/26 14:53, Marc-André Lureau wrote:
> A malicious VNC client can send a SetPixelFormat message with shift
> values >= 32, causing UB mask computation
> (e.g. red_max << red_shift where red_shift is 255). Apparently, this is
> not covered by -fwrapv.
> 
> Reject color shifts >= bits_per_pixel before computing masks, and cast
> the max values to uint32_t to avoid signed integer overflow when a
> valid shift (e.g. 24) would set bit 31 of a signed int.
> 
> Fixes: 9f64916da20 ("pixman/vnc: use pixman images in vnc.")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3948
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/vnc.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 559d3954b87..94a38242c56 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2276,18 +2276,25 @@ static void set_pixel_format(VncState *vs, int bits_per_pixel,
>           return;
>       }
>   
> +    if (red_shift >= bits_per_pixel ||
> +        green_shift >= bits_per_pixel ||
> +        blue_shift >= bits_per_pixel) {
> +        vnc_client_error(vs);
> +        return;
> +    }
> +
>       vs->client_pf.rmax = red_max ? red_max : 0xFF;
>       vs->client_pf.rbits = ctpopl(red_max);
>       vs->client_pf.rshift = red_shift;
> -    vs->client_pf.rmask = red_max << red_shift;
> +    vs->client_pf.rmask = (uint32_t)red_max << red_shift;
>       vs->client_pf.gmax = green_max ? green_max : 0xFF;
>       vs->client_pf.gbits = ctpopl(green_max);
>       vs->client_pf.gshift = green_shift;
> -    vs->client_pf.gmask = green_max << green_shift;
> +    vs->client_pf.gmask = (uint32_t)green_max << green_shift;
>       vs->client_pf.bmax = blue_max ? blue_max : 0xFF;
>       vs->client_pf.bbits = ctpopl(blue_max);
>       vs->client_pf.bshift = blue_shift;
> -    vs->client_pf.bmask = blue_max << blue_shift;
> +    vs->client_pf.bmask = (uint32_t)blue_max << blue_shift;
>       vs->client_pf.bits_per_pixel = bits_per_pixel;
>       vs->client_pf.bytes_per_pixel = bits_per_pixel / 8;
>       vs->client_pf.depth = bits_per_pixel == 32 ? 24 : bits_per_pixel;
> 

What about instead of the casts (keeping the if(shift) block):

-static void set_pixel_format(VncState *vs, int bits_per_pixel,
-                             int big_endian_flag, int true_color_flag,
-                             int red_max, int green_max, int blue_max,
-                             int red_shift, int green_shift, int 
blue_shift)
+static void set_pixel_format(VncState *vs, uint8_t bits_per_pixel,
+                             bool big_endian_flag, bool true_color_flag,
+                             uint16_t red_max, uint16_t green_max, 
uint16_t blue_max,
+                             uint8_t red_shift, uint8_t green_shift, 
uint8_t blue_shift)

?

Re: [PATCH 2/5] ui/vnc: fix OOB write in vnc_refresh_lossy_rect

[Philippe Mathieu-Daudé]

On 6/7/26 14:53, Marc-André Lureau wrote:
> vnc_refresh_lossy_rect() always marks a full VNC_STAT_RECT (64) rows
> as dirty when refreshing a lossy tile. When the display height is not
> a multiple of VNC_STAT_RECT (e.g. VNC_MAX_HEIGHT = 2160), the bottom
> tile is partial -- the last tile at y=2112 has only 48 valid rows.
> The unclamped loop writes to vs->dirty[2160..2175], past the end of
> the VNC_MAX_HEIGHT-sized array.
> 
> Clamp the row count to the actual surface height so partial bottom
> tiles only mark valid dirty bitmap entries.
> 
> Fixes: CVE-2026-48002
> Fixes: 7d964c9d2fc6 ("vnc: refresh lossy rect after a given timeout")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3950
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/vnc.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index b2b69923b75..559d3954b87 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3004,10 +3004,18 @@ static int vnc_refresh_lossy_rect(VncDisplay *vd, int x, int y)
>       int sty = y / VNC_STAT_RECT;
>       int stx = x / VNC_STAT_RECT;
>       int has_dirty = 0;
> +    int height = MIN(pixman_image_get_height(vd->guest.fb),
> +                     pixman_image_get_height(vd->server));

This could deserve some helper, anyway:
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

> +    int rows;
>   
>       y = QEMU_ALIGN_DOWN(y, VNC_STAT_RECT);
>       x = QEMU_ALIGN_DOWN(x, VNC_STAT_RECT);
>   
> +    rows = MIN(VNC_STAT_RECT, height - y);
> +    if (rows <= 0) {
> +        return 0;
> +    }

Re: [PATCH 3/5] ui/vnc: validate color shifts in SetPixelFormat

[Philippe Mathieu-Daudé]

On 13/7/26 13:23, Philippe Mathieu-Daudé wrote:
> On 6/7/26 14:53, Marc-André Lureau wrote:
>> A malicious VNC client can send a SetPixelFormat message with shift
>> values >= 32, causing UB mask computation
>> (e.g. red_max << red_shift where red_shift is 255). Apparently, this is
>> not covered by -fwrapv.
>>
>> Reject color shifts >= bits_per_pixel before computing masks, and cast
>> the max values to uint32_t to avoid signed integer overflow when a
>> valid shift (e.g. 24) would set bit 31 of a signed int.
>>
>> Fixes: 9f64916da20 ("pixman/vnc: use pixman images in vnc.")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3948
>> Reported-by: huntr bubble
>> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
>> ---
>>   ui/vnc.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/ui/vnc.c b/ui/vnc.c
>> index 559d3954b87..94a38242c56 100644
>> --- a/ui/vnc.c
>> +++ b/ui/vnc.c
>> @@ -2276,18 +2276,25 @@ static void set_pixel_format(VncState *vs, int 
>> bits_per_pixel,
>>           return;
>>       }
>> +    if (red_shift >= bits_per_pixel ||
>> +        green_shift >= bits_per_pixel ||
>> +        blue_shift >= bits_per_pixel) {
>> +        vnc_client_error(vs);
>> +        return;
>> +    }
>> +
>>       vs->client_pf.rmax = red_max ? red_max : 0xFF;
>>       vs->client_pf.rbits = ctpopl(red_max);
>>       vs->client_pf.rshift = red_shift;
>> -    vs->client_pf.rmask = red_max << red_shift;
>> +    vs->client_pf.rmask = (uint32_t)red_max << red_shift;
>>       vs->client_pf.gmax = green_max ? green_max : 0xFF;
>>       vs->client_pf.gbits = ctpopl(green_max);
>>       vs->client_pf.gshift = green_shift;
>> -    vs->client_pf.gmask = green_max << green_shift;
>> +    vs->client_pf.gmask = (uint32_t)green_max << green_shift;
>>       vs->client_pf.bmax = blue_max ? blue_max : 0xFF;
>>       vs->client_pf.bbits = ctpopl(blue_max);
>>       vs->client_pf.bshift = blue_shift;
>> -    vs->client_pf.bmask = blue_max << blue_shift;
>> +    vs->client_pf.bmask = (uint32_t)blue_max << blue_shift;
>>       vs->client_pf.bits_per_pixel = bits_per_pixel;
>>       vs->client_pf.bytes_per_pixel = bits_per_pixel / 8;
>>       vs->client_pf.depth = bits_per_pixel == 32 ? 24 : bits_per_pixel;
>>
> 
> What about instead of the casts (keeping the if(shift) block):
> 
> -static void set_pixel_format(VncState *vs, int bits_per_pixel,
> -                             int big_endian_flag, int true_color_flag,
> -                             int red_max, int green_max, int blue_max,
> -                             int red_shift, int green_shift, int 
> blue_shift)
> +static void set_pixel_format(VncState *vs, uint8_t bits_per_pixel,
> +                             bool big_endian_flag, bool true_color_flag,
> +                             uint16_t red_max, uint16_t green_max, 
> uint16_t blue_max,
> +                             uint8_t red_shift, uint8_t green_shift, 
> uint8_t blue_shift)
> 
> ?

Haha this is what the next patch does ;) I got confused by the
(uint32_t) casts, they belong to the next patch.

So with only the if(shift) here:
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 4/5] ui/vnc: use RFB wire types for client message handlers

[Philippe Mathieu-Daudé]

On 6/7/26 14:53, Marc-André Lureau wrote:
> Use exact-width unsigned types for the static functions that process
> RFB client messages, matching the types returned by read_u8(),
> read_u16(), and read_u32():
> 
>   - set_pixel_format: uint8_t/uint16_t for pixel format fields
>   - pointer_event: uint8_t button_mask, uint16_t x/y
>   - key_event/ext_key_event: bool down, uint32_t sym/keycode
>   - do_key_event: uint32_t sym
>   - framebuffer_update_request: uint8_t incremental, uint16_t x/y/w/h
> 
> Drop needless declarations.
> 
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/vnc.c | 33 ++++++++++++++-------------------
>   1 file changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 94a38242c56..14ce2816907 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -608,15 +608,7 @@ bool vnc_display_reload_certs(const char *id, Error **errp)
>      3) resolutions > 1024
>   */
>   
> -static int vnc_update_client(VncState *vs, int has_dirty);
> -static void vnc_disconnect_start(VncState *vs);
> -
>   static void vnc_colordepth(VncState *vs);
> -static void framebuffer_update_request(VncState *vs, int incremental,
> -                                       int x_position, int y_position,
> -                                       int w, int h);
> -static void vnc_refresh(DisplayChangeListener *dcl);
> -static int vnc_refresh_server_surface(VncDisplay *vd);
>   
>   static int vnc_width(VncDisplay *vd)
>   {
> @@ -1763,7 +1755,8 @@ static void check_pointer_type_change(Notifier *notifier, void *data)
>       vs->absolute = absolute;
>   }
>   
> -static void pointer_event(VncState *vs, int button_mask, int x, int y)
> +static void pointer_event(VncState *vs, uint8_t button_mask,
> +                          uint16_t x, uint16_t y)
>   {
>       static uint32_t bmap[INPUT_BUTTON__MAX] = {
>           [INPUT_BUTTON_LEFT]       = 0x01,
> @@ -1841,7 +1834,7 @@ static void kbd_leds(Notifier *notifier, void *data)
>       }
>   }
>   
> -static void do_key_event(VncState *vs, int down, int keycode, int sym)
> +static void do_key_event(VncState *vs, int down, int keycode, uint32_t sym)

bool down.

>   {
>       unsigned int lnx = qemu_input_key_number_to_linux(keycode);
>   
> @@ -2019,7 +2012,7 @@ static const char *code2name(int keycode)
>       return QKeyCode_str(qemu_input_key_number_to_qcode(keycode));
>   }
>   
> -static void key_event(VncState *vs, int down, uint32_t sym)
> +static void key_event(VncState *vs, bool down, uint32_t sym)
>   {
>       int keycode;
>       int lsym = sym;
> @@ -2034,8 +2027,8 @@ static void key_event(VncState *vs, int down, uint32_t sym)
>       do_key_event(vs, down, keycode, sym);
>   }
>   
> -static void ext_key_event(VncState *vs, int down,
> -                          uint32_t sym, uint16_t keycode)
> +static void ext_key_event(VncState *vs, bool down,
> +                          uint32_t sym, uint32_t keycode)
>   {
>       /* if the user specifies a keyboard layout, always use it */
>       if (keyboard_layout) {
> @@ -2046,8 +2039,9 @@ static void ext_key_event(VncState *vs, int down,
>       }
>   }
>   
> -static void framebuffer_update_request(VncState *vs, int incremental,
> -                                       int x, int y, int w, int h)
> +static void framebuffer_update_request(VncState *vs, uint8_t incremental,
> +                                       uint16_t x, uint16_t y,
> +                                       uint16_t w, uint16_t h)
>   {
>       if (incremental) {
>           if (vs->update != VNC_STATE_UPDATE_FORCE) {
> @@ -2250,10 +2244,11 @@ static void send_color_map(VncState *vs)
>       vnc_unlock_output(vs);
>   }
>   
> -static void set_pixel_format(VncState *vs, int bits_per_pixel,
> -                             int big_endian_flag, int true_color_flag,
> -                             int red_max, int green_max, int blue_max,
> -                             int red_shift, int green_shift, int blue_shift)
> +static void set_pixel_format(VncState *vs, uint8_t bits_per_pixel,
> +                             uint8_t big_endian_flag, uint8_t true_color_flag,

bool flag, bool flag.

> +                             uint16_t red_max, uint16_t green_max,
> +                             uint16_t blue_max, uint8_t red_shift,
> +                             uint8_t green_shift, uint8_t blue_shift)
>   {
>       if (!true_color_flag) {
>           /* Expose a reasonable default 256 color map */
> 

Including the (uint32_t) casts in this patch:
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 2/5] ui/vnc: fix OOB write in vnc_refresh_lossy_rect

[Michael Tokarev]

On 7/6/26 15:53, Marc-André Lureau wrote:
> vnc_refresh_lossy_rect() always marks a full VNC_STAT_RECT (64) rows
> as dirty when refreshing a lossy tile. When the display height is not
> a multiple of VNC_STAT_RECT (e.g. VNC_MAX_HEIGHT = 2160), the bottom
> tile is partial -- the last tile at y=2112 has only 48 valid rows.
> The unclamped loop writes to vs->dirty[2160..2175], past the end of
> the VNC_MAX_HEIGHT-sized array.
> 
> Clamp the row count to the actual surface height so partial bottom
> tiles only mark valid dirty bitmap entries.
> 
> Fixes: CVE-2026-48002
> Fixes: 7d964c9d2fc6 ("vnc: refresh lossy rect after a given timeout")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3950
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>

This patch has already been merged to the qemu master branch
(as commit 3543c2b855c), but it seems it's important to mention
still.

There already was a fix for CVE-2026-48002 earlier:

commit 46ee49034d26d04d95ba8f3183d4fbfa9d2b89b4
Author: Daniel P. Berrangé <berrange@redhat.com>
Date:   Thu May 21 11:33:52 2026 +0100

     ui/vnc: fix OOB write in lossy rect worker code

which touched the same code.  Now this one, with a
very similar subject.

Unfortunately this (additional) fix does not mention the
previous fix, so there's some confusion about which commit
actually fixed this issue.  It looks like the first fix was
incomplete and this additional fix were needed.

Thanks,

/mjt

Re: [PATCH 2/5] ui/vnc: fix OOB write in vnc_refresh_lossy_rect

[Marc-André Lureau]

Hi

On Sun, Jul 26, 2026 at 8:54 PM Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> On 7/6/26 15:53, Marc-André Lureau wrote:
> > vnc_refresh_lossy_rect() always marks a full VNC_STAT_RECT (64) rows
> > as dirty when refreshing a lossy tile. When the display height is not
> > a multiple of VNC_STAT_RECT (e.g. VNC_MAX_HEIGHT = 2160), the bottom
> > tile is partial -- the last tile at y=2112 has only 48 valid rows.
> > The unclamped loop writes to vs->dirty[2160..2175], past the end of
> > the VNC_MAX_HEIGHT-sized array.
> >
> > Clamp the row count to the actual surface height so partial bottom
> > tiles only mark valid dirty bitmap entries.
> >
> > Fixes: CVE-2026-48002
> > Fixes: 7d964c9d2fc6 ("vnc: refresh lossy rect after a given timeout")
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3950
> > Reported-by: huntr bubble
> > Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
>
> This patch has already been merged to the qemu master branch
> (as commit 3543c2b855c), but it seems it's important to mention
> still.
>
> There already was a fix for CVE-2026-48002 earlier:
>
> commit 46ee49034d26d04d95ba8f3183d4fbfa9d2b89b4
> Author: Daniel P. Berrangé <berrange@redhat.com>
> Date:   Thu May 21 11:33:52 2026 +0100
>
>      ui/vnc: fix OOB write in lossy rect worker code
>
> which touched the same code.  Now this one, with a
> very similar subject.
>
> Unfortunately this (additional) fix does not mention the
> previous fix, so there's some confusion about which commit
> actually fixed this issue.  It looks like the first fix was
> incomplete and this additional fix were needed.

That's correct, we reused the same CVE

Daniel P. Berrangé said on the issue:
We didn't release master yet. As long as we also didn't release any
stable branches that claimed to fix it, we could re-use it IMHO.

Re: [PATCH 2/5] ui/vnc: fix OOB write in vnc_refresh_lossy_rect

[Michael Tokarev]

On 7/26/26 20:07, Marc-André Lureau wrote:
...>> Unfortunately this (additional) fix does not mention the
>> previous fix, so there's some confusion about which commit
>> actually fixed this issue.  It looks like the first fix was
>> incomplete and this additional fix were needed.
> 
> That's correct, we reused the same CVE
> 
> Daniel P. Berrangé said on the issue:
> We didn't release master yet. As long as we also didn't release any
> stable branches that claimed to fix it, we could re-use it IMHO.

Heh.  This is exactly what actually happened - there were
2 stable releases on Jun-25 (v10.0.11 and v11.0.2) which
contained the first half.  Now there are 2 more stable
releases from Jul-24 which contains the second half.

Unfortunately I haven't noticed this duplicated CVE#,
or else I'd add a note to the second half in the stable
pick-ups.

I know you don't like me rushing things to the stable
series, and this is an example why this might be bad.
Fortunately this is not a regression, at least.  And
usually security fixes come in faster anyway.  Ohwell..
I'm sorry to disappoint you further.

Anyway, thank you for the clarification - everything's
clear now, the confusion is no-more.

Thank you!

/mjt

Re: [PATCH 2/5] ui/vnc: fix OOB write in vnc_refresh_lossy_rect

[Marc-André Lureau]

Hi

On Sun, Jul 26, 2026 at 9:14 PM Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> On 7/26/26 20:07, Marc-André Lureau wrote:
> ...>> Unfortunately this (additional) fix does not mention the
> >> previous fix, so there's some confusion about which commit
> >> actually fixed this issue.  It looks like the first fix was
> >> incomplete and this additional fix were needed.
> >
> > That's correct, we reused the same CVE
> >
> > Daniel P. Berrangé said on the issue:
> > We didn't release master yet. As long as we also didn't release any
> > stable branches that claimed to fix it, we could re-use it IMHO.
>
> Heh.  This is exactly what actually happened - there were
> 2 stable releases on Jun-25 (v10.0.11 and v11.0.2) which
> contained the first half.  Now there are 2 more stable
> releases from Jul-24 which contains the second half.
>
> Unfortunately I haven't noticed this duplicated CVE#,
> or else I'd add a note to the second half in the stable
> pick-ups.
>
> I know you don't like me rushing things to the stable
> series, and this is an example why this might be bad.
> Fortunately this is not a regression, at least.  And
> usually security fixes come in faster anyway.  Ohwell..
> I'm sorry to disappoint you further.

for the record, I do appreciate your work, and I make mistake too :)
Next time I'll check if a release was made already with the CVE#

Re: [PATCH 2/5] ui/vnc: fix OOB write in vnc_refresh_lossy_rect

[Daniel P. Berrangé]

On Sun, Jul 26, 2026 at 09:53:05PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Sun, Jul 26, 2026 at 9:14 PM Michael Tokarev <mjt@tls.msk.ru> wrote:
> >
> > On 7/26/26 20:07, Marc-André Lureau wrote:
> > ...>> Unfortunately this (additional) fix does not mention the
> > >> previous fix, so there's some confusion about which commit
> > >> actually fixed this issue.  It looks like the first fix was
> > >> incomplete and this additional fix were needed.
> > >
> > > That's correct, we reused the same CVE
> > >
> > > Daniel P. Berrangé said on the issue:
> > > We didn't release master yet. As long as we also didn't release any
> > > stable branches that claimed to fix it, we could re-use it IMHO.
> >
> > Heh.  This is exactly what actually happened - there were
> > 2 stable releases on Jun-25 (v10.0.11 and v11.0.2) which
> > contained the first half.  Now there are 2 more stable
> > releases from Jul-24 which contains the second half.
> >
> > Unfortunately I haven't noticed this duplicated CVE#,
> > or else I'd add a note to the second half in the stable
> > pick-ups.
> >
> > I know you don't like me rushing things to the stable
> > series, and this is an example why this might be bad.
> > Fortunately this is not a regression, at least.  And
> > usually security fixes come in faster anyway.  Ohwell..
> > I'm sorry to disappoint you further.
> 
> for the record, I do appreciate your work, and I make mistake too :)
> Next time I'll check if a release was made already with the CVE#

Or we just declare a strict "never reuse a CVE once a patch is
merged to master" rule.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|