WARNING in input_mt_init_slots

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: f5e6c330 Merge tag 'spi-fix-v5.11-rc2' of git://git.kernel..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=121ca757500000
kernel config: https://syzkaller.appspot.com/x/.config?x=8aa30b9da402d224
dashboard link: https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17eee7db500000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1456a0cf500000

Bisection is inconclusive: the issue happens on the oldest tested release.

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=10778f4f500000
final oops: https://syzkaller.appspot.com/x/report.txt?x=12778f4f500000
console output: https://syzkaller.appspot.com/x/log.txt?x=14778f4f500000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+0122fa...@syzkaller.appspotmail.com

------------[ cut here ]------------
WARNING: CPU: 1 PID: 8471 at mm/page_alloc.c:4976 __alloc_pages_nodemask+0x5f8/0x730 mm/page_alloc.c:5011
Modules linked in:
CPU: 1 PID: 8471 Comm: syz-executor772 Not tainted 5.11.0-rc2-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:__alloc_pages_nodemask+0x5f8/0x730 mm/page_alloc.c:4976
Code: 00 00 0c 00 0f 85 a7 00 00 00 8b 3c 24 4c 89 f2 44 89 e6 c6 44 24 70 00 48 89 6c 24 58 e8 d0 d7 ff ff 49 89 c5 e9 ea fc ff ff <0f> 0b e9 b5 fd ff ff 89 74 24 14 4c 89 4c 24 08 4c 89 74 24 18 e8
RSP: 0018:ffffc900015efbb8 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 1ffff920002bdf7b RCX: 0000000000000000
RDX: 0000000000000000 RSI: dffffc0000000000 RDI: 0000000000040dc0
RBP: 0000000000040dc0 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff81b1f7e1 R11: 0000000000000000 R12: 000000000000000b
R13: 000000000000000b R14: 0000000000000000 R15: ffff888025a36168
FS: 00000000006fc880(0000) GS:ffff8880b9f00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000000 CR3: 0000000012990000 CR4: 00000000001506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
alloc_pages_current+0x18c/0x2a0 mm/mempolicy.c:2267
alloc_pages include/linux/gfp.h:547 [inline]
kmalloc_order+0x2e/0xb0 mm/slab_common.c:837
kmalloc_order_trace+0x14/0x120 mm/slab_common.c:853
kmalloc include/linux/slab.h:557 [inline]
kzalloc include/linux/slab.h:682 [inline]
input_mt_init_slots drivers/input/input-mt.c:49 [inline]
input_mt_init_slots+0xd9/0x5f0 drivers/input/input-mt.c:38
uinput_create_device drivers/input/misc/uinput.c:327 [inline]
uinput_ioctl_handler.isra.0+0x81d/0x1dc0 drivers/input/misc/uinput.c:870
vfs_ioctl fs/ioctl.c:48 [inline]
__do_sys_ioctl fs/ioctl.c:753 [inline]
__se_sys_ioctl fs/ioctl.c:739 [inline]
__x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x440e99
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 7b 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc567274b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440e99
RDX: 0000000000000000 RSI: 0000000000005501 RDI: 0000000000000003
RBP: 00000000006cb018 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000004002c8 R11: 0000000000000246 R12: 00000000004026a0
R13: 0000000000402730 R14: 0000000000000000 R15: 0000000000000000


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[Tetsuo Handa]

syzbot is reporting too large allocation at input_mt_init_slots() {1], for
num_slots is supplied from userspace using ioctl(UI_DEV_CREATE).
Also, replace n2 with array_size(), for 32bits variable n2 will overflow
if num_slots >= 65536.

Link: https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5 [1]
Reported-by: syzbot <syzbot+0122fa...@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
---
drivers/input/input-mt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 14b53dac1253..cf74579462ba 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -47,7 +47,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
if (mt)
return mt->num_slots != num_slots ? -EINVAL : 0;

- mt = kzalloc(struct_size(mt, slots, num_slots), GFP_KERNEL);
+ mt = kzalloc(struct_size(mt, slots, num_slots), GFP_KERNEL | __GFP_NOWARN);
if (!mt)
goto err_mem;

@@ -80,8 +80,8 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
if (flags & INPUT_MT_SEMI_MT)
__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
if (flags & INPUT_MT_TRACK) {
- unsigned int n2 = num_slots * num_slots;
- mt->red = kcalloc(n2, sizeof(*mt->red), GFP_KERNEL);
+ mt->red = kcalloc(array_size(num_slots, num_slots),
+ sizeof(*mt->red), GFP_KERNEL | __GFP_NOWARN);
if (!mt->red)
goto err_mem;
}
--
2.34.1

[Dmitry Torokhov]

Hi Tetsuo,

On Sat, Nov 19, 2022 at 04:09:56PM +0900, Tetsuo Handa wrote:
> syzbot is reporting too large allocation at input_mt_init_slots() {1], for
> num_slots is supplied from userspace using ioctl(UI_DEV_CREATE).
> Also, replace n2 with array_size(), for 32bits variable n2 will overflow
> if num_slots >= 65536.

Not really keen on fiddling with the memory allocator flags just to
appease syzbot. Maybe keep them as is, and simply limit the number of
slots to something more reasonable, like 64, and return -EINVAL if it is
above?

Thanks.

--
Dmitry

[Tetsuo Handa]

On 2022/11/23 8:23, Dmitry Torokhov wrote:
> Hi Tetsuo,
>
> On Sat, Nov 19, 2022 at 04:09:56PM +0900, Tetsuo Handa wrote:
>> syzbot is reporting too large allocation at input_mt_init_slots() {1], for
>> num_slots is supplied from userspace using ioctl(UI_DEV_CREATE).
>> Also, replace n2 with array_size(), for 32bits variable n2 will overflow
>> if num_slots >= 65536.
>
> Not really keen on fiddling with the memory allocator flags just to
> appease syzbot. Maybe keep them as is, and simply limit the number of
> slots to something more reasonable, like 64, and return -EINVAL if it is
> above?
>

Hmm, although most users seem to pass values within "unsigned char" range,
not limited to syzbot.

drivers/input/misc/uinput.c has

nslot = input_abs_get_max(dev, ABS_MT_SLOT) + 1;
error = input_mt_init_slots(dev, nslot, 0);

and drivers/virtio/virtio_input.c has

nslots = input_abs_get_max(vi->idev, ABS_MT_SLOT) + 1;
err = input_mt_init_slots(vi->idev, nslots, 0);

and drivers/input/misc/xen-kbdfront.c has

int num_cont, width, height;
num_cont = xenbus_read_unsigned(info->xbdev->otherend,
XENKBD_FIELD_MT_NUM_CONTACTS,
1);
ret = input_mt_init_slots(mtouch, num_cont, INPUT_MT_DIRECT);

. Since these users might need to pass values beyond "unsigned char" range,
I think we should not limit to too small values like 64.

More worrisome thing is that several users are not handling
input_mt_init_slots() failures.

[Tetsuo Handa]

syzbot is reporting too large allocation at input_mt_init_slots(), for

num_slots is supplied from userspace using ioctl(UI_DEV_CREATE).

Since nobody knows possible max slots, this patch chose 1024.

Reported-by: syzbot <syzbot+0122fa...@syzkaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5
Suggested-by: Dmitry Torokhov <dmitry....@gmail.com>

Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
---

Changes in v2:
Limit max slots instead of using __GFP_NOWARN.

drivers/input/input-mt.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 14b53dac1253..6b04a674f832 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -46,6 +46,9 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
return 0;

if (mt)
return mt->num_slots != num_slots ? -EINVAL : 0;

+ /* Arbitrary limit for avoiding too large memory allocation. */
+ if (num_slots > 1024)
+ return -EINVAL;

mt = kzalloc(struct_size(mt, slots, num_slots), GFP_KERNEL);

if (!mt)
--
2.18.4