linux-kernel - Re: [PATCH (resend)] Input: MT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zqgi7NYEbpRsJfa2@google.com>
Date: Mon, 29 Jul 2024 16:17:00 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	Henrik Rydberg <rydberg@...math.org>,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH (resend)] Input: MT - limit max slots

On Mon, Jul 29, 2024 at 01:14:36PM -0700, Linus Torvalds wrote:
> On Mon, 29 Jul 2024 at 13:00, Dmitry Torokhov <dmitry.torokhov@...il.com> wrote:
> >
> > Hmm, maybe the checks should go into drivers/input/misc/uinput.c which
> > is the only place that allows userspace to create input device instances
> > and drive them rather than into input core logic because all other
> > devices are backed by real hardware.
> 
> Ack, that sounds like a good idea, particularly if there is some
> single location that could validate the input.
> 
> uinput_validate_absinfo(), perhaps?

Yes, I think that's the right place.

> 
> We do end up trying to protect against some forms of bad hardware too
> when possible, but realistically _that_ kind of protection should be
> more along the lines of "don't cause security issues".

Most of them have statically defined slot limit. The exception is HID
(which can also be fed garbage through uhid) but hid-multitouch uses u8
for maximum number of contacts, so can't go above 256.

Thanks.

-- 
Dmitry

---8<------------------8<------------------8<---------------

Input: uinput - reject requests with unreasonable number of slots

From: Dmitry Torokhov <dmitry.torokhov@...il.com>

When exercising uinput interface syzkaller may try setting up device
with a really large number of slots, which causes memory allocation
failure in input_mt_init_slots(). While this allocation failure is
handled properly and request is rejected, it results in syzkaller
reports. Additionally, such request may put undue burden on the
system which will try to free a lot of memory for a bogus request.

Fix it by limiting allowed number of slots to 100. This can easily
be extended if we see devices that can track more than 100 contacts.

Reported-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Reported-by: syzbot <syzbot+0122fa359a69694395d5@...kaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
---
 drivers/input/misc/uinput.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index d23f3225b00f..ea3d34e62eb2 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -417,6 +417,20 @@ static int uinput_validate_absinfo(struct input_dev *dev, unsigned int code,
 		return -EINVAL;
 	}
 
+	/*
+	 * Limit number of contacts to a reasonable value (100). This
+	 * ensures that we need less than 2 pages for struct input_mt
+	 * (we are not using in-kernel slot assignment so not going to
+	 * allocate memory for the "red" table), and we should have no
+	 * trouble getting this much memory.
+	 */
+	if (code == ABS_MT_SLOT && max > 99) {
+		printk(KERN_DEBUG
+		       "%s: unreasonably large number of slots requested: %d\n",
+		       UINPUT_NAME, max);
+		return -EINVAL;
+	}
+
 	return 0;
 }