[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>