From 94e9f593b70e50dea8471aac01f99295785a3fbb Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Thu, 19 Jun 2025 15:41:42 +0200 Subject: [PATCH] xkb: fix buffer assembly in ProcXkbGetKbdByName() Computing how much to move the buffer pointer from the individual reply struct's length fields was wrong: that field tells how many CARD32 units the packet is *longer* than xGenericReply. All replies have at least this size (so length=0), but may be longer. Even more: some of the reply structs are same size as xGenericReply, some are longer (and sometimes we've got extra payload beyond the reply struct). (We really should have convenient macros, so we can get rid of those nasty stumbling blocks once and for all) Signed-off-by: Enrico Weigelt, metux IT consult --- xkb/xkb.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/xkb/xkb.c b/xkb/xkb.c index 303b5a48e..c40643c67 100644 --- a/xkb/xkb.c +++ b/xkb/xkb.c @@ -6065,7 +6065,9 @@ ProcXkbGetKbdByName(ClientPtr client) } memcpy(payload_walk, &mrep, sizeof(mrep)); - payload_walk = buf + (mrep.length * 4) - (sizeof(mrep) - sizeof(xGenericReply)); + + // length is in 4 byte units and only accounts what's above sizeof(xGenericReply) + payload_walk += mrep.length * sizeof(CARD32) + sizeof(xGenericReply); } if (reported & XkbGBN_CompatMapMask) { @@ -6081,7 +6083,9 @@ ProcXkbGetKbdByName(ClientPtr client) } memcpy(payload_walk, &crep, sizeof(crep)); - payload_walk = buf + (crep.length * 4) - (sizeof(crep) - sizeof(xGenericReply)); + + // length is in 4 byte units and only accounts what's above sizeof(xGenericReply) + payload_walk += crep.length * sizeof(CARD32) + sizeof(xGenericReply); } if (reported & XkbGBN_IndicatorMapMask) { @@ -6096,7 +6100,9 @@ ProcXkbGetKbdByName(ClientPtr client) } memcpy(payload_walk, &irep, sizeof(irep)); - payload_walk = buf + (irep.length * 4) - (sizeof(irep) - sizeof(xGenericReply)); + + // length is in 4 byte units and only accounts what's above sizeof(xGenericReply) + payload_walk += irep.length * sizeof(CARD32) + sizeof(xGenericReply); } if (reported & (XkbGBN_KeyNamesMask | XkbGBN_OtherNamesMask)) { @@ -6112,7 +6118,9 @@ ProcXkbGetKbdByName(ClientPtr client) } memcpy(payload_walk, &nrep, sizeof(nrep)); - payload_walk = buf + (nrep.length * 4) - (sizeof(nrep) - sizeof(xGenericReply)); + + // length is in 4 byte units and only accounts what's above sizeof(xGenericReply) + payload_walk += nrep.length * sizeof(CARD32) + sizeof(xGenericReply); } if (reported & XkbGBN_GeometryMask) { @@ -6134,7 +6142,9 @@ ProcXkbGetKbdByName(ClientPtr client) } memcpy(payload_walk, &grep, sizeof(grep)); - payload_walk = buf + (grep.length * 4) - (sizeof(grep) - sizeof(xGenericReply)); + + // length is in 4 byte units and only accounts what's above sizeof(xGenericReply) + payload_walk += grep.length * sizeof(CARD32) + sizeof(xGenericReply); } WriteToClient(client, sizeof(xkbGetKbdByNameReply), &rep);