Received-SPF: pass (google.com: domain of linux-kernel+bounces-170312-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Mon, 06 May 2024 18:55:19 +0000
Precedence: bulk
Mime-Version: 1.0
Message-ID: <20240506-b4-sio-scrollback-delta-v1-1-4164d162a2b8@google.com>
Subject: [PATCH] tty: vt: saturate scrollback_delta to avoid overflow
From: Justin Stitt <justinstitt@google.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Jiri Slaby <jirislaby@kernel.org>, 
	Nathan Chancellor <nathan@kernel.org>, Bill Wendling <morbo@google.com>
Cc: linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, 
	llvm@lists.linux.dev, Justin Stitt <justinstitt@google.com>
Content-Type: text/plain; charset="utf-8"

Using the signed overflow sanitizer with syzkaller produces this UBSAN
report:

[   31.304043] ------------[ cut here ]------------
[   31.304048] UBSAN: signed-integer-overflow in ../drivers/tty/vt/vt.c:309:19
[   31.304055] -2147483648 + -1073741824 cannot be represented in type 'int'
[   31.304066] CPU: 1 PID: 3894 Comm: syz-executor Not tainted 6.8.0-rc2-00035-gb3ef86b5a957 #1
[   31.304073] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   31.304077] Call Trace:
[   31.304080]  <TASK>
[   31.304083]  dump_stack_lvl+0x93/0xd0
[   31.304177]  handle_overflow+0x171/0x1b0
[   31.304186]  scrollfront+0xcb/0xd0
[   31.304196]  tioclinux+0x3cc/0x450
[   31.304205]  tty_ioctl+0x7fc/0xc00
[   31.304212]  ? __pfx_tty_ioctl+0x10/0x10
[   31.304219]  __se_sys_ioctl+0xe0/0x140
[   31.304228]  do_syscall_64+0xd7/0x1b0
[   31.304236]  ? arch_exit_to_user_mode_prepare+0x11/0x60
[   31.304244]  entry_SYSCALL_64_after_hwframe+0x6f/0x77
[   31.304254] RIP: 0033:0x7fc3902ae539
[   31.304263] Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 14 00 00 90 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 73 01 c3 48 c7 c1 8
[   31.304282] RSP: 002b:00007ffc8a457998 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[   31.304289] RAX: ffffffffffffffda RBX: 00007fc3903e2f80 RCX: 00007fc3902ae539
[   31.304293] RDX: 0000000020000040 RSI: 000000000000541c RDI: 0000000000000003
[   31.304297] RBP: 00007fc39030d496 R08: 0000000000000000 R09: 0000000000000000
[   31.304300] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[   31.304304] R13: 0000000000000800 R14: 00007fc3903e2f80 R15: 00007fc3903e2f80
[   31.304310]  </TASK>
[   31.304371] ---[ end trace ]---

This is caused by the scrollback_delta overflowing. Historically, the
signed integer overflow sanitizer did not work in the kernel due to its
interaction with `-fwrapv` but this has since been changed [1] in the
newest version of Clang; It being re-enabled in the kernel with Commit
557f8c582a9ba8ab ("ubsan: Reintroduce signed overflow sanitizer").

Note that it would be difficult to reproduce this bug in a non-fuzzing
scenario as it requires inputting tons of scroll inputs via keyboard
before the scheduled console callback has had a chance to update.
Nonetheless, let's saturate scrollback_delta so it stays clamped to
integer bounds without wrapping around.

[1]: https://github.com/llvm/llvm-project/pull/82432

Closes: https://github.com/KSPP/linux/issues/351
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: I am using Kees' SIO tree as a base:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=dev/v6.8-rc2/signed-overflow-sanitizer
---
 drivers/tty/vt/vt.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 9b5b98dfc8b4..b4768336868e 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -308,7 +308,14 @@ static inline void scrolldelta(int lines)
 	/* FIXME */
 	/* scrolldelta needs some kind of consistency lock, but the BKL was
 	   and still is not protecting versus the scheduled back end */
-	scrollback_delta += lines;
+
+	/* saturate scrollback_delta so that it never wraps around */
+	if (lines > 0 && unlikely(INT_MAX - lines < scrollback_delta))
+		scrollback_delta = INT_MAX;
+	else if (lines < 0 && unlikely(INT_MIN - lines > scrollback_delta))
+		scrollback_delta = INT_MIN;
+	else
+		scrollback_delta += lines;
 	schedule_console_callback();
 }
 

---
base-commit: 0106679839f7c69632b3b9833c3268c316c0a9fc
change-id: 20240506-b4-sio-scrollback-delta-dff9aabace26

Best regards,
--
Justin Stitt <justinstitt@google.com>