frederik
/
xserver
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1762>
This commit is contained in:
Olivier Fourdan 2025-01-10 15:02:54 +01:00
parent 8707d2835c
commit 92bcebfd7e
2 changed files with 44 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
xkb/XKBMAlloc.c
View File

@ -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) {

51
xkb/xkbUtils.c
View File

@ -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));
}