xserver: Branch 'master'
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Fri Jan 17 08:23:16 UTC 2025
xkb/XKBMAlloc.c | 74 +++++---------------------------------------------------
xkb/xkbUtils.c | 51 ++++++++++++++++++++++++++++----------
2 files changed, 44 insertions(+), 81 deletions(-)
New commits:
commit 92bcebfd7e248f695503c0a6e7bee80be4c96834
Author: Olivier Fourdan <ofourdan at redhat.com>
Date: Fri Jan 10 15:02:54 2025 +0100
xkb: Always use MAP_LENGTH keymap size
Generating the modifier modmap, the helper function generate_modkeymap()
would check the entire range up to the MAP_LENGTH.
However, the given keymap might have less keycodes than MAP_LENGTH, in
which case we would go beyond the size of the modmap, as reported by
ASAN:
==ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1 at 0x5110001c225b thread T0
#0 0x5e7369393873 in generate_modkeymap ../dix/inpututils.c:309
#1 0x5e736930dcce in ProcGetModifierMapping ../dix/devices.c:1794
#2 0x5e7369336489 in Dispatch ../dix/dispatch.c:550
#3 0x5e736934407d in dix_main ../dix/main.c:275
#5 0x7e46d47b2ecb in __libc_start_main
#6 0x5e73691be324 in _start (xserver/build/hw/xwayland/Xwayland)
Address is located 0 bytes after 219-byte region
allocated by thread T0 here:
#0 0x7e46d4cfc542 in realloc
#1 0x5e73695aa90e in _XkbCopyClientMap ../xkb/xkbUtils.c:1142
#2 0x5e73695aa90e in XkbCopyKeymap ../xkb/xkbUtils.c:1966
#3 0x5e73695b1b2f in XkbDeviceApplyKeymap ../xkb/xkbUtils.c:2023
#4 0x5e73691c6c18 in keyboard_handle_keymap ../hw/xwayland/xwayland-input.c:1194
As MAP_LENGTH is used in various code paths where the max keycode might
not be easily available, best is to always use MAP_LENGTH to allocate the
keymaps so that the code never run past the buffer size.
If the max key code is smaller than the MAP_LENGTH limit, fill-in the gap
with zeros.
That also simplifies the code slightly as we do not constantly need to
reallocate the keymap to adjust to the max key code size.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1780
Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1762>
diff --git a/xkb/XKBMAlloc.c b/xkb/XKBMAlloc.c
index 46b61f53c..c442eb566 100644
--- a/xkb/XKBMAlloc.c
+++ b/xkb/XKBMAlloc.c
@@ -39,7 +39,6 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
Status
XkbAllocClientMap(XkbDescPtr xkb, unsigned which, unsigned nTotalTypes)
{
- register int i;
XkbClientMapPtr map;
if ((xkb == NULL) ||
@@ -101,8 +100,7 @@ XkbAllocClientMap(XkbDescPtr xkb, unsigned which, unsigned nTotalTypes)
map->syms[0] = NoSymbol;
}
if (map->key_sym_map == NULL) {
- i = xkb->max_key_code + 1;
- map->key_sym_map = calloc(i, sizeof(XkbSymMapRec));
+ map->key_sym_map = calloc(MAP_LENGTH, sizeof(XkbSymMapRec));
if (map->key_sym_map == NULL)
return BadAlloc;
}
@@ -113,8 +111,7 @@ XkbAllocClientMap(XkbDescPtr xkb, unsigned which, unsigned nTotalTypes)
(xkb->max_key_code < xkb->min_key_code))
return BadMatch;
if (map->modmap == NULL) {
- i = xkb->max_key_code + 1;
- map->modmap = calloc(i, sizeof(unsigned char));
+ map->modmap = calloc(MAP_LENGTH, sizeof(unsigned char));
if (map->modmap == NULL)
return BadAlloc;
}
@@ -147,8 +144,7 @@ XkbAllocServerMap(XkbDescPtr xkb, unsigned which, unsigned nNewActions)
(xkb->max_key_code < xkb->min_key_code))
return BadMatch;
if (map->explicit == NULL) {
- i = xkb->max_key_code + 1;
- map->explicit = calloc(i, sizeof(unsigned char));
+ map->explicit = calloc(MAP_LENGTH, sizeof(unsigned char));
if (map->explicit == NULL)
return BadAlloc;
}
@@ -183,8 +179,7 @@ XkbAllocServerMap(XkbDescPtr xkb, unsigned which, unsigned nNewActions)
((map->size_acts - map->num_acts) * sizeof(XkbAction)));
}
if (map->key_acts == NULL) {
- i = xkb->max_key_code + 1;
- map->key_acts = calloc(i, sizeof(unsigned short));
+ map->key_acts = calloc(MAP_LENGTH, sizeof(unsigned short));
if (map->key_acts == NULL)
return BadAlloc;
}
@@ -195,8 +190,7 @@ XkbAllocServerMap(XkbDescPtr xkb, unsigned which, unsigned nNewActions)
(xkb->max_key_code < xkb->min_key_code))
return BadMatch;
if (map->behaviors == NULL) {
- i = xkb->max_key_code + 1;
- map->behaviors = calloc(i, sizeof(XkbBehavior));
+ map->behaviors = calloc(MAP_LENGTH, sizeof(XkbBehavior));
if (map->behaviors == NULL)
return BadAlloc;
}
@@ -207,8 +201,7 @@ XkbAllocServerMap(XkbDescPtr xkb, unsigned which, unsigned nNewActions)
(xkb->max_key_code < xkb->min_key_code))
return BadMatch;
if (map->vmodmap == NULL) {
- i = xkb->max_key_code + 1;
- map->vmodmap = calloc(i, sizeof(unsigned short));
+ map->vmodmap = calloc(MAP_LENGTH, sizeof(unsigned short));
if (map->vmodmap == NULL)
return BadAlloc;
}
@@ -649,18 +642,9 @@ XkbChangeKeycodeRange(XkbDescPtr xkb,
if (maxKC > xkb->max_key_code) {
if (changes)
changes->map.max_key_code = maxKC;
- tmp = maxKC - xkb->max_key_code;
+ tmp = MAP_LENGTH - xkb->max_key_code;
if (xkb->map) {
if (xkb->map->key_sym_map) {
- XkbSymMapRec *prev_key_sym_map = xkb->map->key_sym_map;
-
- xkb->map->key_sym_map = reallocarray(xkb->map->key_sym_map,
- maxKC + 1,
- sizeof(XkbSymMapRec));
- if (!xkb->map->key_sym_map) {
- free(prev_key_sym_map);
- return BadAlloc;
- }
memset((char *) &xkb->map->key_sym_map[xkb->max_key_code], 0,
tmp * sizeof(XkbSymMapRec));
if (changes) {
@@ -673,15 +657,6 @@ XkbChangeKeycodeRange(XkbDescPtr xkb,
}
}
if (xkb->map->modmap) {
- unsigned char *prev_modmap = xkb->map->modmap;
-
- xkb->map->modmap = reallocarray(xkb->map->modmap,
- maxKC + 1,
- sizeof(unsigned char));
- if (!xkb->map->modmap) {
- free(prev_modmap);
- return BadAlloc;
- }
memset((char *) &xkb->map->modmap[xkb->max_key_code], 0, tmp);
if (changes) {
changes->map.changed = _ExtendRange(changes->map.changed,
@@ -696,15 +671,6 @@ XkbChangeKeycodeRange(XkbDescPtr xkb,
}
if (xkb->server) {
if (xkb->server->behaviors) {
- XkbBehavior *prev_behaviors = xkb->server->behaviors;
-
- xkb->server->behaviors = reallocarray(xkb->server->behaviors,
- maxKC + 1,
- sizeof(XkbBehavior));
- if (!xkb->server->behaviors) {
- free(prev_behaviors);
- return BadAlloc;
- }
memset((char *) &xkb->server->behaviors[xkb->max_key_code], 0,
tmp * sizeof(XkbBehavior));
if (changes) {
@@ -718,15 +684,6 @@ XkbChangeKeycodeRange(XkbDescPtr xkb,
}
}
if (xkb->server->key_acts) {
- unsigned short *prev_key_acts = xkb->server->key_acts;
-
- xkb->server->key_acts = reallocarray(xkb->server->key_acts,
- maxKC + 1,
- sizeof(unsigned short));
- if (!xkb->server->key_acts) {
- free(prev_key_acts);
- return BadAlloc;
- }
memset((char *) &xkb->server->key_acts[xkb->max_key_code], 0,
tmp * sizeof(unsigned short));
if (changes) {
@@ -740,15 +697,6 @@ XkbChangeKeycodeRange(XkbDescPtr xkb,
}
}
if (xkb->server->vmodmap) {
- unsigned short *prev_vmodmap = xkb->server->vmodmap;
-
- xkb->server->vmodmap = reallocarray(xkb->server->vmodmap,
- maxKC + 1,
- sizeof(unsigned short));
- if (!xkb->server->vmodmap) {
- free(prev_vmodmap);
- return BadAlloc;
- }
memset((char *) &xkb->server->vmodmap[xkb->max_key_code], 0,
tmp * sizeof(unsigned short));
if (changes) {
@@ -763,14 +711,6 @@ XkbChangeKeycodeRange(XkbDescPtr xkb,
}
}
if ((xkb->names) && (xkb->names->keys)) {
- XkbKeyNameRec *prev_keys = xkb->names->keys;
-
- xkb->names->keys = reallocarray(xkb->names->keys,
- maxKC + 1, sizeof(XkbKeyNameRec));
- if (!xkb->names->keys) {
- free(prev_keys);
- return BadAlloc;
- }
memset((char *) &xkb->names->keys[xkb->max_key_code], 0,
tmp * sizeof(XkbKeyNameRec));
if (changes) {
diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c
index ff0785ddb..8ca92bd15 100644
--- a/xkb/xkbUtils.c
+++ b/xkb/xkbUtils.c
@@ -930,6 +930,7 @@ _XkbCopyClientMap(XkbDescPtr src, XkbDescPtr dst)
{
void *tmp = NULL;
int i;
+ int gap;
XkbKeyTypePtr stype = NULL, dtype = NULL;
/* client map */
@@ -959,15 +960,20 @@ _XkbCopyClientMap(XkbDescPtr src, XkbDescPtr dst)
}
dst->map->num_syms = src->map->num_syms;
dst->map->size_syms = src->map->size_syms;
+ gap = MAP_LENGTH - (src->max_key_code + 1);
if (src->map->key_sym_map) {
- if (src->max_key_code != dst->max_key_code) {
+ if (!dst->map->key_sym_map) {
tmp = reallocarray(dst->map->key_sym_map,
- src->max_key_code + 1, sizeof(XkbSymMapRec));
+ MAP_LENGTH, sizeof(XkbSymMapRec));
if (!tmp)
return FALSE;
dst->map->key_sym_map = tmp;
}
+ if (gap > 0) {
+ memset((char *) &dst->map->key_sym_map[gap], 0,
+ gap * sizeof(XkbSymMapRec));
+ }
memcpy(dst->map->key_sym_map, src->map->key_sym_map,
(src->max_key_code + 1) * sizeof(XkbSymMapRec));
}
@@ -1138,12 +1144,15 @@ _XkbCopyClientMap(XkbDescPtr src, XkbDescPtr dst)
}
if (src->map->modmap) {
- if (src->max_key_code != dst->max_key_code) {
- tmp = realloc(dst->map->modmap, src->max_key_code + 1);
+ if (!dst->map->modmap) {
+ tmp = realloc(dst->map->modmap, MAP_LENGTH);
if (!tmp)
return FALSE;
dst->map->modmap = tmp;
}
+ if (gap > 0) {
+ memset(dst->map->modmap + gap, 0, gap);
+ }
memcpy(dst->map->modmap, src->map->modmap, src->max_key_code + 1);
}
else {
@@ -1163,6 +1172,7 @@ static Bool
_XkbCopyServerMap(XkbDescPtr src, XkbDescPtr dst)
{
void *tmp = NULL;
+ int gap;
/* server map */
if (src->server) {
@@ -1173,13 +1183,16 @@ _XkbCopyServerMap(XkbDescPtr src, XkbDescPtr dst)
dst->server = tmp;
}
+ gap = MAP_LENGTH - (src->max_key_code + 1);
if (src->server->explicit) {
- if (src->max_key_code != dst->max_key_code) {
- tmp = realloc(dst->server->explicit, src->max_key_code + 1);
+ if (!dst->server->explicit) {
+ tmp = realloc(dst->server->explicit, MAP_LENGTH);
if (!tmp)
return FALSE;
dst->server->explicit = tmp;
}
+ if (gap > 0)
+ memset(dst->server->explicit + gap, 0, gap);
memcpy(dst->server->explicit, src->server->explicit,
src->max_key_code + 1);
}
@@ -1207,13 +1220,15 @@ _XkbCopyServerMap(XkbDescPtr src, XkbDescPtr dst)
dst->server->num_acts = src->server->num_acts;
if (src->server->key_acts) {
- if (src->max_key_code != dst->max_key_code) {
+ if (!dst->server->key_acts) {
tmp = reallocarray(dst->server->key_acts,
- src->max_key_code + 1, sizeof(unsigned short));
+ MAP_LENGTH, sizeof(unsigned short));
if (!tmp)
return FALSE;
dst->server->key_acts = tmp;
}
+ if (gap > 0)
+ memset((char *) &dst->server->key_acts[gap], 0, gap * sizeof(unsigned short));
memcpy(dst->server->key_acts, src->server->key_acts,
(src->max_key_code + 1) * sizeof(unsigned short));
}
@@ -1223,13 +1238,15 @@ _XkbCopyServerMap(XkbDescPtr src, XkbDescPtr dst)
}
if (src->server->behaviors) {
- if (src->max_key_code != dst->max_key_code) {
+ if (!dst->server->behaviors) {
tmp = reallocarray(dst->server->behaviors,
- src->max_key_code + 1, sizeof(XkbBehavior));
+ MAP_LENGTH, sizeof(XkbBehavior));
if (!tmp)
return FALSE;
dst->server->behaviors = tmp;
}
+ if (gap > 0)
+ memset((char *) &dst->server->behaviors[gap], 0, gap * sizeof(XkbBehavior));
memcpy(dst->server->behaviors, src->server->behaviors,
(src->max_key_code + 1) * sizeof(XkbBehavior));
}
@@ -1241,13 +1258,15 @@ _XkbCopyServerMap(XkbDescPtr src, XkbDescPtr dst)
memcpy(dst->server->vmods, src->server->vmods, XkbNumVirtualMods);
if (src->server->vmodmap) {
- if (src->max_key_code != dst->max_key_code) {
+ if (!dst->server->vmodmap) {
tmp = reallocarray(dst->server->vmodmap,
- src->max_key_code + 1, sizeof(unsigned short));
+ MAP_LENGTH, sizeof(unsigned short));
if (!tmp)
return FALSE;
dst->server->vmodmap = tmp;
}
+ if (gap > 0)
+ memset((char *) &dst->server->vmodmap[gap], 0, gap * sizeof(unsigned short));
memcpy(dst->server->vmodmap, src->server->vmodmap,
(src->max_key_code + 1) * sizeof(unsigned short));
}
@@ -1268,6 +1287,7 @@ static Bool
_XkbCopyNames(XkbDescPtr src, XkbDescPtr dst)
{
void *tmp = NULL;
+ int gap;
/* names */
if (src->names) {
@@ -1277,14 +1297,17 @@ _XkbCopyNames(XkbDescPtr src, XkbDescPtr dst)
return FALSE;
}
+ gap = MAP_LENGTH - (src->max_key_code + 1);
if (src->names->keys) {
- if (src->max_key_code != dst->max_key_code) {
- tmp = reallocarray(dst->names->keys, src->max_key_code + 1,
+ if (!dst->names->keys) {
+ tmp = reallocarray(dst->names->keys, MAP_LENGTH,
sizeof(XkbKeyNameRec));
if (!tmp)
return FALSE;
dst->names->keys = tmp;
}
+ if (gap > 0)
+ memset((char *) &dst->names->keys[gap], 0, gap * sizeof(XkbKeyNameRec));
memcpy(dst->names->keys, src->names->keys,
(src->max_key_code + 1) * sizeof(XkbKeyNameRec));
}
More information about the xorg-commit
mailing list