Input: fix two kernel stack disclosures via ioctl
Subsystems (3)
cover Input: fix two kernel stack disclosures via ioctl
Two places in the input layer copy a user supplied, possibly short buffer into an uninitialized on-stack structure and then keep the whole structure, including the part that was never written. In both cases the stale stack bytes can be read back from userspace.
Patch 1: EVIOCSABS copies min(_IOC_SIZE(cmd), sizeof(struct input_absinfo)) bytes into an uninitialized struct and stores the result in dev->absinfo[]. EVIOCGABS returns it. Only the resolution field is currently cleared for short sizes.
Patch 2: the compat path of input_ff_effect_from_user() aliases the native struct ff_effect with the smaller struct ff_effect_compat and copies only the compat sized prefix, leaving the tail untouched. input_ff_upload() stores the full structure, which a uinput based force feedback daemon can read back via UI_BEGIN_FF_UPLOAD.
Both are fixed by zeroing the structure before the copy. The patches are independent of each other.
Compile tested on x86_64 with CONFIG_INPUT_EVDEV, CONFIG_INPUT_UINPUT and CONFIG_IA32_EMULATION enabled.
Iván Ezequiel Rodriguez (2):
Input: evdev: zero absinfo before partial copy in EVIOCSABS
Input: zero ff_effect before compat copy in input_ff_effect_from_user
drivers/input/evdev.c | 2 ++ drivers/input/input-compat.c | 2 ++ 2 files changed, 4 insertions(+)
Signature · 1 line
2.43.0
PATCH 1/2 Input: evdev: zero absinfo before partial copy in EVIOCSABS
The EVIOCSABS handler copies at most the user supplied ioctl size into an uninitialized on-stack struct input_absinfo:
if (copy_from_user(&abs, p, min_t(size_t, size, sizeof(struct input_absinfo))))
The size comes from _IOC_SIZE() of the ioctl command and is therefore fully controlled by userspace. A short size leaves the trailing part of the structure holding whatever was on the kernel stack, and the whole structure is then stored into the device:
dev->absinfo[t] = abs;
EVIOCGABS hands that back to userspace, disclosing the stale stack bytes. Only the resolution field is currently cleared, which covers the legacy struct layout but not an arbitrarily short size.
Zero the structure before the copy so any part not supplied by the caller reads back as zero. The existing resolution fixup is kept, since it also handles a size that partially overlaps that field.
--- drivers/input/evdev.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/input/evdev.c +2 -0
@@ -1229,6 +1229,8 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd, t =_IOC_NR (cmd) &ABS_MAX ; +memset (&abs,0 ,sizeof (abs)); +if (copy_from_user (&abs, p,min_t (size_t, size,sizeof (struct input_absinfo ))))return -EFAULT ;
Signature · 1 line
2.43.0
PATCH 2/2 Input: zero ff_effect before compat copy in input_ff_effect_from_user
In reply to Iván Ezequiel RodriguezIn the compat path input_ff_effect_from_user() aliases the caller's native struct ff_effect with the smaller struct ff_effect_compat and copies only the compat sized prefix:
compat_effect = (struct ff_effect_compat *)effect; if (copy_from_user(compat_effect, buffer, sizeof(struct ff_effect_compat)))
The tail of the native structure is never written. Callers pass an uninitialized on-stack object, for example evdev_do_ioctl() for EVIOCSFF, so those bytes keep their previous stack contents. input_ff_upload() then stores the full native structure in ff->effects[id], from where a uinput based force feedback daemon can read it back via UI_BEGIN_FF_UPLOAD, disclosing kernel stack memory to userspace.
Zero the effect before the compat copy.
--- drivers/input/input-compat.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/input/input-compat.c +2 -0
@@ -76,6 +76,8 @@ int input_ff_effect_from_user(const char __user *buffer, size_t size,*/ compat_effect = (struct ff_effect_compat *)effect; +memset (effect,0 ,sizeof (*effect)); +if (copy_from_user (compat_effect, buffer,sizeof (struct ff_effect_compat )))return -EFAULT ;
Signature · 1 line
2.43.0
Dmitry Torokhov
maintainerQuoted 16 lines · Iván Ezequiel Rodriguez
Two places in the input layer copy a user supplied, possibly short buffer into an uninitialized on-stack structure and then keep the whole structure, including the part that was never written. In both cases the stale stack bytes can be read back from userspace.
Patch 1: EVIOCSABS copies
min(_IOC_SIZE(cmd), sizeof(struct input_absinfo))bytes into an uninitialized struct and stores the result in dev->absinfo[]. EVIOCGABS returns it. Only the resolution field is currently cleared for short sizes.Patch 2: the compat path of
input_ff_effect_from_user()aliases the native structff_effectwith the smaller structff_effect_compatand copies only the compat sized prefix, leaving the tail untouched.input_ff_upload()stores the full structure, which a uinput based force feedback daemon can read back viaUI_BEGIN_FF_UPLOAD.
Quoted 5 lines
Both are fixed by zeroing the structure before the copy. The patches are independent of each other.
Compile tested on
x86_64withCONFIG_INPUT_EVDEV,CONFIG_INPUT_UINPUTandCONFIG_IA32_EMULATIONenabled.
Applied the lot, thank you.
Signature · 1 line
Dmitry