linux-kernel - [PATCH 3.10 128/180] tty/vt/keyboard: fix OOB access in do_compute

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: <1471793510-13022-129-git-send-email-w@1wt.eu>
Date:   Sun, 21 Aug 2016 17:30:58 +0200
From:   Willy Tarreau <w@....eu>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Willy Tarreau <w@....eu>
Subject: [PATCH 3.10 128/180] tty/vt/keyboard: fix OOB access in do_compute_shiftstate()

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

commit 510cccb5b0c8868a2b302a0ab524da7912da648b upstream.

The size of individual keymap in drivers/tty/vt/keyboard.c is NR_KEYS,
which is currently 256, whereas number of keys/buttons in input device (and
therefor in key_down) is much larger - KEY_CNT - 768, and that can cause
out-of-bound access when we do

	sym = U(key_maps[0][k]);

with large 'k'.

To fix it we should not attempt iterating beyond smaller of NR_KEYS and
KEY_CNT.

Also while at it let's switch to for_each_set_bit() instead of open-coding
it.

Reported-by: Sasha Levin <sasha.levin@...cle.com>
Reviewed-by: Guenter Roeck <linux@...ck-us.net>
Cc: stable@...r.kernel.org
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
Signed-off-by: Willy Tarreau <w@....eu>
---
 drivers/tty/vt/keyboard.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index a9af1b9a..1f6e096 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -371,34 +371,22 @@ static void to_utf8(struct vc_data *vc, uint c)
 
 static void do_compute_shiftstate(void)
 {
-	unsigned int i, j, k, sym, val;
+	unsigned int k, sym, val;
 
 	shift_state = 0;
 	memset(shift_down, 0, sizeof(shift_down));
 
-	for (i = 0; i < ARRAY_SIZE(key_down); i++) {
-
-		if (!key_down[i])
+	for_each_set_bit(k, key_down, min(NR_KEYS, KEY_CNT)) {
+		sym = U(key_maps[0][k]);
+		if (KTYP(sym) != KT_SHIFT && KTYP(sym) != KT_SLOCK)
 			continue;
 
-		k = i * BITS_PER_LONG;
-
-		for (j = 0; j < BITS_PER_LONG; j++, k++) {
-
-			if (!test_bit(k, key_down))
-				continue;
+		val = KVAL(sym);
+		if (val == KVAL(K_CAPSSHIFT))
+			val = KVAL(K_SHIFT);
 
-			sym = U(key_maps[0][k]);
-			if (KTYP(sym) != KT_SHIFT && KTYP(sym) != KT_SLOCK)
-				continue;
-
-			val = KVAL(sym);
-			if (val == KVAL(K_CAPSSHIFT))
-				val = KVAL(K_SHIFT);
-
-			shift_down[val]++;
-			shift_state |= (1 << val);
-		}
+		shift_down[val]++;
+		shift_state |= BIT(val);
 	}
 }
 
-- 
2.8.0.rc2.1.gbe9624a