Bug 191811 - uinput crashes with misconfigured ABS axes
Summary: uinput crashes with misconfigured ABS axes
Status: NEW
Alias: None
Product: Drivers
Classification: Unclassified
Component: Input Devices (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Dmitry Torokhov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-03 12:12 UTC by Rodrigo Rivas Costa
Modified: 2017-01-04 12:34 UTC (History)
2 users (show)

See Also:
Kernel Version: 4.8
Tree: Mainline
Regression: No


Attachments

Description Rodrigo Rivas Costa 2017-01-03 12:12:56 UTC
I am creating a uinput device with ABS axes ABS_X & ABS_Y. But then I send events with ABS_RX (wrong, I know) and this happens:

Jan 03 12:32:33 casa kernel: BUG: unable to handle kernel NULL pointer dereference at 0000000000000024
Jan 03 12:32:33 casa kernel: IP: [<ffffffff81483fe4>] input_handle_event+0x344/0x510
Jan 03 12:32:33 casa kernel: PGD 19315e067 PUD 151fde067 PMD 0 
Jan 03 12:32:33 casa kernel: Oops: 0000 [#1] PREEMPT SMP
Jan 03 12:32:33 casa kernel: Modules linked in: uinput rfcomm bridge stp llc bnep iptable_filter btusb btrtl btbcm btintel bluetooth rfkill usblp input_leds led_class joydev hid_pl mousedev ff_memless nvidia_drm(PO) nvidia_modeset(PO) nls
Jan 03 12:32:33 casa kernel:  tpm sch_fq_codel nfsd auth_rpcgss oid_registry nfs_acl lockd grace sunrpc fuse vboxnetflt(O) vboxnetadp(O) pci_stub vboxpci(O) vboxdrv(O) usbip_host usbip_core ip_tables x_tables ext4 crc16 jbd2 fscrypto mbca
Jan 03 12:32:33 casa kernel: CPU: 1 PID: 1150 Comm: a.out Tainted: P           O    4.8.13-1-ARCH #1
Jan 03 12:32:33 casa kernel: Hardware name: MEDION H61H2-LM3/H61H2-LM3, BIOS EM0422-M8 04/22/2012
Jan 03 12:32:33 casa kernel: task: ffff880156258000 task.stack: ffff88019151c000
Jan 03 12:32:33 casa kernel: RIP: 0010:[<ffffffff81483fe4>]  [<ffffffff81483fe4>] input_handle_event+0x344/0x510
Jan 03 12:32:33 casa kernel: RSP: 0018:ffff88019151fd50  EFLAGS: 00010006
Jan 03 12:32:33 casa kernel: RAX: 0000000000000018 RBX: ffff8801930c9000 RCX: 0000000000000003
Jan 03 12:32:33 casa kernel: RDX: 0000000000000000 RSI: 0000000000000003 RDI: ffff8801930c9000
Jan 03 12:32:33 casa kernel: RBP: ffff88019151fd80 R08: 0000000000000018 R09: 0000000000000008
Jan 03 12:32:33 casa kernel: R10: 0000000000000871 R11: 0000000000000246 R12: 0000000000000003
Jan 03 12:32:33 casa kernel: R13: 0000000000000001 R14: 000000000000008b R15: ffff8801930c9208
Jan 03 12:32:33 casa kernel: FS:  00007f91293cdb80(0000) GS:ffff88019ec80000(0000) knlGS:0000000000000000
Jan 03 12:32:33 casa kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Jan 03 12:32:33 casa kernel: CR2: 0000000000000024 CR3: 0000000151f2c000 CR4: 00000000000406e0
Jan 03 12:32:33 casa kernel: Stack:
Jan 03 12:32:33 casa kernel:  00000001813eb30e ffff8801930c9000 0000000000000003 000000000000008b
Jan 03 12:32:33 casa kernel:  0000000000000001 ffff8801930c9208 ffff88019151fdc0 ffffffff81484204
Jan 03 12:32:33 casa kernel:  0000000000000286 0000000000000018 00007fff49e31cb0 ffff880191520000
Jan 03 12:32:33 casa kernel: Call Trace:
Jan 03 12:32:33 casa kernel:  [<ffffffff81484204>] input_event+0x54/0x80
Jan 03 12:32:33 casa kernel:  [<ffffffffa15254d6>] uinput_write+0x156/0x380 [uinput]
Jan 03 12:32:33 casa kernel:  [<ffffffff813ef086>] ? tty_ldisc_deref+0x16/0x20
Jan 03 12:32:33 casa kernel:  [<ffffffff81208797>] __vfs_write+0x37/0x140
Jan 03 12:32:33 casa kernel:  [<ffffffff81209566>] vfs_write+0xb6/0x1a0
Jan 03 12:32:33 casa kernel:  [<ffffffff8120a9e5>] SyS_write+0x55/0xc0
Jan 03 12:32:33 casa kernel:  [<ffffffff815f8032>] entry_SYSCALL_64_fastpath+0x1a/0xa4
Jan 03 12:32:33 casa kernel: Code: fe ff ff 41 8d 4d d0 83 f9 0d 40 0f 96 c7 0f 86 80 01 00 00 48 8d 0c 40 48 8b 83 60 01 00 00 48 8d 04 c8 48 85 c0 49 89 c0 74 75 <8b> 40 0c 41 8b 08 85 c0 74 5f 89 c6 41 89 ca c1 ee 1f 01 c6 41 
Jan 03 12:32:33 casa kernel: RIP  [<ffffffff81483fe4>] input_handle_event+0x344/0x510
Jan 03 12:32:33 casa kernel:  RSP <ffff88019151fd50>
Jan 03 12:32:33 casa kernel: CR2: 0000000000000024
Jan 03 12:32:33 casa kernel: ---[ end trace 397fc46b02970a3c ]---


I think that the problem is with the way input_dev::absbit is managed from uinput.c. In input.c, input_set_capability() has this code:

	case EV_ABS:
		input_alloc_absinfo(dev);
		if (!dev->absinfo)
			return;

		__set_bit(code, dev->absbit);
		break;

while the equivalent from uinput.c, in uinput_ioctl_handler():

		case UI_SET_ABSBIT:
			retval = uinput_set_bit(arg, absbit, ABS_MAX);
			goto out;

does not call input_alloc_absinfo() until the uses issues a UI_ABS_SETUP.
The rest of code from input.c assumes that when any absbit is set, dev->absinfo is not NULL. But from a uinput device that may not be true.
Comment 1 Dmitry Torokhov 2017-01-03 21:17:31 UTC
It appears you are mixing the old and new way of creating uinput device. Do you mind listing all operations that you do in order to set up your uinput device?
Comment 2 Rodrigo Rivas Costa 2017-01-03 23:04:03 UTC
Sorry, my bad, I thought that it was easier to reproduce.

I managed to crash my system with this code:

    #include <fcntl.h>
    #include <unistd.h>
    #include <string.h>
    #include <sys/ioctl.h>
    #include <linux/uinput.h>

    int main()
    {
        int ui = open("/dev/uinput", O_RDWR | O_CLOEXEC);

        struct uinput_setup us = {0};
        us.id.bustype = BUS_VIRTUAL;
        us.id.version = 1;
        strcpy(us.name, "Test");
        ioctl(ui, UI_DEV_SETUP, &us);
        ioctl(ui, UI_SET_PHYS, "Test");

        ioctl(ui, UI_SET_EVBIT, EV_ABS);
        ioctl(ui, UI_SET_ABSBIT, ABS_X);
        ioctl(ui, UI_SET_ABSBIT, ABS_Y);
        ioctl(ui, UI_DEV_CREATE, 0);

        struct input_event e = {0};
        e.type = EV_ABS;
        e.code = ABS_Y;
        e.value = 100;
        write(ui, &e, sizeof(e));
    }

I thought that the "new way" was to use UI_DEV_SETUP instead of write() the "struct uinput_setup", isn't it? I'm trying to use the new style, but I forgot to do the UI_ABS_SETUP. Curiously, if I write() a ABS_X event, it will not crash.

Is UI_ABS_SETUP really mandatory? If UI_ABS_SETUP does an implicit UI_SET_ABSBIT, and a UI_SET_ABSBIT cannot be used without a UI_ABS_SETUP, then UI_SET_ABSBIT is useless! Or is UI_SET_ABSBIT part of the "old way"? If that is the case it should be marked as such in uinput.h.
Comment 3 David Herrmann 2017-01-04 11:47:46 UTC
Yeah, you mix the new ioctls with the old. You cannot use UI_DEV_SETUP with UI_SET_ABSBIT. You need to specify the ABS parameters, and this is either done via UI_ABS_SETUP or the old write() method.

So just replace your UI_SET_ABSBIT calls with UI_ABS_SETUP and it should work fine (but pass `struct uinput_abs_setup` as argument). Btw., the libevdev project contains a lot of helpers to deal with uinput as well as evdev. Even if you don't end up using it, always worth a look to see how things were designed to be used.

We still need to fix the NULL-deref, though. How about this:

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 92595b98e7ed..dd765ab1a332 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -832,7 +832,11 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
                        goto out;
 
                case UI_SET_ABSBIT:
-                       retval = uinput_set_bit(arg, absbit, ABS_MAX);
+                       input_alloc_absinfo(udev->dev);
+                       if (!udev->dev->absinfo)
+                               retval = -ENOMEM;
+                       else
+                               retval = uinput_set_bit(arg, absbit, ABS_MAX);
                        goto out;
 
                case UI_SET_MSCBIT:
Comment 4 Rodrigo Rivas Costa 2017-01-04 12:34:22 UTC
Yeah, my code was badly written and easy to fix. Thanks for the libevdev point (you cannot have too many references). Until now I was using the kernel source as reference, because the documentation for uinput is somewhat missing...

Your patch will certainly fix the NULL reference. I would have implemented it by calling input_set_capability(), just in case in the future other invariants arise:

		case UI_SET_RELBIT:
-			retval = uinput_set_bit(arg, relbit, REL_MAX);
+                       input_set_capability(udev->deev, EV_REL, arg);
			goto out;
		case UI_SET_ABSBIT:
-			retval = uinput_set_bit(arg, absbit, ABS_MAX);
+                       input_set_capability(udev->deev, EV_ABS, arg);
			goto out;

And so on for all the other bits. But then you'll miss the retval, so you'll need a few more changes, and I don't know if it is worth it...

Note You need to log in before you can comment on or make changes to this bug.