HID: i2c-hid: fix OOB write in i2c_hid_set_or_send_report()
authorDmitry Torokhov <dtor@chromium.org>
Mon, 14 Mar 2016 22:21:04 +0000 (15:21 -0700)
committerJiri Kosina <jkosina@suse.cz>
Tue, 15 Mar 2016 13:27:10 +0000 (14:27 +0100)
Even though hid_hw_* checks that passed in data_len is less than
HID_MAX_BUFFER_SIZE it is not enough, as i2c-hid does not necessarily
allocate buffers of HID_MAX_BUFFER_SIZE but rather checks all device
reports and select largest size. In-kernel users normally just send as much
data as report needs, so there is no problem, but hidraw users can do
whatever they please:

BUG: KASAN: slab-out-of-bounds in memcpy+0x34/0x54 at addr ffffffc07135ea80
Write of size 4101 by task syz-executor/8747
CPU: 2 PID: 8747 Comm: syz-executor Tainted: G    BU         3.18.0 #37
Hardware name: Google Tegra210 Smaug Rev 1,3+ (DT)
Call trace:
[<ffffffc00020ebcc>] dump_backtrace+0x0/0x258 arch/arm64/kernel/traps.c:83
[<ffffffc00020ee40>] show_stack+0x1c/0x2c arch/arm64/kernel/traps.c:172
[<     inline     >] __dump_stack lib/dump_stack.c:15
[<ffffffc001958114>] dump_stack+0x90/0x140 lib/dump_stack.c:50
[<     inline     >] print_error_description mm/kasan/report.c:97
[<     inline     >] kasan_report_error mm/kasan/report.c:278
[<ffffffc0004597dc>] kasan_report+0x268/0x530 mm/kasan/report.c:305
[<ffffffc0004592e8>] __asan_storeN+0x20/0x150 mm/kasan/kasan.c:718
[<ffffffc0004594e0>] memcpy+0x30/0x54 mm/kasan/kasan.c:299
[<ffffffc001306354>] __i2c_hid_command+0x2b0/0x7b4 drivers/hid/i2c-hid/i2c-hid.c:178
[<     inline     >] i2c_hid_set_or_send_report drivers/hid/i2c-hid/i2c-hid.c:321
[<ffffffc0013079a0>] i2c_hid_output_raw_report.isra.2+0x3d4/0x4b8 drivers/hid/i2c-hid/i2c-hid.c:589
[<ffffffc001307ad8>] i2c_hid_output_report+0x54/0x68 drivers/hid/i2c-hid/i2c-hid.c:602
[<     inline     >] hid_hw_output_report include/linux/hid.h:1039
[<ffffffc0012cc7a0>] hidraw_send_report+0x400/0x414 drivers/hid/hidraw.c:154
[<ffffffc0012cc7f4>] hidraw_write+0x40/0x64 drivers/hid/hidraw.c:177
[<ffffffc0004681dc>] vfs_write+0x1d4/0x3cc fs/read_write.c:534
[<     inline     >] SYSC_pwrite64 fs/read_write.c:627
[<ffffffc000468984>] SyS_pwrite64+0xec/0x144 fs/read_write.c:614
Object at ffffffc07135ea80, in cache kmalloc-512
Object allocated with size 268 bytes.

Let's check data length against the buffer size before attempting to copy
data over.

Cc: stable@vger.kernel.org
Reported-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
drivers/hid/i2c-hid/i2c-hid.c

index 5b10a5d6e2d176cdbd448451eceaf2c03cf4ed1e..2e021ba8ff052eacdf1dda7bf68af24d9d628543 100644 (file)
@@ -283,17 +283,21 @@ static int i2c_hid_set_or_send_report(struct i2c_client *client, u8 reportType,
        u16 dataRegister = le16_to_cpu(ihid->hdesc.wDataRegister);
        u16 outputRegister = le16_to_cpu(ihid->hdesc.wOutputRegister);
        u16 maxOutputLength = le16_to_cpu(ihid->hdesc.wMaxOutputLength);
+       u16 size;
+       int args_len;
+       int index = 0;
+
+       i2c_hid_dbg(ihid, "%s\n", __func__);
+
+       if (data_len > ihid->bufsize)
+               return -EINVAL;
 
-       /* hid_hw_* already checked that data_len < HID_MAX_BUFFER_SIZE */
-       u16 size =      2                       /* size */ +
+       size =          2                       /* size */ +
                        (reportID ? 1 : 0)      /* reportID */ +
                        data_len                /* buf */;
-       int args_len =  (reportID >= 0x0F ? 1 : 0) /* optional third byte */ +
+       args_len =      (reportID >= 0x0F ? 1 : 0) /* optional third byte */ +
                        2                       /* dataRegister */ +
                        size                    /* args */;
-       int index = 0;
-
-       i2c_hid_dbg(ihid, "%s\n", __func__);
 
        if (!use_data && maxOutputLength == 0)
                return -ENOSYS;