Hello,
syzbot found the following crash on:
HEAD commit: 262f7a6b Merge tag 'for-5.7-rc3-tag' of git://git.kernel.o..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12786888100000
kernel config: https://syzkaller.appspot.com/x/.config?x=5b075813ec8b93cd
dashboard link: https://syzkaller.appspot.com/bug?extid=1f29e126cf461c4de3b3
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]
BUG: unable to handle page fault for address: ffff8880ffca0e80
#PF: supervisor write access in kernel mode
#PF: error_code(0x0002) - not-present page
PGD d401067 P4D d401067 PUD 0
Oops: 0002 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 710 Comm: syz-executor.5 Not tainted 5.7.0-rc3-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
RIP: 0010:vga_imageblit_expand drivers/video/fbdev/vga16fb.c:1168 [inline]
RIP: 0010:vga16fb_imageblit+0xa5b/0x2210 drivers/video/fbdev/vga16fb.c:1260
Code: 48 89 fa 48 c1 ea 03 0f b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 09 84 d2 74 05 e8 ee 59 ed fd 41 8b 47 14 48 8b 74 24 08 <88> 06 0f ae e8 8a 06 b8 05 00 00 00 ba ce 03 00 00 ee 48 c7 c2 18
RSP: 0000:ffffc90002ea71f0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000007 RCX: ffffc90014391000
RDX: 0000000000000000 RSI: ffff8880ffca0e80 RDI: ffffc90002ea739c
RBP: ffffc90002ea738c R08: ffff8880922ac200 R09: 0000000000000000
R10: ffffffff8a895007 R11: fffffbfff1512a00 R12: 0000000000000000
R13: ffff888218de5140 R14: 0000000000000001 R15: ffffc90002ea7388
FS: 00007fbeeb282700(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff8880ffca0e80 CR3: 000000008e9c5000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
bit_putcs_unaligned drivers/video/fbdev/core/bitblit.c:139 [inline]
bit_putcs+0x910/0xe10 drivers/video/fbdev/core/bitblit.c:188
fbcon_putcs+0x345/0x3f0 drivers/video/fbdev/core/fbcon.c:1362
con_flush drivers/tty/vt/vt.c:2569 [inline]
do_con_write.part.0+0x7d1/0x1dc0 drivers/tty/vt/vt.c:2772
do_con_write drivers/tty/vt/vt.c:2588 [inline]
con_write+0x41/0xe0 drivers/tty/vt/vt.c:3154
process_output_block drivers/tty/n_tty.c:595 [inline]
n_tty_write+0x3f0/0xf90 drivers/tty/n_tty.c:2333
do_tty_write drivers/tty/tty_io.c:962 [inline]
tty_write+0x495/0x800 drivers/tty/tty_io.c:1046
__vfs_write+0x76/0x100 fs/read_write.c:495
__kernel_write+0x11c/0x3a0 fs/read_write.c:516
write_pipe_buf+0x153/0x1e0 fs/splice.c:809
splice_from_pipe_feed fs/splice.c:512 [inline]
__splice_from_pipe+0x3e6/0x7b0 fs/splice.c:636
splice_from_pipe+0xd9/0x140 fs/splice.c:671
default_file_splice_write+0x37/0x90 fs/splice.c:821
do_splice_from fs/splice.c:863 [inline]
direct_splice_actor+0x115/0x160 fs/splice.c:1037
splice_direct_to_actor+0x38c/0x980 fs/splice.c:992
do_splice_direct+0x1b4/0x280 fs/splice.c:1080
do_sendfile+0x555/0xc50 fs/read_write.c:1521
__do_sys_sendfile64 fs/read_write.c:1582 [inline]
__se_sys_sendfile64 fs/read_write.c:1568 [inline]
__x64_sys_sendfile64+0x1cc/0x210 fs/read_write.c:1568
do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295
entry_SYSCALL_64_after_hwframe+0x49/0xb3
RIP: 0033:0x45c829
Code: 0d b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 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 0f 83 db b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fbeeb281c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000028
RAX: ffffffffffffffda RBX: 00000000004fc0c0 RCX: 000000000045c829
RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000003
RBP: 000000000078bf00 R08: 0000000000000000 R09: 0000000000000000
R10: 0800000080004103 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000000008d6 R14: 00000000004cb7a1 R15: 00007fbeeb2826d4
Modules linked in:
CR2: ffff8880ffca0e80
---[ end trace 5bb103c4fc7bf525 ]---
RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
RIP: 0010:vga_imageblit_expand drivers/video/fbdev/vga16fb.c:1168 [inline]
RIP: 0010:vga16fb_imageblit+0xa5b/0x2210 drivers/video/fbdev/vga16fb.c:1260
Code: 48 89 fa 48 c1 ea 03 0f b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 09 84 d2 74 05 e8 ee 59 ed fd 41 8b 47 14 48 8b 74 24 08 <88> 06 0f ae e8 8a 06 b8 05 00 00 00 ba ce 03 00 00 ee 48 c7 c2 18
RSP: 0000:ffffc90002ea71f0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000007 RCX: ffffc90014391000
RDX: 0000000000000000 RSI: ffff8880ffca0e80 RDI: ffffc90002ea739c
RBP: ffffc90002ea738c R08: ffff8880922ac200 R09: 0000000000000000
R10: ffffffff8a895007 R11: fffffbfff1512a00 R12: 0000000000000000
R13: ffff888218de5140 R14: 0000000000000001 R15: ffffc90002ea7388
FS: 00007fbeeb282700(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff8880ffca0e80 CR3: 000000008e9c5000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
This bug 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 [email protected].
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot has found a reproducer for the following issue on:
HEAD commit: d2b6f8a1 Merge tag 'xfs-5.13-merge-3' of git://git.kernel...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11d80be1d00000
kernel config: https://syzkaller.appspot.com/x/.config?x=53fdf14defd48c56
dashboard link: https://syzkaller.appspot.com/bug?extid=1f29e126cf461c4de3b3
compiler: Debian clang version 11.0.1-2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16d9ff43d00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10981693d00000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]
BUG: unable to handle page fault for address: ffff888001000040
#PF: supervisor write access in kernel mode
#PF: error_code(0x0003) - permissions violation
PGD 11201067 P4D 11201067 PUD 11202067 PMD 80000000010001e1
Oops: 0003 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 8403 Comm: syz-executor112 Not tainted 5.12.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
RIP: 0010:vga_imageblit_expand drivers/video/fbdev/vga16fb.c:1176 [inline]
RIP: 0010:vga16fb_imageblit+0xcee/0x1cb0 drivers/video/fbdev/vga16fb.c:1260
Code: 66 66 2e 0f 1f 84 00 00 00 00 00 90 4c 89 e0 48 c1 e8 03 48 b9 00 00 00 00 00 fc ff df 0f b6 04 08 84 c0 75 1b 41 0f b6 04 24 <41> 88 06 85 ed 74 2b 49 ff c4 49 ff c6 e8 80 ae 43 fd ff cd eb cc
RSP: 0018:ffffc9000163f0a0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff888001000040 RCX: dffffc0000000000
RDX: ffff888022ad54c0 RSI: 0000000000000002 RDI: 0000000000000000
RBP: 0000000000000001 R08: ffffffff843b289b R09: 0000000000000000
R10: 0000000000000002 R11: ffff888022ad54c0 R12: ffff8880181bcea8
R13: ffffc9000163f2cc R14: ffff888001000040 R15: 0000000000000004
FS: 0000000001207300(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff888001000040 CR3: 0000000028d32000 CR4: 00000000001506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
bit_putcs+0x18e8/0x1de0 drivers/video/fbdev/core/bitblit.c:105
fbcon_putcs+0x2ae/0x430 drivers/video/fbdev/core/fbcon.c:1296
do_update_region+0x4d6/0x6a0 drivers/tty/vt/vt.c:676
redraw_screen+0x8f6/0x1270 drivers/tty/vt/vt.c:1035
fbcon_blank+0x556/0xa50 drivers/video/fbdev/core/fbcon.c:2207
do_unblank_screen+0x27e/0xb20 drivers/tty/vt/vt.c:4406
vt_kdsetmode drivers/tty/vt/vt_ioctl.c:276 [inline]
vt_k_ioctl drivers/tty/vt/vt_ioctl.c:381 [inline]
vt_ioctl+0x2a82/0x3180 drivers/tty/vt/vt_ioctl.c:713
tty_ioctl+0xf51/0x1720 drivers/tty/tty_io.c:2805
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:1069 [inline]
__se_sys_ioctl+0xfb/0x170 fs/ioctl.c:1055
do_syscall_64+0x3f/0xb0 arch/x86/entry/common.c:47
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x43fef9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 b1 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 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc931a4c48 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000013f84 RCX: 000000000043fef9
RDX: 0000000000000000 RSI: 0000000000004b3a RDI: 0000000000000003
RBP: 0000000000000000 R08: 000000000000000d R09: 00007ffc931a4de8
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc931a4c5c
R13: 431bde82d7b634db R14: 00000000004ae018 R15: 0000000000400488
Modules linked in:
CR2: ffff888001000040
---[ end trace 96734cf7ef5cce91 ]---
RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
RIP: 0010:vga_imageblit_expand drivers/video/fbdev/vga16fb.c:1176 [inline]
RIP: 0010:vga16fb_imageblit+0xcee/0x1cb0 drivers/video/fbdev/vga16fb.c:1260
Code: 66 66 2e 0f 1f 84 00 00 00 00 00 90 4c 89 e0 48 c1 e8 03 48 b9 00 00 00 00 00 fc ff df 0f b6 04 08 84 c0 75 1b 41 0f b6 04 24 <41> 88 06 85 ed 74 2b 49 ff c4 49 ff c6 e8 80 ae 43 fd ff cd eb cc
RSP: 0018:ffffc9000163f0a0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff888001000040 RCX: dffffc0000000000
RDX: ffff888022ad54c0 RSI: 0000000000000002 RDI: 0000000000000000
RBP: 0000000000000001 R08: ffffffff843b289b R09: 0000000000000000
R10: 0000000000000002 R11: ffff888022ad54c0 R12: ffff8880181bcea8
R13: ffffc9000163f2cc R14: ffff888001000040 R15: 0000000000000004
FS: 0000000001207300(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff888001000040 CR3: 0000000028d32000 CR4: 00000000001506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
syzbot has bisected this issue to:
commit 988d0763361bb65690d60e2bc53a6b72777040c3
Author: Tetsuo Handa <[email protected]>
Date: Sun Sep 27 11:46:30 2020 +0000
vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=15633759d00000
start commit: d2b6f8a1 Merge tag 'xfs-5.13-merge-3' of git://git.kernel...
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=17633759d00000
console output: https://syzkaller.appspot.com/x/log.txt?x=13633759d00000
kernel config: https://syzkaller.appspot.com/x/.config?x=53fdf14defd48c56
dashboard link: https://syzkaller.appspot.com/bug?extid=1f29e126cf461c4de3b3
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16d9ff43d00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10981693d00000
Reported-by: [email protected]
Fixes: 988d0763361b ("vt_ioctl: make VT_RESIZEX behave like VT_RESIZE")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
On 2021/05/02 10:53, syzbot wrote:
> syzbot has bisected this issue to:
>
> commit 988d0763361bb65690d60e2bc53a6b72777040c3
> Author: Tetsuo Handa <[email protected]>
> Date: Sun Sep 27 11:46:30 2020 +0000
>
> vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
>
That commit is irrelevant. Below is a simplified reproducer.
----------
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <linux/kd.h>
#include <linux/vt.h>
int main(int argc, char *argv[])
{
const int fd = open("/dev/char/4:1", O_RDWR);
struct vt_sizes vt = { 0x4100, 2 };
ioctl(fd, KDSETMODE, KD_GRAPHICS);
ioctl(fd, VT_RESIZE, &vt);
ioctl(fd, KDSETMODE, KD_TEXT);
return 0;
}
----------
In vga16fb_probe(), we have
----------
/* XXX share VGA_FB_PHYS and I/O region with vgacon and others */
info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS, 0);
if (!info->screen_base) {
printk(KERN_ERR "vga16fb: unable to map device\n");
ret = -ENOMEM;
goto err_ioremap;
}
printk(KERN_INFO "vga16fb: mapped to 0x%p\n", info->screen_base);
----------
and ffff8880000a0000 is assigned for 80 x 30 screen upon boot.
----------
[ 4.584361][ T1] vga16fb: mapped to 0xffff8880000a0000
[ 6.137556][ T1] Console: switching to colour frame buffer device 80x30
[ 7.829276][ T1] fb0: VGA16 VGA frame buffer device
----------
With debug printk() patch shown below,
----------
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 01645e87b3d5..af860b12db44 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -641,6 +641,8 @@ static void con_scroll(struct vc_data *vc, unsigned int t, unsigned int b,
scr_memsetw(clear, vc->vc_video_erase_char, vc->vc_size_row * nr);
}
+extern struct task_struct *trace_me;
+
static void do_update_region(struct vc_data *vc, unsigned long start, int count)
{
unsigned int xx, yy, offset;
@@ -656,6 +658,8 @@ static void do_update_region(struct vc_data *vc, unsigned long start, int count)
start = vc->vc_sw->con_getxy(vc, start, &nxx, &nyy);
xx = nxx; yy = nyy;
}
+ if (trace_me == current)
+ pr_info("p=%px vc->{ vc_origin=%lx vc_rows=%u vc_cols=%u vc_scr_end=%lx } start=%lx count=%d xx=%u yy=%u\n", p, vc->vc_origin, vc->vc_rows, vc->vc_cols, vc->vc_scr_end, start, count, xx, yy);
for(;;) {
u16 attrib = scr_readw(p) & 0xff00;
int startx = xx;
@@ -1227,6 +1231,8 @@ static int vc_do_resize(struct tty_struct *tty, struct vc_data *vc,
newscreen = kzalloc(new_screen_size, GFP_USER);
if (!newscreen)
return -ENOMEM;
+ if (trace_me == current)
+ pr_info("vc=%px new_cols=%u new_rows=%u new_screen_size=%u newscreen=%px\n", vc, new_cols, new_rows, new_screen_size, newscreen);
if (get_vc_uniscr(vc)) {
new_uniscr = vc_uniscr_alloc(new_cols, new_rows);
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 89aeaf3c1bca..137befd09d22 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -240,6 +240,8 @@ int vt_waitactive(int n)
#define GPLAST 0x3df
#define GPNUM (GPLAST - GPFIRST + 1)
+struct task_struct *trace_me;
+
/*
* currently, setting the mode from KD_TEXT to KD_GRAPHICS doesn't do a whole
* lot. i'm not sure if it should do any restoration of modes or what...
@@ -272,10 +274,12 @@ static int vt_kdsetmode(struct vc_data *vc, unsigned long mode)
/* explicitly blank/unblank the screen if switching modes */
console_lock();
+ trace_me = current;
if (mode == KD_TEXT)
do_unblank_screen(1);
else
do_blank_screen(1);
+ trace_me = NULL;
console_unlock();
return 0;
@@ -877,6 +881,7 @@ int vt_ioctl(struct tty_struct *tty,
return -EFAULT;
console_lock();
+ trace_me = current;
for (i = 0; i < MAX_NR_CONSOLES; i++) {
vc = vc_cons[i].d;
@@ -886,6 +891,7 @@ int vt_ioctl(struct tty_struct *tty,
vc_resize(vc_cons[i].d, cc, ll);
}
}
+ trace_me = NULL;
console_unlock();
break;
}
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index 1e8a38a7967d..fca76f6b9ab6 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -1145,6 +1145,8 @@ static void vga_8planes_imageblit(struct fb_info *info, const struct fb_image *i
setindex(oldindex);
}
+extern struct task_struct *trace_me;
+
static void vga_imageblit_expand(struct fb_info *info, const struct fb_image *image)
{
char __iomem *where = info->screen_base + (image->dx/8) +
@@ -1170,6 +1172,9 @@ static void vga_imageblit_expand(struct fb_info *info, const struct fb_image *im
readb(where); /* fill latches */
setmode(3);
wmb();
+ if (trace_me == current)
+ pr_info_ratelimited("image->{ data=%px dx=%u dy=%u height=%u width=%u } info->{ screen_base=%px fix.line_length=%u } where=%px\n",
+ cdat, image->dx, image->dy, image->height, image->width, info->screen_base, info->fix.line_length, where);
for (y = 0; y < image->height; y++) {
dst = where;
for (x = image->width/8; x--;)
----------
we can see that at least ffff8880000a0000-ffff888001000040 are accessed, and 'dy'
would be 201520 when 'where' is ffff888001000000, which corresponds with 'real_y()'
being 12595, for ioctl(VT_RESIZE) changed screen size to 2 x 16640.
----------
[ 222.885841][ T1675] vc=ffff888100109800 new_cols=2 new_rows=16640 new_screen_size=66560 newscreen=ffff888112120000
[ 222.886520][ T1675] vc=ffff888105d26800 new_cols=2 new_rows=16640 new_screen_size=66560 newscreen=ffff888112140000
[ 222.886662][ T1675] vc=ffff888105ca5800 new_cols=2 new_rows=16640 new_screen_size=66560 newscreen=ffff888112140000
[ 222.886799][ T1675] vc=ffff8881017ff800 new_cols=2 new_rows=16640 new_screen_size=66560 newscreen=ffff888112140000
[ 222.886951][ T1675] vc=ffff888102770800 new_cols=2 new_rows=16640 new_screen_size=66560 newscreen=ffff888112140000
[ 222.887089][ T1675] vc=ffff8881052e5800 new_cols=2 new_rows=16640 new_screen_size=66560 newscreen=ffff888112140000
[ 222.956019][ T1675] image->{ data=ffff888100a6e180 dx=8 dy=48 height=16 width=8 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a0f01
[ 223.252930][ T1675] p=ffff888112120000 vc->{ vc_origin=ffff888112120000 vc_rows=16640 vc_cols=2 vc_scr_end=ffff888112130400 } start=ffff888112120004 count=33280 xx=0 yy=0
[ 223.253187][ T1675] image->{ data=ffff888100a6e190 dx=0 dy=0 height=16 width=16 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a0000
[ 223.253777][ T1675] image->{ data=ffff888100a6e1b0 dx=0 dy=16 height=16 width=16 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a0500
[ 223.254373][ T1675] image->{ data=ffff888100a6e1d0 dx=0 dy=32 height=16 width=16 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a0a00
[ 223.254867][ T1675] image->{ data=ffff888100a6e1f0 dx=0 dy=48 height=16 width=16 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a0f00
[ 223.255340][ T1675] image->{ data=ffff888100a6e210 dx=0 dy=64 height=16 width=16 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a1400
[ 223.255834][ T1675] image->{ data=ffff888100a6e230 dx=0 dy=80 height=16 width=16 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a1900
[ 223.256307][ T1675] image->{ data=ffff888100a6e250 dx=0 dy=96 height=16 width=16 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a1e00
[ 223.256800][ T1675] image->{ data=ffff888100a6e270 dx=0 dy=112 height=16 width=16 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a2300
[ 223.257274][ T1675] image->{ data=ffff888100a6e290 dx=0 dy=128 height=16 width=16 } info->{ screen_base=ffff8880000a0000 fix.line_length=80 } where=ffff8880000a2800
[ 224.897239][ T1675] BUG: unable to handle page fault for address: ffff888001000040
[ 224.897276][ T1675] #PF: supervisor write access in kernel mode
[ 224.897316][ T1675] #PF: error_code(0x0003) - permissions violation
[ 224.897354][ T1675] PGD 3c01067 P4D 3c01067 PUD 3c02067 PMD 80000000010001e1
[ 224.897454][ T1675] Oops: 0003 [#1] PREEMPT SMP
[ 224.897516][ T1675] CPU: 3 PID: 1675 Comm: a.out Not tainted 5.12.0+ #652
[ 224.897591][ T1675] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 224.897632][ T1675] RIP: 0010:vga16fb_imageblit+0x4b1/0x9d0
[ 224.897740][ T1675] Code: f6 74 47 45 89 fe 48 89 1c 24 4d 89 ef e8 b7 a0 9d ff 48 8b 04 24 49 83 c7 01 48 89 c2 48 83 c0 01 48 89 04 24 41 0f b6 47 ff <88> 02 31 ff 44 89 f6 41 83 ee 01 e8 ef a1 9d ff 41 83 fe ff 75 cd
[ 224.897820][ T1675] RSP: 0018:ffffc900010d3a30 EFLAGS: 00010286
[ 224.897879][ T1675] RAX: 0000000000000000 RBX: ffff888001000040 RCX: ffff888103a40100
[ 224.897972][ T1675] RDX: ffff888001000040 RSI: ffff888103a40100 RDI: 0000000000000002
[ 224.898027][ T1675] RBP: ffffc900010d3af8 R08: ffffffff8182e1b9 R09: 0000000000000000
[ 224.898083][ T1675] R10: 0000000000000005 R11: 0000000000080000 R12: ffff888101f36800
[ 224.898137][ T1675] R13: ffff888100a6ebc8 R14: 0000000000000001 R15: ffff888100a6ebc9
[ 224.898194][ T1675] FS: 00007fbefdeb6540(0000) GS:ffff88811bd80000(0000) knlGS:0000000000000000
[ 224.898269][ T1675] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 224.898328][ T1675] CR2: ffff888001000040 CR3: 0000000103a56003 CR4: 00000000000706e0
[ 224.898387][ T1675] Call Trace:
[ 224.898405][ T1675] bit_putcs+0x4dd/0x700
[ 224.898493][ T1675] ? write_comp_data+0x1c/0x70
[ 224.898568][ T1675] ? __sanitizer_cov_trace_switch+0x50/0x90
[ 224.898655][ T1675] ? bit_clear+0x1e0/0x1e0
[ 224.898742][ T1675] fbcon_putcs+0x13c/0x150
[ 224.898823][ T1675] do_update_region+0x1c6/0x2b0
[ 224.898935][ T1675] redraw_screen+0x2e4/0x310
[ 224.899051][ T1675] fbcon_blank+0x38f/0x440
[ 224.899139][ T1675] do_unblank_screen+0x10f/0x210
[ 224.899240][ T1675] vt_ioctl+0x116f/0x19c0
[ 224.899325][ T1675] ? lock_is_held_type+0xfc/0x170
[ 224.899413][ T1675] ? write_comp_data+0x1c/0x70
[ 224.899488][ T1675] ? __sanitizer_cov_trace_switch+0x50/0x90
[ 224.899576][ T1675] ? complete_change_console+0x160/0x160
[ 224.899661][ T1675] tty_ioctl+0x630/0xbb0
[ 224.899731][ T1675] ? __sanitizer_cov_trace_pc+0x1a/0x40
[ 224.899813][ T1675] ? do_vfs_ioctl+0x9b/0xca0
[ 224.899904][ T1675] ? lock_is_held_type+0xfc/0x170
[ 224.899982][ T1675] ? tty_vhangup+0x30/0x30
[ 224.900053][ T1675] __x64_sys_ioctl+0xbb/0x110
[ 224.900134][ T1675] do_syscall_64+0x3a/0xb0
[ 224.900229][ T1675] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 224.900326][ T1675] RIP: 0033:0x7fbefddda50b
[ 224.900379][ T1675] Code: 0f 1e fa 48 8b 05 85 39 0d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 55 39 0d 00 f7 d8 64 89 01 48
[ 224.900459][ T1675] RSP: 002b:00007ffe60c4e2b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 224.900532][ T1675] RAX: ffffffffffffffda RBX: 0000557f9f780220 RCX: 00007fbefddda50b
[ 224.900589][ T1675] RDX: 0000000000000000 RSI: 0000000000004b3a RDI: 0000000000000003
[ 224.900641][ T1675] RBP: 0000000000000003 R08: 0000000000000000 R09: 00007fbefded0d50
[ 224.900694][ T1675] R10: 0000000000000000 R11: 0000000000000246 R12: 0000557f9f780130
[ 224.900748][ T1675] R13: 00007ffe60c4e3c0 R14: 0000000000000000 R15: 0000000000000000
[ 224.900804][ T1675] Modules linked in: sg video rapl evdev backlight input_leds mousedev led_class button ac binfmt_misc sd_mod t10_pi crc_t10dif crct10dif_generic sr_mod cdrom crct10dif_pclmul ata_generic crct10dif_common crc32_pclmul crc32c_intel ahci ghash_clmulni_intel psmouse libahci aesni_intel atkbd ata_piix libaes libps2 crypto_simd i2c_piix4 libata i8042 rtc_cmos i2c_core cryptd serio scsi_mod
[ 224.901745][ T1675] CR2: ffff888001000040
[ 224.901777][ T1675] ---[ end trace 045541aa43f10c56 ]---
[ 224.901841][ T1675] RIP: 0010:vga16fb_imageblit+0x4b1/0x9d0
[ 224.901948][ T1675] Code: f6 74 47 45 89 fe 48 89 1c 24 4d 89 ef e8 b7 a0 9d ff 48 8b 04 24 49 83 c7 01 48 89 c2 48 83 c0 01 48 89 04 24 41 0f b6 47 ff <88> 02 31 ff 44 89 f6 41 83 ee 01 e8 ef a1 9d ff 41 83 fe ff 75 cd
[ 224.902062][ T1675] RSP: 0018:ffffc900010d3a30 EFLAGS: 00010286
[ 224.902181][ T1675] RAX: 0000000000000000 RBX: ffff888001000040 RCX: ffff888103a40100
[ 224.902245][ T1675] RDX: ffff888001000040 RSI: ffff888103a40100 RDI: 0000000000000002
[ 224.902347][ T1675] RBP: ffffc900010d3af8 R08: ffffffff8182e1b9 R09: 0000000000000000
[ 224.902461][ T1675] R10: 0000000000000005 R11: 0000000000080000 R12: ffff888101f36800
[ 224.902524][ T1675] R13: ffff888100a6ebc8 R14: 0000000000000001 R15: ffff888100a6ebc9
[ 224.902580][ T1675] FS: 00007fbefdeb6540(0000) GS:ffff88811bd80000(0000) knlGS:0000000000000000
[ 224.902655][ T1675] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 224.902714][ T1675] CR2: ffff888001000040 CR3: 0000000103a56003 CR4: 00000000000706e0
[ 224.902777][ T1675] Kernel panic - not syncing: Fatal exception
[ 224.903014][ T1675] Kernel Offset: disabled
[ 225.661115][ T1675] Rebooting in 10 seconds..
----------
Therefore, I guess that the problem is that fbcon_putcs() from do_update_region() from
redraw_screen() from vt_kdsetmode(KD_TEXT) from ioctl(fd, KDSETMODE, KD_TEXT) tries to
redraw 2 x 16640 despite memory amount allocated for actual screen is only 80 x 30.
I don't know how to fix this problem...
On 2021/05/03 22:41, Tetsuo Handa wrote:
> Therefore, I guess that the problem is that fbcon_putcs() from do_update_region() from
> redraw_screen() from vt_kdsetmode(KD_TEXT) from ioctl(fd, KDSETMODE, KD_TEXT) tries to
> redraw 2 x 16640 despite memory amount allocated for actual screen is only 80 x 30.
>
> I don't know how to fix this problem...
>
Daniel Vetter suggested me that parameter validation is missing/wrong somewhere, for
resize requests that don't fit should be rejected. Thus, I'm thinking how to add
parameter validation.
Like a diff shown bottom, adding a hook for validating whether rows / columns are
small enough (VGA_FB_PHYS_LEN bytes starting from VGA_FB_PHYS ?) survives the
----------
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <linux/kd.h>
#include <linux/vt.h>
int main(int argc, char *argv[])
{
const int fd = open("/dev/char/4:1", O_RDWR);
struct vt_sizes vt = { 0x4100, 2 };
ioctl(fd, KDSETMODE, KD_GRAPHICS);
ioctl(fd, VT_RESIZE, &vt);
ioctl(fd, KDSETMODE, KD_TEXT);
return 0;
}
----------
reproducer. But I don't know how to calculate upper boundary values for vga16fb_tty_resize(),
for I even don't know where these values are derived from...
Also, currently resize_screen() calls vc->vc_sw->con_resize() only if vc->vc_mode != KD_GRAPHICS.
But we need to unconditionally call vga16fb_tty_resize() in order to survive the reproducer; we
need to prevent vc->vc_{rows,cols} from exceeding the upper boundary values regardless of current
vc->vc_mode setting. The "vc->vc_mode != KD_GRAPHICS" check predates the git history, and I don't
know the side effect of removing this check...
----------
e400b6ec4ede4 drivers/char/vt.c (Antonino A. Daplas 2007-10-16 01:29:35 -0700 1168) static inline int resize_screen(struct vc_data *vc, int width, int height,
e400b6ec4ede4 drivers/char/vt.c (Antonino A. Daplas 2007-10-16 01:29:35 -0700 1169) int user)
^1da177e4c3f4 drivers/char/vt.c (Linus Torvalds 2005-04-16 15:20:36 -0700 1170) {
^1da177e4c3f4 drivers/char/vt.c (Linus Torvalds 2005-04-16 15:20:36 -0700 1171) /* Resizes the resolution of the display adapater */
^1da177e4c3f4 drivers/char/vt.c (Linus Torvalds 2005-04-16 15:20:36 -0700 1172) int err = 0;
^1da177e4c3f4 drivers/char/vt.c (Linus Torvalds 2005-04-16 15:20:36 -0700 1173)
^1da177e4c3f4 drivers/char/vt.c (Linus Torvalds 2005-04-16 15:20:36 -0700 1174) if (vc->vc_mode != KD_GRAPHICS && vc->vc_sw->con_resize)
e400b6ec4ede4 drivers/char/vt.c (Antonino A. Daplas 2007-10-16 01:29:35 -0700 1175) err = vc->vc_sw->con_resize(vc, width, height, user);
e400b6ec4ede4 drivers/char/vt.c (Antonino A. Daplas 2007-10-16 01:29:35 -0700 1176)
^1da177e4c3f4 drivers/char/vt.c (Linus Torvalds 2005-04-16 15:20:36 -0700 1177) return err;
^1da177e4c3f4 drivers/char/vt.c (Linus Torvalds 2005-04-16 15:20:36 -0700 1178) }
----------
I need more help from those who know this area.
drivers/tty/vt/vt.c | 2 +-
drivers/video/fbdev/core/fbcon.c | 7 +++++++
drivers/video/fbdev/vga16fb.c | 8 ++++++++
include/linux/fb.h | 3 +++
4 files changed, 19 insertions(+), 1 deletion(-)
----------
diff --git a/include/linux/fb.h b/include/linux/fb.h
index a8dccd23c249..870384afb5e9 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -289,14 +289,17 @@ struct fb_ops {
/* teardown any resources to do with this framebuffer */
void (*fb_destroy)(struct fb_info *info);
/* called at KDB enter and leave time to prepare the console */
int (*fb_debug_enter)(struct fb_info *info);
int (*fb_debug_leave)(struct fb_info *info);
+
+ /* Check if resizing TTY to these sizes is safe. */
+ int (*fb_tty_resize)(struct fb_info *info, unsigned int cols, unsigned int rows);
};
#ifdef CONFIG_FB_TILEBLITTING
#define FB_TILE_CURSOR_NONE 0
#define FB_TILE_CURSOR_UNDERLINE 1
#define FB_TILE_CURSOR_LOWER_THIRD 2
#define FB_TILE_CURSOR_LOWER_HALF 3
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index 1e8a38a7967d..54aa6a99b09c 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -1266,27 +1266,35 @@ static void vga16fb_destroy(struct fb_info *info)
{
iounmap(info->screen_base);
fb_dealloc_cmap(&info->cmap);
/* XXX unshare VGA regions */
framebuffer_release(info);
}
+static int vga16fb_tty_resize(struct fb_info *info, unsigned int cols, unsigned int rows)
+{
+ if (cols <= 80 && rows <= 30)
+ return 0;
+ return -EINVAL;
+}
+
static const struct fb_ops vga16fb_ops = {
.owner = THIS_MODULE,
.fb_open = vga16fb_open,
.fb_release = vga16fb_release,
.fb_destroy = vga16fb_destroy,
.fb_check_var = vga16fb_check_var,
.fb_set_par = vga16fb_set_par,
.fb_setcolreg = vga16fb_setcolreg,
.fb_pan_display = vga16fb_pan_display,
.fb_blank = vga16fb_blank,
.fb_fillrect = vga16fb_fillrect,
.fb_copyarea = vga16fb_copyarea,
.fb_imageblit = vga16fb_imageblit,
+ .fb_tty_resize = vga16fb_tty_resize,
};
#ifndef MODULE
static int __init vga16fb_setup(char *options)
{
char *this_opt;
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 3406067985b1..c0eac87a2a56 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1990,14 +1990,21 @@ static int fbcon_resize(struct vc_data *vc, unsigned int width,
if (pitch <= 0)
return -EINVAL;
size = CALC_FONTSZ(vc->vc_font.height, pitch, vc->vc_font.charcount);
if (size > FNTSIZE(vc->vc_font.data))
return -EINVAL;
}
+ if (info->fbops && info->fbops->fb_tty_resize) {
+ int err = info->fbops->fb_tty_resize(info, width, height);
+
+ if (err)
+ return err;
+ }
+
virt_w = FBCON_SWAP(ops->rotate, width, height);
virt_h = FBCON_SWAP(ops->rotate, height, width);
virt_fw = FBCON_SWAP(ops->rotate, vc->vc_font.width,
vc->vc_font.height);
virt_fh = FBCON_SWAP(ops->rotate, vc->vc_font.height,
vc->vc_font.width);
var.xres = virt_w * virt_fw;
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 01645e87b3d5..fa1548d4f94b 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1167,15 +1167,15 @@ int vc_allocate(unsigned int currcons) /* return 0 on success */
static inline int resize_screen(struct vc_data *vc, int width, int height,
int user)
{
/* Resizes the resolution of the display adapater */
int err = 0;
- if (vc->vc_mode != KD_GRAPHICS && vc->vc_sw->con_resize)
+ if (vc->vc_sw->con_resize)
err = vc->vc_sw->con_resize(vc, width, height, user);
return err;
}
/**
* vc_do_resize - resizing method for the tty
----------
syzbot is reporting that a local user with the framebuffer console can
crash the kernel [1], for ioctl(VT_RESIZE) allows a TTY to set arbitrary
rows/columns values regardless of amount of memory reserved for
the graphical screen.
----------
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <linux/kd.h>
#include <linux/vt.h>
int main(int argc, char *argv[])
{
const int fd = open("/dev/char/4:1", O_RDWR);
struct vt_sizes vt = { 0x4100, 2 };
ioctl(fd, KDSETMODE, KD_GRAPHICS);
ioctl(fd, VT_RESIZE, &vt);
ioctl(fd, KDSETMODE, KD_TEXT);
return 0;
}
----------
Currently it is impossible to control upper limit of rows/columns values
based on amount of memory reserved for the graphical screen, for
resize_screen() calls vc->vc_sw->con_resize() only if vc->vc_mode is not
already KD_GRAPHICS. I don't know the reason, and this condition predates
the git history. Even if it turns out to be safe to always call this
callback, we will need to involve another callback via "struct fb_ops" for
checking the upper limits from fbcon_resize(). As a result, we will need
to modify
drivers/tty/vt/vt.c
drivers/video/fbdev/core/fbcon.c
drivers/video/fbdev/vga16fb.c
include/linux/fb.h
files only for checking rows/columns values passed to ioctl(VT_RESIZE)
request.
Therefore, instead of introducing such a complicated callback chain, avoid
this problem by simply checking whether the address to read or write is in
[VGA_FB_PHYS, VGA_FB_PHYS + VGA_FB_PHYS_LEN) range.
[1] https://syzkaller.appspot.com/bug?extid=1f29e126cf461c4de3b3
Reported-by: syzbot <[email protected]>
Signed-off-by: Tetsuo Handa <[email protected]>
Tested-by: syzbot <[email protected]>
---
drivers/video/fbdev/vga16fb.c | 54 +++++++++++++++++++++++------------
1 file changed, 36 insertions(+), 18 deletions(-)
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index e2757ff1c23d..13732a3b1d69 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -98,6 +98,18 @@ static const struct fb_fix_screeninfo vga16fb_fix = {
.accel = FB_ACCEL_NONE
};
+/*
+ * Verify that the address to read or write is in [VGA_FB_PHYS, VGA_FB_PHYS + VGA_FB_PHYS_LEN)
+ * range, for ioctl(VT_RESIZE) allows a TTY to set arbitrary rows/columns values which will crash
+ * the kernel due to out of bounds access when trying to redraw the screen.
+ */
+static inline bool is_valid_iomem(const struct fb_info *info, const char __iomem *where)
+{
+ return info->screen_base <= where && where < info->screen_base + VGA_FB_PHYS_LEN;
+}
+
+#define IS_SAFE(where) is_valid_iomem(info, (where))
+
/* The VGA's weird architecture often requires that we read a byte and
write a byte to the same location. It doesn't matter *what* byte
we write, however. This is because all the action goes on behind
@@ -851,7 +863,7 @@ static void vga_8planes_fillrect(struct fb_info *info, const struct fb_fillrect
int x;
/* we can do memset... */
- for (x = width; x > 0; --x) {
+ for (x = width; x > 0 && IS_SAFE(where); --x) {
writeb(rect->color, where);
where++;
}
@@ -864,7 +876,7 @@ static void vga_8planes_fillrect(struct fb_info *info, const struct fb_fillrect
oldop = setop(0x18);
oldsr = setsr(0xf);
setmask(0x0F);
- for (y = 0; y < rect->height; y++) {
+ for (y = 0; y < rect->height && IS_SAFE(where) && IS_SAFE(where + 1); y++) {
rmw(where);
rmw(where+1);
where += info->fix.line_length;
@@ -919,7 +931,7 @@ static void vga16fb_fillrect(struct fb_info *info, const struct fb_fillrect *rec
setmask(0xff);
while (height--) {
- for (x = 0; x < width; x++) {
+ for (x = 0; x < width && IS_SAFE(dst); x++) {
writeb(0, dst);
dst++;
}
@@ -935,7 +947,7 @@ static void vga16fb_fillrect(struct fb_info *info, const struct fb_fillrect *rec
setmask(0xff);
while (height--) {
- for (x = 0; x < width; x++) {
+ for (x = 0; x < width && IS_SAFE(dst); x++) {
rmw(dst);
dst++;
}
@@ -975,7 +987,7 @@ static void vga_8planes_copyarea(struct fb_info *info, const struct fb_copyarea
dest = info->screen_base + dx + area->dy * info->fix.line_length;
src = info->screen_base + sx + area->sy * info->fix.line_length;
while (height--) {
- for (x = 0; x < width; x++) {
+ for (x = 0; x < width && IS_SAFE(src) && IS_SAFE(dest); x++) {
readb(src);
writeb(0, dest);
src++;
@@ -991,7 +1003,7 @@ static void vga_8planes_copyarea(struct fb_info *info, const struct fb_copyarea
src = info->screen_base + sx + width +
(area->sy + height - 1) * info->fix.line_length;
while (height--) {
- for (x = 0; x < width; x++) {
+ for (x = 0; x < width && IS_SAFE(src - 1) && IS_SAFE(dest - 1); x++) {
--src;
--dest;
readb(src);
@@ -1065,7 +1077,7 @@ static void vga16fb_copyarea(struct fb_info *info, const struct fb_copyarea *are
dst = info->screen_base + (dx/8) + dy * info->fix.line_length;
src = info->screen_base + (sx/8) + sy * info->fix.line_length;
while (height--) {
- for (x = 0; x < width; x++) {
+ for (x = 0; x < width && IS_SAFE(src) && IS_SAFE(dst); x++) {
readb(src);
writeb(0, dst);
dst++;
@@ -1080,7 +1092,7 @@ static void vga16fb_copyarea(struct fb_info *info, const struct fb_copyarea *are
src = info->screen_base + (sx/8) + width +
(sy + height - 1) * info->fix.line_length;
while (height--) {
- for (x = 0; x < width; x++) {
+ for (x = 0; x < width && IS_SAFE(src - 1) && IS_SAFE(dst - 1); x++) {
dst--;
src--;
readb(src);
@@ -1130,13 +1142,15 @@ static void vga_8planes_imageblit(struct fb_info *info, const struct fb_image *i
where = info->screen_base + dx + image->dy * info->fix.line_length;
setmask(0xff);
- writeb(image->bg_color, where);
- readb(where);
+ if (IS_SAFE(where)) {
+ writeb(image->bg_color, where);
+ readb(where);
+ }
selectmask();
setmask(image->fg_color ^ image->bg_color);
setmode(0x42);
setop(0x18);
- for (y = 0; y < image->height; y++, where += info->fix.line_length)
+ for (y = 0; y < image->height && IS_SAFE(where); y++, where += info->fix.line_length)
writew(transl_h[cdat[y]&0xF] | transl_l[cdat[y] >> 4], where);
setmask(oldmask);
setsr(oldsr);
@@ -1165,14 +1179,16 @@ static void vga_imageblit_expand(struct fb_info *info, const struct fb_image *im
selectmask();
setmask(0xff);
- writeb(image->bg_color, where);
- rmb();
- readb(where); /* fill latches */
+ if (IS_SAFE(where)) {
+ writeb(image->bg_color, where);
+ rmb();
+ readb(where); /* fill latches */
+ }
setmode(3);
wmb();
for (y = 0; y < image->height; y++) {
dst = where;
- for (x = image->width/8; x--;)
+ for (x = image->width/8; x-- && IS_SAFE(dst);)
writeb(*cdat++, dst++);
where += info->fix.line_length;
}
@@ -1187,7 +1203,7 @@ static void vga_imageblit_expand(struct fb_info *info, const struct fb_image *im
setmask(0xff);
for (y = 0; y < image->height; y++) {
dst = where;
- for (x=image->width/8; x--;){
+ for (x = image->width/8 && IS_SAFE(dst); x--;) {
rmw(dst);
setcolor(image->fg_color);
selectmask();
@@ -1237,8 +1253,10 @@ static void vga_imageblit_color(struct fb_info *info, const struct fb_image *ima
setcolor(*cdat);
selectmask();
setmask(1 << (7 - (x % 8)));
- fb_readb(dst);
- fb_writeb(0, dst);
+ if (IS_SAFE(dst)) {
+ fb_readb(dst);
+ fb_writeb(0, dst);
+ }
cdat++;
}
--
2.18.4
On Mon, May 17, 2021 at 3:07 PM Daniel Vetter <[email protected]> wrote:
>
> On Fri, May 14, 2021 at 10:33 PM Linus Torvalds
> <[email protected]> wrote:
> >
> > On Fri, May 14, 2021 at 1:25 PM Maciej W. Rozycki <[email protected]> wrote:
> > >
> > > Overall I think it does make sense to resize the text console at any
> > > time, even if the visible console (VT) chosen is in the graphics mode,
> >
> > It might make sense, but only if we call the function to update the
> > low-level data.
> >
> > Not calling it, and then starting to randomly use the (wrong)
> > geometry, and just limiting it so that it's all within the buffer -
> > THAT does not make sense.
> >
> > So I think your patch is fundamentally wrong. It basically says "let's
> > use random stale incorrect data, but just make sure that the end
> > result is still within the allocated buffer".
> >
> > My patch is at least conceptually sane.
> >
> > An alternative would be to just remove the "vcmode != KD_GRAPHICS"
> > check entirely, and always call con_resize() to update the low-level
> > data, but honestly, that seems very likelty to break something very
> > fundamentally, since it's not how any of fbcon has ever been tested,
>
> Just an aside: I think with fbdev drivers this would go boom, because
> you'd have fbcon interferring with a direct /dev/fb/* user.
Boom here means a bit of screen corruption, because fbcon overdraws
your X sessions. Fixed by the next redraw of X.
> But if your fbdev driver is actually a drm modeset driver, then we
> have additional limitations: If the userspace accesses the display
> through /dev/dri/card0, then the kernel blocks all access through
> /dev/fb/* (including fbcon) to the actual display (it only goes into
> the buffer used for fbdev emulation). And everything would be fine.
>
> Also generally you'd get away with this even in problematic cases,
> since usually you resize your console when looking at it, not when X
> or something else is using your fbdev direct access.
>
> The one thing that's left out here a bit in the cold is userspace
> modeset drivers in X. Those would get hosed. But also, we stopped
> supporting those in at least i915/amd/radeon/nouveau drivers,
> automatically falling back to the fbdev stuff in most cases (with or
> without the drm drivers underneath that), and no one screamed. So
> probably not many users left.
This one could lead to incosistent hw state, which would be worse.
> So I /think/ we could wager this, if it's the least intrusive fix from
> the kernel pov. But it has some risks that we need to revert again if
> we break some of the really old use-cases here.
Cheers, Daniel
> > Another alternative would be to just delay the resize to when vcmode
> > is put back to text mode again. That sounds somewhat reasonable to me,
> > but it's a pretty big thing.
> >
> > But no, your patch to just "knowingly use entirely wrong values, then
> > add a limit check because we know the values are possibly garbage and
> > not consistent with reality" is simply not acceptable.
> >
> > Linus
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch