* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-26 16:25 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-26 16:25 UTC (permalink / raw)
To: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs
Cc: linux-fbdev, linux-kernel, dri-devel
A simplified reproducer and debug printk() patch shown below reported that
vc_font.height is increased to 9 via ioctl(VT_RESIZEX) after it was once
decreased from 16 to 2 via ioctl(PIO_FONT).
Since vc_resize() with v.v_rows == 0 preserves current vc->vc_rows value,
this reproducer is bypassing
if (v.v_clin) {
int rows = v.v_vlin / v.v_clin;
if (v.v_rows != rows) {
if (v.v_rows) /* Parameters don't add up */
return -EINVAL;
v.v_rows = rows;
}
}
check by setting v.v_vlin == 1 and v.v_clin == 9.
If v.v_vcol > 0 and v.v_vcol != vc->vc_cols (though this reproducer is passing
v.v_vcol == 0), tty_do_resize() from vc_do_resize() from vc_resize() can make
"struct tty_struct"->winsize.ws_ypixel = 1 despite
"struct tty_struct"->winsize.vc->vc_rows = vc->vc_rows (which is usually larger
than 1). Does such winsize (a row has 1 / vc->vc_rows pixel) make sense?
Since I don't know the meaning of "struct vt_consize"->v_clin (which is commented
with "/* number of pixel rows per character */" but does it mean font size ?),
I don't know why we can assign that value to vcp->vc_font.height via
if (v.v_clin)
vcp->vc_font.height = v.v_clin;
in vt_resizex(). While ioctl(PIO_FONT) needs to pass vc->vc_sw->con_font_set()
check in con_font_set(), ioctl(VT_RESIZEX) does not pass it in vt_resizex()...
Since this problem does not happen if I remove
if (v.v_clin)
vcp->vc_font.height = v.v_clin;
from vt_resizex(), I guess that some variables are getting confused by change
of vc->vc_font.height ...
#define VT_RESIZE 0x5609 /* set kernel's idea of screensize */
#define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */
Hmm, what comes to the "+ more" part? Who is using VT_RESIZEX ?
If nobody is using VT_RESIZEX, better to make VT_RESIZEX == VT_RESIZE ?
----------
#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[])
{
int fd = open("/dev/tty1", 3);
static char fontdata[8192] = { 2, 3 };
struct vt_consize v_c = { 0, 0, 1, 9, 0, 0 };
ioctl(fd, PIO_FONT, fontdata);
ioctl(fd, VT_RESIZEX, &v_c);
return 0;
}
----------
----------
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520bdd521..df6b7abd1068 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -769,64 +769,70 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
{
struct vt_consize v;
int i;
+ int ret = 0;
if (copy_from_user(&v, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
+ console_lock();
+ for (i = 0; i < MAX_NR_CONSOLES; i++) {
+ struct vc_data *vcp = vc_cons[i].d;
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
+ if (!vcp)
+ continue;
+ if (v.v_clin) {
+ int rows = (v.v_vlin ? v.v_vlin : vcp->vc_scan_lines) / v.v_clin;
+ if (v.v_rows != rows) {
+ if (v.v_rows) { /* Parameters don't add up */
+ ret = -EINVAL;
+ break;
+ }
+ v.v_rows = rows;
+ }
}
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
+ if (v.v_vcol && v.v_ccol) {
+ int cols = v.v_vcol / v.v_ccol;
+ if (v.v_cols != cols) {
+ if (v.v_cols) {
+ ret = -EINVAL;
+ break;
+ }
+ v.v_cols = cols;
+ }
+ }
+ if (v.v_clin > 32) {
+ ret = -EINVAL;
+ break;
}
}
+ printk(KERN_INFO "vc=%px v.v_rows=%hu v.v_cols=%hu v.v_vlin=%hu v.v_clin=%u v.v_vcol=%hu v.v_ccol=%hu ret=%d\n", vc, v.v_rows, v.v_cols, v.v_vlin, v.v_clin, v.v_vcol, v.v_ccol, ret);
+ for (i = 0; !ret && i < MAX_NR_CONSOLES; i++) {
+ unsigned int save_scan_lines;
+ unsigned int save_font_height;
+ struct vc_data *vcp = vc_cons[i].d;
- if (v.v_clin > 32)
- return -EINVAL;
-
- for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
-
- if (!vc_cons[i].d)
+ if (!vcp)
continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
- vcp->vc_scan_lines = v.v_vlin;
- if (v.v_clin)
- vcp->vc_font.height = v.v_clin;
- vcp->vc_resize_user = 1;
- ret = vc_resize(vcp, v.v_cols, v.v_rows);
- if (ret) {
- vcp->vc_scan_lines = save_scan_lines;
- vcp->vc_font.height = save_font_height;
- console_unlock();
- return ret;
- }
+ save_scan_lines = vcp->vc_scan_lines;
+ save_font_height = vcp->vc_font.height;
+ if (v.v_vlin)
+ vcp->vc_scan_lines = v.v_vlin;
+ if (v.v_clin)
+ vcp->vc_font.height = v.v_clin;
+ printk(KERN_INFO "vcp=%px before: ->vc_rows=%u ->vc_cols=%u ->vc_scan_lines=%u save_scan_lines=%u ->vc_font.height=%u save_font_height=%u\n",
+ vcp, vcp->vc_rows, vcp->vc_cols, vcp->vc_scan_lines, save_scan_lines, vcp->vc_font.height, save_font_height);
+ vcp->vc_resize_user = 1;
+ ret = vc_resize(vcp, v.v_cols, v.v_rows);
+ printk(KERN_INFO "vcp=%px after: ->vc_rows=%u ->vc_cols=%u ->vc_scan_lines=%u save_scan_lines=%u ->vc_font.height=%u save_font_height=%u ret=%d\n",
+ vcp, vcp->vc_rows, vcp->vc_cols, vcp->vc_scan_lines, save_scan_lines, vcp->vc_font.height, save_font_height, ret);
+ if (ret) {
+ vcp->vc_scan_lines = save_scan_lines;
+ vcp->vc_font.height = save_font_height;
}
- console_unlock();
}
+ console_unlock();
- return 0;
+ return ret;
}
/*
diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
index 9725ecd1255b..c1b9f43ff657 100644
--- a/drivers/video/fbdev/core/bitblit.c
+++ b/drivers/video/fbdev/core/bitblit.c
@@ -181,6 +181,8 @@ static void bit_putcs(struct vc_data *vc, struct fb_info *info,
dst = fb_get_buffer_offset(info, &info->pixmap, size);
image.data = dst;
+ printk(KERN_DEBUG "%s: width=%u cellsize=%u count=%d maxcnt=%u scan_align=%u buf_align=%u image.width=%u image.height=%u pitch=%u\n",
+ __func__, width, cellsize, count, maxcnt, scan_align, buf_align, image.width, image.height, pitch);
if (!mod)
bit_putcs_aligned(vc, info, s, attribute, cnt, pitch,
width, cellsize, &image, buf, dst);
----------
----------
[ 297.298013] [ T2823] bit_putcs: width=1 cellsize=16 count=80 maxcnt=512 scan_align=0 buf_align=0 image.width=640 image.height=16 pitch=80
[ 297.312092] [ T2823] bit_putcs: width=1 cellsize=16 count=80 maxcnt=512 scan_align=0 buf_align=0 image.width=640 image.height=16 pitch=80
[ 297.324735] [ T2823] bit_putcs: width=1 cellsize=16 count=80 maxcnt=512 scan_align=0 buf_align=0 image.width=640 image.height=16 pitch=80
[ 297.337634] [ T2823] bit_putcs: width=1 cellsize=16 count=80 maxcnt=512 scan_align=0 buf_align=0 image.width=640 image.height=16 pitch=80
[ 297.350185] [ T2823] bit_putcs: width=1 cellsize=16 count=80 maxcnt=512 scan_align=0 buf_align=0 image.width=640 image.height=16 pitch=80
[ 297.361861] [ T2823] bit_putcs: width=1 cellsize=16 count=80 maxcnt=512 scan_align=0 buf_align=0 image.width=640 image.height=16 pitch=80
[ 297.474116] [ T2823] bit_putcs: width=1 cellsize=16 count=28 maxcnt=512 scan_align=0 buf_align=0 image.width=224 image.height=16 pitch=28
[ 297.481866] [ T2823] bit_putcs: width=1 cellsize=16 count=4 maxcnt=512 scan_align=0 buf_align=0 image.width=32 image.height=16 pitch=4
[ 297.483529] [ T2823] bit_putcs: width=1 cellsize=16 count=1 maxcnt=512 scan_align=0 buf_align=0 image.width=8 image.height=16 pitch=1
[ 297.484792] [ T2823] bit_putcs: width=1 cellsize=16 count=7 maxcnt=512 scan_align=0 buf_align=0 image.width=56 image.height=16 pitch=7
[ 357.373828] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
[ 357.375232] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
[ 357.376821] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
[ 357.378256] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
[ 357.379684] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
(...snipped...)
[ 357.720089] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
[ 357.721519] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
[ 357.722962] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
[ 357.724390] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
[ 357.725834] [ T2940] bit_putcs: width=1 cellsize=2 count=80 maxcnt=4096 scan_align=0 buf_align=0 image.width=640 image.height=2 pitch=80
[ 357.727876] [ T2940] vc=ffff8880d69b6000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
[ 357.727933] [ T2940] vcp=ffff8880d69b6000 before: ->vc_rows=240 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=2
[ 357.727994] [ T2940] vcp=ffff8880d69b6000 after: ->vc_rows=240 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=2 ret=0
[ 357.728050] [ T2940] vcp=ffff8880d46d9000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 357.728110] [ T2940] vcp=ffff8880d46d9000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 357.728167] [ T2940] vcp=ffff8880d40a8000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 357.728227] [ T2940] vcp=ffff8880d40a8000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 357.728283] [ T2940] vcp=ffff8880d46fb000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 357.728344] [ T2940] vcp=ffff8880d46fb000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 357.728400] [ T2940] vcp=ffff8880d46f9000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 357.728460] [ T2940] vcp=ffff8880d46f9000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 357.728516] [ T2940] vcp=ffff8880ce2d1000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 357.728616] [ T2940] vcp=ffff8880ce2d1000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 357.743762] [ C0] bit_putcs: width=1 cellsize=9 count=68 maxcnt=910 scan_align=0 buf_align=0 image.width=544 image.height=9 pitch=68
[ 357.743783] [ C0] ==================================================================
[ 357.743803] [ C0] BUG: KASAN: slab-out-of-bounds in bit_putcs+0x6a0/0x980
[ 357.743822] [ C0] Read of size 1 at addr ffff8880d46ff343 by task a.out/2940
[ 357.743830] [ C0]
[ 357.743848] [ C0] CPU: 0 PID: 2940 Comm: a.out Not tainted 5.9.0-rc6+ #635
[ 357.743871] [ C0] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 357.743880] [ C0] Call Trace:
[ 357.743891] [ C0] dump_stack+0x161/0x1c3
[ 357.743902] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.743914] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.743932] [ C0] print_address_description.constprop.0.cold+0xd3/0x4c5
[ 357.743944] [ C0] ? log_store.cold+0x16/0x16
[ 357.743956] [ C0] ? vprintk_func+0xe2/0x155
[ 357.743968] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.743979] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.743992] [ C0] kasan_report.cold+0x1f/0x42
[ 357.744003] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.744014] [ C0] bit_putcs+0x6a0/0x980
[ 357.744026] [ C0] ? bit_clear+0x2f0/0x2f0
[ 357.744038] [ C0] ? find_held_lock+0x85/0xa0
[ 357.744052] [ C0] ? raw_notifier_call_chain+0x31/0x40
[ 357.744067] [ C0] ? fb_get_color_depth.part.0+0x57/0xe0
[ 357.744082] [ C0] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 357.744093] [ C0] fbcon_putcs+0x1d8/0x1e0
[ 357.744105] [ C0] ? bit_clear+0x2f0/0x2f0
[ 357.744118] [ C0] vt_console_print+0x72d/0x870
[ 357.744130] [ C0] ? fb_flashcursor+0x230/0x230
[ 357.744144] [ C0] ? screen_glyph_unicode+0x140/0x140
[ 357.744157] [ C0] ? rwlock_bug.part.0+0x50/0x50
[ 357.744171] [ C0] ? screen_glyph_unicode+0x140/0x140
[ 357.744183] [ C0] console_unlock+0x92c/0xb30
[ 357.744195] [ C0] vt_ioctl.cold+0x182/0x3a2
[ 357.744210] [ C0] ? complete_change_console+0x1e0/0x1e0
[ 357.744222] [ C0] ? find_held_lock+0x85/0xa0
[ 357.744237] [ C0] ? debug_check_no_obj_freed+0x18d/0x276
[ 357.744249] [ C0] ? lock_downgrade+0x3e0/0x3e0
[ 357.744261] [ C0] ? find_held_lock+0x85/0xa0
[ 357.744274] [ C0] ? lock_is_held_type+0xbf/0xf0
[ 357.744285] [ C0] ? putname+0xa7/0xc0
[ 357.744296] [ C0] ? putname+0xa7/0xc0
[ 357.744306] [ C0] ? putname+0xa7/0xc0
[ 357.744322] [ C0] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 357.744336] [ C0] ? complete_change_console+0x1e0/0x1e0
[ 357.744361] [ C0] ? tty_ioctl+0x7c4/0xec0
[ 357.744373] [ C0] tty_ioctl+0x7c4/0xec0
[ 357.744387] [ C0] ? kmem_cache_free.part.0+0x1b0/0x1e0
[ 357.744399] [ C0] ? tty_vhangup+0x30/0x30
[ 357.744414] [ C0] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 357.744426] [ C0] ? do_vfs_ioctl+0x224/0xc50
[ 357.744439] [ C0] ? ioctl_file_clone+0x140/0x140
[ 357.744452] [ C0] ? file_open_root+0x420/0x420
[ 357.744467] [ C0] ? check_preemption_disabled+0x50/0x130
[ 357.744479] [ C0] ? lock_is_held_type+0xbf/0xf0
[ 357.744495] [ C0] ? syscall_enter_from_user_mode+0x1c/0x60
[ 357.744509] [ C0] ? rcu_read_lock_sched_held+0xa0/0xd0
[ 357.744521] [ C0] ? mark_held_locks+0x24/0x90
[ 357.744533] [ C0] ? tty_vhangup+0x30/0x30
[ 357.744545] [ C0] __x64_sys_ioctl+0xec/0x140
[ 357.744557] [ C0] do_syscall_64+0x31/0x70
[ 357.744572] [ C0] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 357.744583] [ C0] RIP: 0033:0x7f9b8316150b
[ 357.744632] [ C0] 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
[ 357.744647] [ C0] RSP: 002b:00007ffe190139b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 357.744680] [ C0] RAX: ffffffffffffffda RBX: 000055dc2ea7b220 RCX: 00007f9b8316150b
[ 357.744701] [ C0] RDX: 00007ffe190139cc RSI: 000000000000560a RDI: 0000000000000003
[ 357.744721] [ C0] RBP: 0000000000000003 R08: 0000000000000000 R09: 00007f9b83257d50
[ 357.744741] [ C0] R10: 0000000000000000 R11: 0000000000000246 R12: 000055dc2ea7b130
[ 357.744762] [ C0] R13: 00007ffe19013ad0 R14: 0000000000000000 R15: 0000000000000000
[ 357.744769] [ C0]
[ 357.744781] [ C0] Allocated by task 2940:
[ 357.744793] [ C0] kasan_save_stack+0x1f/0x40
[ 357.744808] [ C0] __kasan_kmalloc.constprop.0+0xbf/0xd0
[ 357.744819] [ C0] __kmalloc+0x57d/0x9d0
[ 357.744831] [ C0] fbcon_set_font+0x1a6/0x4a0
[ 357.744843] [ C0] con_font_op+0x8e2/0xac0
[ 357.744854] [ C0] vt_ioctl+0x1186/0x21a0
[ 357.744866] [ C0] tty_ioctl+0x7c4/0xec0
[ 357.744878] [ C0] __x64_sys_ioctl+0xec/0x140
[ 357.744889] [ C0] do_syscall_64+0x31/0x70
[ 357.744905] [ C0] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 357.744912] [ C0]
[ 357.744932] [ C0] The buggy address belongs to the object at ffff8880d46ff000
[ 357.744949] [ C0] which belongs to the cache kmalloc-1k of size 1024
[ 357.744967] [ C0] The buggy address is located 835 bytes inside of
[ 357.744985] [ C0] 1024-byte region [ffff8880d46ff000, ffff8880d46ff400)
[ 357.745000] [ C0] The buggy address belongs to the page:
[ 357.745027] [ C0] page:00000000878ccadc refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0xd46ff
[ 357.745040] [ C0] flags: 0xfffe0000000200(slab)
[ 357.745063] [ C0] raw: 00fffe0000000200 ffffea00032a0808 ffffea00035ae308 ffff8880d6840700
[ 357.745086] [ C0] raw: 0000000000000000 ffff8880d46ff000 0000000100000002 ffff8880cbe8b281
[ 357.745103] [ C0] page dumped because: kasan: bad access detected
[ 357.745117] [ C0] page->mem_cgroup:ffff8880cbe8b281
[ 357.745125] [ C0]
[ 357.745140] [ C0] Memory state around the buggy address:
[ 357.745162] [ C0] ffff8880d46ff200: 00 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745186] [ C0] ffff8880d46ff280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745208] [ C0] >ffff8880d46ff300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745224] [ C0] ^
[ 357.745246] [ C0] ffff8880d46ff380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745267] [ C0] ffff8880d46ff400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745288] [ C0] ==================================================================
[ 357.745305] [ C0] Disabling lock debugging due to kernel taint
[ 357.745349] [ C0] bit_putcs: width=1 cellsize=9 count=46 maxcnt=910 scan_align=0 buf_align=0 image.width=368 image.height=9 pitch=46
[ 357.759878] [ C0] bit_putcs: width=1 cellsize=9 count=80 maxcnt=910 scan_align=0 buf_align=0 image.width=640 image.height=9 pitch=80
[ 357.759910] [ C0] bit_putcs: width=1 cellsize=9 count=74 maxcnt=910 scan_align=0 buf_align=0 image.width=592 image.height=9 pitch=74
[ 357.775006] [ C0] bit_putcs: width=1 cellsize=9 count=80 maxcnt=910 scan_align=0 buf_align=0 image.width=640 image.height=9 pitch=80
----------
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-26 16:25 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-26 16:25 UTC (permalink / raw)
To: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs
Cc: linux-fbdev, linux-kernel, dri-devel
A simplified reproducer and debug printk() patch shown below reported that
vc_font.height is increased to 9 via ioctl(VT_RESIZEX) after it was once
decreased from 16 to 2 via ioctl(PIO_FONT).
Since vc_resize() with v.v_rows = 0 preserves current vc->vc_rows value,
this reproducer is bypassing
if (v.v_clin) {
int rows = v.v_vlin / v.v_clin;
if (v.v_rows != rows) {
if (v.v_rows) /* Parameters don't add up */
return -EINVAL;
v.v_rows = rows;
}
}
check by setting v.v_vlin = 1 and v.v_clin = 9.
If v.v_vcol > 0 and v.v_vcol != vc->vc_cols (though this reproducer is passing
v.v_vcol = 0), tty_do_resize() from vc_do_resize() from vc_resize() can make
"struct tty_struct"->winsize.ws_ypixel = 1 despite
"struct tty_struct"->winsize.vc->vc_rows = vc->vc_rows (which is usually larger
than 1). Does such winsize (a row has 1 / vc->vc_rows pixel) make sense?
Since I don't know the meaning of "struct vt_consize"->v_clin (which is commented
with "/* number of pixel rows per character */" but does it mean font size ?),
I don't know why we can assign that value to vcp->vc_font.height via
if (v.v_clin)
vcp->vc_font.height = v.v_clin;
in vt_resizex(). While ioctl(PIO_FONT) needs to pass vc->vc_sw->con_font_set()
check in con_font_set(), ioctl(VT_RESIZEX) does not pass it in vt_resizex()...
Since this problem does not happen if I remove
if (v.v_clin)
vcp->vc_font.height = v.v_clin;
from vt_resizex(), I guess that some variables are getting confused by change
of vc->vc_font.height ...
#define VT_RESIZE 0x5609 /* set kernel's idea of screensize */
#define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */
Hmm, what comes to the "+ more" part? Who is using VT_RESIZEX ?
If nobody is using VT_RESIZEX, better to make VT_RESIZEX = VT_RESIZE ?
----------
#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[])
{
int fd = open("/dev/tty1", 3);
static char fontdata[8192] = { 2, 3 };
struct vt_consize v_c = { 0, 0, 1, 9, 0, 0 };
ioctl(fd, PIO_FONT, fontdata);
ioctl(fd, VT_RESIZEX, &v_c);
return 0;
}
----------
----------
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520bdd521..df6b7abd1068 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -769,64 +769,70 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
{
struct vt_consize v;
int i;
+ int ret = 0;
if (copy_from_user(&v, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
+ console_lock();
+ for (i = 0; i < MAX_NR_CONSOLES; i++) {
+ struct vc_data *vcp = vc_cons[i].d;
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
+ if (!vcp)
+ continue;
+ if (v.v_clin) {
+ int rows = (v.v_vlin ? v.v_vlin : vcp->vc_scan_lines) / v.v_clin;
+ if (v.v_rows != rows) {
+ if (v.v_rows) { /* Parameters don't add up */
+ ret = -EINVAL;
+ break;
+ }
+ v.v_rows = rows;
+ }
}
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
+ if (v.v_vcol && v.v_ccol) {
+ int cols = v.v_vcol / v.v_ccol;
+ if (v.v_cols != cols) {
+ if (v.v_cols) {
+ ret = -EINVAL;
+ break;
+ }
+ v.v_cols = cols;
+ }
+ }
+ if (v.v_clin > 32) {
+ ret = -EINVAL;
+ break;
}
}
+ printk(KERN_INFO "vc=%px v.v_rows=%hu v.v_cols=%hu v.v_vlin=%hu v.v_clin=%u v.v_vcol=%hu v.v_ccol=%hu ret=%d\n", vc, v.v_rows, v.v_cols, v.v_vlin, v.v_clin, v.v_vcol, v.v_ccol, ret);
+ for (i = 0; !ret && i < MAX_NR_CONSOLES; i++) {
+ unsigned int save_scan_lines;
+ unsigned int save_font_height;
+ struct vc_data *vcp = vc_cons[i].d;
- if (v.v_clin > 32)
- return -EINVAL;
-
- for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
-
- if (!vc_cons[i].d)
+ if (!vcp)
continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
- vcp->vc_scan_lines = v.v_vlin;
- if (v.v_clin)
- vcp->vc_font.height = v.v_clin;
- vcp->vc_resize_user = 1;
- ret = vc_resize(vcp, v.v_cols, v.v_rows);
- if (ret) {
- vcp->vc_scan_lines = save_scan_lines;
- vcp->vc_font.height = save_font_height;
- console_unlock();
- return ret;
- }
+ save_scan_lines = vcp->vc_scan_lines;
+ save_font_height = vcp->vc_font.height;
+ if (v.v_vlin)
+ vcp->vc_scan_lines = v.v_vlin;
+ if (v.v_clin)
+ vcp->vc_font.height = v.v_clin;
+ printk(KERN_INFO "vcp=%px before: ->vc_rows=%u ->vc_cols=%u ->vc_scan_lines=%u save_scan_lines=%u ->vc_font.height=%u save_font_height=%u\n",
+ vcp, vcp->vc_rows, vcp->vc_cols, vcp->vc_scan_lines, save_scan_lines, vcp->vc_font.height, save_font_height);
+ vcp->vc_resize_user = 1;
+ ret = vc_resize(vcp, v.v_cols, v.v_rows);
+ printk(KERN_INFO "vcp=%px after: ->vc_rows=%u ->vc_cols=%u ->vc_scan_lines=%u save_scan_lines=%u ->vc_font.height=%u save_font_height=%u ret=%d\n",
+ vcp, vcp->vc_rows, vcp->vc_cols, vcp->vc_scan_lines, save_scan_lines, vcp->vc_font.height, save_font_height, ret);
+ if (ret) {
+ vcp->vc_scan_lines = save_scan_lines;
+ vcp->vc_font.height = save_font_height;
}
- console_unlock();
}
+ console_unlock();
- return 0;
+ return ret;
}
/*
diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
index 9725ecd1255b..c1b9f43ff657 100644
--- a/drivers/video/fbdev/core/bitblit.c
+++ b/drivers/video/fbdev/core/bitblit.c
@@ -181,6 +181,8 @@ static void bit_putcs(struct vc_data *vc, struct fb_info *info,
dst = fb_get_buffer_offset(info, &info->pixmap, size);
image.data = dst;
+ printk(KERN_DEBUG "%s: width=%u cellsize=%u count=%d maxcnt=%u scan_align=%u buf_align=%u image.width=%u image.height=%u pitch=%u\n",
+ __func__, width, cellsize, count, maxcnt, scan_align, buf_align, image.width, image.height, pitch);
if (!mod)
bit_putcs_aligned(vc, info, s, attribute, cnt, pitch,
width, cellsize, &image, buf, dst);
----------
----------
[ 297.298013] [ T2823] bit_putcs: width=1 cellsize\x16 count€ maxcntQ2 scan_align=0 buf_align=0 image.widthd0 image.height\x16 pitch€
[ 297.312092] [ T2823] bit_putcs: width=1 cellsize\x16 count€ maxcntQ2 scan_align=0 buf_align=0 image.widthd0 image.height\x16 pitch€
[ 297.324735] [ T2823] bit_putcs: width=1 cellsize\x16 count€ maxcntQ2 scan_align=0 buf_align=0 image.widthd0 image.height\x16 pitch€
[ 297.337634] [ T2823] bit_putcs: width=1 cellsize\x16 count€ maxcntQ2 scan_align=0 buf_align=0 image.widthd0 image.height\x16 pitch€
[ 297.350185] [ T2823] bit_putcs: width=1 cellsize\x16 count€ maxcntQ2 scan_align=0 buf_align=0 image.widthd0 image.height\x16 pitch€
[ 297.361861] [ T2823] bit_putcs: width=1 cellsize\x16 count€ maxcntQ2 scan_align=0 buf_align=0 image.widthd0 image.height\x16 pitch€
[ 297.474116] [ T2823] bit_putcs: width=1 cellsize\x16 count( maxcntQ2 scan_align=0 buf_align=0 image.width"4 image.height\x16 pitch(
[ 297.481866] [ T2823] bit_putcs: width=1 cellsize\x16 count=4 maxcntQ2 scan_align=0 buf_align=0 image.width2 image.height\x16 pitch=4
[ 297.483529] [ T2823] bit_putcs: width=1 cellsize\x16 count=1 maxcntQ2 scan_align=0 buf_align=0 image.width=8 image.height\x16 pitch=1
[ 297.484792] [ T2823] bit_putcs: width=1 cellsize\x16 count=7 maxcntQ2 scan_align=0 buf_align=0 image.widthV image.height\x16 pitch=7
[ 357.373828] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
[ 357.375232] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
[ 357.376821] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
[ 357.378256] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
[ 357.379684] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
(...snipped...)
[ 357.720089] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
[ 357.721519] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
[ 357.722962] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
[ 357.724390] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
[ 357.725834] [ T2940] bit_putcs: width=1 cellsize=2 count€ maxcnt@96 scan_align=0 buf_align=0 image.widthd0 image.height=2 pitch€
[ 357.727876] [ T2940] vcÿff8880d69b6000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
[ 357.727933] [ T2940] vcpÿff8880d69b6000 before: ->vc_rows$0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines@0 ->vc_font.height=9 save_font_height=2
[ 357.727994] [ T2940] vcpÿff8880d69b6000 after: ->vc_rows$0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines@0 ->vc_font.height=9 save_font_height=2 ret=0
[ 357.728050] [ T2940] vcpÿff8880d46d9000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 357.728110] [ T2940] vcpÿff8880d46d9000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 357.728167] [ T2940] vcpÿff8880d40a8000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 357.728227] [ T2940] vcpÿff8880d40a8000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 357.728283] [ T2940] vcpÿff8880d46fb000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 357.728344] [ T2940] vcpÿff8880d46fb000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 357.728400] [ T2940] vcpÿff8880d46f9000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 357.728460] [ T2940] vcpÿff8880d46f9000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 357.728516] [ T2940] vcpÿff8880ce2d1000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 357.728616] [ T2940] vcpÿff8880ce2d1000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 357.743762] [ C0] bit_putcs: width=1 cellsize=9 counth maxcnt‘0 scan_align=0 buf_align=0 image.widthT4 image.height=9 pitchh
[ 357.743783] [ C0] =================================
[ 357.743803] [ C0] BUG: KASAN: slab-out-of-bounds in bit_putcs+0x6a0/0x980
[ 357.743822] [ C0] Read of size 1 at addr ffff8880d46ff343 by task a.out/2940
[ 357.743830] [ C0]
[ 357.743848] [ C0] CPU: 0 PID: 2940 Comm: a.out Not tainted 5.9.0-rc6+ #635
[ 357.743871] [ C0] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 357.743880] [ C0] Call Trace:
[ 357.743891] [ C0] dump_stack+0x161/0x1c3
[ 357.743902] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.743914] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.743932] [ C0] print_address_description.constprop.0.cold+0xd3/0x4c5
[ 357.743944] [ C0] ? log_store.cold+0x16/0x16
[ 357.743956] [ C0] ? vprintk_func+0xe2/0x155
[ 357.743968] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.743979] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.743992] [ C0] kasan_report.cold+0x1f/0x42
[ 357.744003] [ C0] ? bit_putcs+0x6a0/0x980
[ 357.744014] [ C0] bit_putcs+0x6a0/0x980
[ 357.744026] [ C0] ? bit_clear+0x2f0/0x2f0
[ 357.744038] [ C0] ? find_held_lock+0x85/0xa0
[ 357.744052] [ C0] ? raw_notifier_call_chain+0x31/0x40
[ 357.744067] [ C0] ? fb_get_color_depth.part.0+0x57/0xe0
[ 357.744082] [ C0] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 357.744093] [ C0] fbcon_putcs+0x1d8/0x1e0
[ 357.744105] [ C0] ? bit_clear+0x2f0/0x2f0
[ 357.744118] [ C0] vt_console_print+0x72d/0x870
[ 357.744130] [ C0] ? fb_flashcursor+0x230/0x230
[ 357.744144] [ C0] ? screen_glyph_unicode+0x140/0x140
[ 357.744157] [ C0] ? rwlock_bug.part.0+0x50/0x50
[ 357.744171] [ C0] ? screen_glyph_unicode+0x140/0x140
[ 357.744183] [ C0] console_unlock+0x92c/0xb30
[ 357.744195] [ C0] vt_ioctl.cold+0x182/0x3a2
[ 357.744210] [ C0] ? complete_change_console+0x1e0/0x1e0
[ 357.744222] [ C0] ? find_held_lock+0x85/0xa0
[ 357.744237] [ C0] ? debug_check_no_obj_freed+0x18d/0x276
[ 357.744249] [ C0] ? lock_downgrade+0x3e0/0x3e0
[ 357.744261] [ C0] ? find_held_lock+0x85/0xa0
[ 357.744274] [ C0] ? lock_is_held_type+0xbf/0xf0
[ 357.744285] [ C0] ? putname+0xa7/0xc0
[ 357.744296] [ C0] ? putname+0xa7/0xc0
[ 357.744306] [ C0] ? putname+0xa7/0xc0
[ 357.744322] [ C0] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 357.744336] [ C0] ? complete_change_console+0x1e0/0x1e0
[ 357.744361] [ C0] ? tty_ioctl+0x7c4/0xec0
[ 357.744373] [ C0] tty_ioctl+0x7c4/0xec0
[ 357.744387] [ C0] ? kmem_cache_free.part.0+0x1b0/0x1e0
[ 357.744399] [ C0] ? tty_vhangup+0x30/0x30
[ 357.744414] [ C0] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 357.744426] [ C0] ? do_vfs_ioctl+0x224/0xc50
[ 357.744439] [ C0] ? ioctl_file_clone+0x140/0x140
[ 357.744452] [ C0] ? file_open_root+0x420/0x420
[ 357.744467] [ C0] ? check_preemption_disabled+0x50/0x130
[ 357.744479] [ C0] ? lock_is_held_type+0xbf/0xf0
[ 357.744495] [ C0] ? syscall_enter_from_user_mode+0x1c/0x60
[ 357.744509] [ C0] ? rcu_read_lock_sched_held+0xa0/0xd0
[ 357.744521] [ C0] ? mark_held_locks+0x24/0x90
[ 357.744533] [ C0] ? tty_vhangup+0x30/0x30
[ 357.744545] [ C0] __x64_sys_ioctl+0xec/0x140
[ 357.744557] [ C0] do_syscall_64+0x31/0x70
[ 357.744572] [ C0] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 357.744583] [ C0] RIP: 0033:0x7f9b8316150b
[ 357.744632] [ C0] 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
[ 357.744647] [ C0] RSP: 002b:00007ffe190139b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 357.744680] [ C0] RAX: ffffffffffffffda RBX: 000055dc2ea7b220 RCX: 00007f9b8316150b
[ 357.744701] [ C0] RDX: 00007ffe190139cc RSI: 000000000000560a RDI: 0000000000000003
[ 357.744721] [ C0] RBP: 0000000000000003 R08: 0000000000000000 R09: 00007f9b83257d50
[ 357.744741] [ C0] R10: 0000000000000000 R11: 0000000000000246 R12: 000055dc2ea7b130
[ 357.744762] [ C0] R13: 00007ffe19013ad0 R14: 0000000000000000 R15: 0000000000000000
[ 357.744769] [ C0]
[ 357.744781] [ C0] Allocated by task 2940:
[ 357.744793] [ C0] kasan_save_stack+0x1f/0x40
[ 357.744808] [ C0] __kasan_kmalloc.constprop.0+0xbf/0xd0
[ 357.744819] [ C0] __kmalloc+0x57d/0x9d0
[ 357.744831] [ C0] fbcon_set_font+0x1a6/0x4a0
[ 357.744843] [ C0] con_font_op+0x8e2/0xac0
[ 357.744854] [ C0] vt_ioctl+0x1186/0x21a0
[ 357.744866] [ C0] tty_ioctl+0x7c4/0xec0
[ 357.744878] [ C0] __x64_sys_ioctl+0xec/0x140
[ 357.744889] [ C0] do_syscall_64+0x31/0x70
[ 357.744905] [ C0] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 357.744912] [ C0]
[ 357.744932] [ C0] The buggy address belongs to the object at ffff8880d46ff000
[ 357.744949] [ C0] which belongs to the cache kmalloc-1k of size 1024
[ 357.744967] [ C0] The buggy address is located 835 bytes inside of
[ 357.744985] [ C0] 1024-byte region [ffff8880d46ff000, ffff8880d46ff400)
[ 357.745000] [ C0] The buggy address belongs to the page:
[ 357.745027] [ C0] page:00000000878ccadc refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0xd46ff
[ 357.745040] [ C0] flags: 0xfffe0000000200(slab)
[ 357.745063] [ C0] raw: 00fffe0000000200 ffffea00032a0808 ffffea00035ae308 ffff8880d6840700
[ 357.745086] [ C0] raw: 0000000000000000 ffff8880d46ff000 0000000100000002 ffff8880cbe8b281
[ 357.745103] [ C0] page dumped because: kasan: bad access detected
[ 357.745117] [ C0] page->mem_cgroup:ffff8880cbe8b281
[ 357.745125] [ C0]
[ 357.745140] [ C0] Memory state around the buggy address:
[ 357.745162] [ C0] ffff8880d46ff200: 00 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745186] [ C0] ffff8880d46ff280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745208] [ C0] >ffff8880d46ff300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745224] [ C0] ^
[ 357.745246] [ C0] ffff8880d46ff380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745267] [ C0] ffff8880d46ff400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 357.745288] [ C0] =================================
[ 357.745305] [ C0] Disabling lock debugging due to kernel taint
[ 357.745349] [ C0] bit_putcs: width=1 cellsize=9 countF maxcnt‘0 scan_align=0 buf_align=0 image.width68 image.height=9 pitchF
[ 357.759878] [ C0] bit_putcs: width=1 cellsize=9 count€ maxcnt‘0 scan_align=0 buf_align=0 image.widthd0 image.height=9 pitch€
[ 357.759910] [ C0] bit_putcs: width=1 cellsize=9 countt maxcnt‘0 scan_align=0 buf_align=0 image.widthY2 image.height=9 pitcht
[ 357.775006] [ C0] bit_putcs: width=1 cellsize=9 count€ maxcnt‘0 scan_align=0 buf_align=0 image.widthd0 image.height=9 pitch€
----------
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
2020-09-26 16:25 ` Tetsuo Handa
(?)
@ 2020-09-26 19:39 ` Peilin Ye
-1 siblings, 0 replies; 51+ messages in thread
From: Peilin Ye @ 2020-09-26 19:39 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs, dri-devel, linux-fbdev, linux-kernel,
yepeilin.cs
On Sun, Sep 27, 2020 at 01:25:17AM +0900, Tetsuo Handa wrote:
> A simplified reproducer and debug printk() patch shown below reported that
> vc_font.height is increased to 9 via ioctl(VT_RESIZEX) after it was once
> decreased from 16 to 2 via ioctl(PIO_FONT).
>
>
>
> Since vc_resize() with v.v_rows == 0 preserves current vc->vc_rows value,
> this reproducer is bypassing
>
> if (v.v_clin) {
> int rows = v.v_vlin / v.v_clin;
> if (v.v_rows != rows) {
> if (v.v_rows) /* Parameters don't add up */
> return -EINVAL;
> v.v_rows = rows;
> }
> }
>
> check by setting v.v_vlin == 1 and v.v_clin == 9.
>
> If v.v_vcol > 0 and v.v_vcol != vc->vc_cols (though this reproducer is passing
> v.v_vcol == 0), tty_do_resize() from vc_do_resize() from vc_resize() can make
> "struct tty_struct"->winsize.ws_ypixel = 1 despite
> "struct tty_struct"->winsize.vc->vc_rows = vc->vc_rows (which is usually larger
> than 1). Does such winsize (a row has 1 / vc->vc_rows pixel) make sense?
>
>
>
> Since I don't know the meaning of "struct vt_consize"->v_clin (which is commented
> with "/* number of pixel rows per character */" but does it mean font size ?),
> I don't know why we can assign that value to vcp->vc_font.height via
>
> if (v.v_clin)
> vcp->vc_font.height = v.v_clin;
>
> in vt_resizex(). While ioctl(PIO_FONT) needs to pass vc->vc_sw->con_font_set()
> check in con_font_set(), ioctl(VT_RESIZEX) does not pass it in vt_resizex()...
>
> Since this problem does not happen if I remove
>
> if (v.v_clin)
> vcp->vc_font.height = v.v_clin;
Hi Tetsuo!
> from vt_resizex(), I guess that some variables are getting confused by change
> of vc->vc_font.height ...
Yes, see bit_putcs():
(drivers/video/fbdev/core/bitblit.c)
static void bit_putcs(struct vc_data *vc, struct fb_info *info,
const unsigned short *s, int count, int yy, int xx,
int fg, int bg)
{
struct fb_image image;
u32 width = DIV_ROUND_UP(vc->vc_font.width, 8);
u32 cellsize = width * vc->vc_font.height;
^^^^^^^^ ^^^^^^^^^^^^^^
`cellsize` is now too large. Later, in bit_putcs_aligned():
while (cnt--) {
src = vc->vc_font.data + (scr_readw(s++)&
charmask)*cellsize;
^^^^^^^^
`src` goes out of bounds of the data buffer. At first glance I guess
this is an out-of-bound read reported as a use-after-free read? The
crashlog says:
[ 149.732103][ T6693] Allocated by task 6667:
[ 149.732115][ T6693] kasan_save_stack (mm/kasan/common.c:48)
[ 149.732121][ T6693] __kasan_kmalloc.constprop.0 (mm/kasan/common.c:56 mm/kasan/common.c:461)
[ 149.732126][ T6693] __kmalloc (mm/slab.c:3656 mm/slab.c:3664)
[ 149.732133][ T6693] alloc_pipe_info (fs/pipe.c:810)
[ 149.732139][ T6693] create_pipe_files (fs/pipe.c:883 fs/pipe.c:914)
[ 149.732145][ T6693] do_pipe2 (fs/pipe.c:965 fs/pipe.c:1012)
I'm not sure, but I don't think a buffer allocated in fs/pipe.c is
related here. Maybe they just live near each other on the heap?
To resolve this out-of-bound issue for now, I think the easiest way
is to add a range check in bit_putcs(), or bit_putcs_aligned().
...but yeah, that `VT_RESIZEX` ioctl looks really buggy, and is already
causing more issues:
KASAN: global-out-of-bounds Read in fbcon_get_font
Link: https://syzkaller.appspot.com/bug?id=08b8be45afea11888776f897895aef9ad1c3ecfd
This was also caused by `VT_RESIZEX`...
Thank you,
Peilin Ye
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-26 19:39 ` Peilin Ye
0 siblings, 0 replies; 51+ messages in thread
From: Peilin Ye @ 2020-09-26 19:39 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, b.zolnierkie, daniel.vetter, deller,
syzkaller-bugs, linux-kernel, dri-devel, gregkh, jirislaby,
yepeilin.cs
On Sun, Sep 27, 2020 at 01:25:17AM +0900, Tetsuo Handa wrote:
> A simplified reproducer and debug printk() patch shown below reported that
> vc_font.height is increased to 9 via ioctl(VT_RESIZEX) after it was once
> decreased from 16 to 2 via ioctl(PIO_FONT).
>
>
>
> Since vc_resize() with v.v_rows == 0 preserves current vc->vc_rows value,
> this reproducer is bypassing
>
> if (v.v_clin) {
> int rows = v.v_vlin / v.v_clin;
> if (v.v_rows != rows) {
> if (v.v_rows) /* Parameters don't add up */
> return -EINVAL;
> v.v_rows = rows;
> }
> }
>
> check by setting v.v_vlin == 1 and v.v_clin == 9.
>
> If v.v_vcol > 0 and v.v_vcol != vc->vc_cols (though this reproducer is passing
> v.v_vcol == 0), tty_do_resize() from vc_do_resize() from vc_resize() can make
> "struct tty_struct"->winsize.ws_ypixel = 1 despite
> "struct tty_struct"->winsize.vc->vc_rows = vc->vc_rows (which is usually larger
> than 1). Does such winsize (a row has 1 / vc->vc_rows pixel) make sense?
>
>
>
> Since I don't know the meaning of "struct vt_consize"->v_clin (which is commented
> with "/* number of pixel rows per character */" but does it mean font size ?),
> I don't know why we can assign that value to vcp->vc_font.height via
>
> if (v.v_clin)
> vcp->vc_font.height = v.v_clin;
>
> in vt_resizex(). While ioctl(PIO_FONT) needs to pass vc->vc_sw->con_font_set()
> check in con_font_set(), ioctl(VT_RESIZEX) does not pass it in vt_resizex()...
>
> Since this problem does not happen if I remove
>
> if (v.v_clin)
> vcp->vc_font.height = v.v_clin;
Hi Tetsuo!
> from vt_resizex(), I guess that some variables are getting confused by change
> of vc->vc_font.height ...
Yes, see bit_putcs():
(drivers/video/fbdev/core/bitblit.c)
static void bit_putcs(struct vc_data *vc, struct fb_info *info,
const unsigned short *s, int count, int yy, int xx,
int fg, int bg)
{
struct fb_image image;
u32 width = DIV_ROUND_UP(vc->vc_font.width, 8);
u32 cellsize = width * vc->vc_font.height;
^^^^^^^^ ^^^^^^^^^^^^^^
`cellsize` is now too large. Later, in bit_putcs_aligned():
while (cnt--) {
src = vc->vc_font.data + (scr_readw(s++)&
charmask)*cellsize;
^^^^^^^^
`src` goes out of bounds of the data buffer. At first glance I guess
this is an out-of-bound read reported as a use-after-free read? The
crashlog says:
[ 149.732103][ T6693] Allocated by task 6667:
[ 149.732115][ T6693] kasan_save_stack (mm/kasan/common.c:48)
[ 149.732121][ T6693] __kasan_kmalloc.constprop.0 (mm/kasan/common.c:56 mm/kasan/common.c:461)
[ 149.732126][ T6693] __kmalloc (mm/slab.c:3656 mm/slab.c:3664)
[ 149.732133][ T6693] alloc_pipe_info (fs/pipe.c:810)
[ 149.732139][ T6693] create_pipe_files (fs/pipe.c:883 fs/pipe.c:914)
[ 149.732145][ T6693] do_pipe2 (fs/pipe.c:965 fs/pipe.c:1012)
I'm not sure, but I don't think a buffer allocated in fs/pipe.c is
related here. Maybe they just live near each other on the heap?
To resolve this out-of-bound issue for now, I think the easiest way
is to add a range check in bit_putcs(), or bit_putcs_aligned().
...but yeah, that `VT_RESIZEX` ioctl looks really buggy, and is already
causing more issues:
KASAN: global-out-of-bounds Read in fbcon_get_font
Link: https://syzkaller.appspot.com/bug?id=08b8be45afea11888776f897895aef9ad1c3ecfd
This was also caused by `VT_RESIZEX`...
Thank you,
Peilin Ye
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-26 19:39 ` Peilin Ye
0 siblings, 0 replies; 51+ messages in thread
From: Peilin Ye @ 2020-09-26 19:39 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, b.zolnierkie, daniel.vetter, deller,
syzkaller-bugs, linux-kernel, dri-devel, gregkh, jirislaby,
yepeilin.cs
On Sun, Sep 27, 2020 at 01:25:17AM +0900, Tetsuo Handa wrote:
> A simplified reproducer and debug printk() patch shown below reported that
> vc_font.height is increased to 9 via ioctl(VT_RESIZEX) after it was once
> decreased from 16 to 2 via ioctl(PIO_FONT).
>
>
>
> Since vc_resize() with v.v_rows = 0 preserves current vc->vc_rows value,
> this reproducer is bypassing
>
> if (v.v_clin) {
> int rows = v.v_vlin / v.v_clin;
> if (v.v_rows != rows) {
> if (v.v_rows) /* Parameters don't add up */
> return -EINVAL;
> v.v_rows = rows;
> }
> }
>
> check by setting v.v_vlin = 1 and v.v_clin = 9.
>
> If v.v_vcol > 0 and v.v_vcol != vc->vc_cols (though this reproducer is passing
> v.v_vcol = 0), tty_do_resize() from vc_do_resize() from vc_resize() can make
> "struct tty_struct"->winsize.ws_ypixel = 1 despite
> "struct tty_struct"->winsize.vc->vc_rows = vc->vc_rows (which is usually larger
> than 1). Does such winsize (a row has 1 / vc->vc_rows pixel) make sense?
>
>
>
> Since I don't know the meaning of "struct vt_consize"->v_clin (which is commented
> with "/* number of pixel rows per character */" but does it mean font size ?),
> I don't know why we can assign that value to vcp->vc_font.height via
>
> if (v.v_clin)
> vcp->vc_font.height = v.v_clin;
>
> in vt_resizex(). While ioctl(PIO_FONT) needs to pass vc->vc_sw->con_font_set()
> check in con_font_set(), ioctl(VT_RESIZEX) does not pass it in vt_resizex()...
>
> Since this problem does not happen if I remove
>
> if (v.v_clin)
> vcp->vc_font.height = v.v_clin;
Hi Tetsuo!
> from vt_resizex(), I guess that some variables are getting confused by change
> of vc->vc_font.height ...
Yes, see bit_putcs():
(drivers/video/fbdev/core/bitblit.c)
static void bit_putcs(struct vc_data *vc, struct fb_info *info,
const unsigned short *s, int count, int yy, int xx,
int fg, int bg)
{
struct fb_image image;
u32 width = DIV_ROUND_UP(vc->vc_font.width, 8);
u32 cellsize = width * vc->vc_font.height;
^^^^^^^^ ^^^^^^^^^^^^^^
`cellsize` is now too large. Later, in bit_putcs_aligned():
while (cnt--) {
src = vc->vc_font.data + (scr_readw(s++)&
charmask)*cellsize;
^^^^^^^^
`src` goes out of bounds of the data buffer. At first glance I guess
this is an out-of-bound read reported as a use-after-free read? The
crashlog says:
[ 149.732103][ T6693] Allocated by task 6667:
[ 149.732115][ T6693] kasan_save_stack (mm/kasan/common.c:48)
[ 149.732121][ T6693] __kasan_kmalloc.constprop.0 (mm/kasan/common.c:56 mm/kasan/common.c:461)
[ 149.732126][ T6693] __kmalloc (mm/slab.c:3656 mm/slab.c:3664)
[ 149.732133][ T6693] alloc_pipe_info (fs/pipe.c:810)
[ 149.732139][ T6693] create_pipe_files (fs/pipe.c:883 fs/pipe.c:914)
[ 149.732145][ T6693] do_pipe2 (fs/pipe.c:965 fs/pipe.c:1012)
I'm not sure, but I don't think a buffer allocated in fs/pipe.c is
related here. Maybe they just live near each other on the heap?
To resolve this out-of-bound issue for now, I think the easiest way
is to add a range check in bit_putcs(), or bit_putcs_aligned().
...but yeah, that `VT_RESIZEX` ioctl looks really buggy, and is already
causing more issues:
KASAN: global-out-of-bounds Read in fbcon_get_font
Link: https://syzkaller.appspot.com/bug?id\bb8be45afea11888776f897895aef9ad1c3ecfd
This was also caused by `VT_RESIZEX`...
Thank you,
Peilin Ye
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: KASAN: use-after-free Read in bit_putcs
2020-09-26 16:25 ` Tetsuo Handa
(?)
@ 2020-09-27 0:25 ` Tetsuo Handa
-1 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-27 0:25 UTC (permalink / raw)
To: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs, Linus Torvalds
Cc: dri-devel, linux-fbdev, linux-kernel
On 2020/09/27 4:39, Peilin Ye wrote:
> On Sun, Sep 27, 2020 at 01:25:17AM +0900, Tetsuo Handa wrote:
>> Since I don't know the meaning of "struct vt_consize"->v_clin (which is commented
>> with "/* number of pixel rows per character */" but does it mean font size ?),
>> I don't know why we can assign that value to vcp->vc_font.height via
>>
>> if (v.v_clin)
>> vcp->vc_font.height = v.v_clin;
>>
>> in vt_resizex(). While ioctl(PIO_FONT) needs to pass vc->vc_sw->con_font_set()
>> check in con_font_set(), ioctl(VT_RESIZEX) does not pass it in vt_resizex()...
>>
>> Since this problem does not happen if I remove
>>
>> if (v.v_clin)
>> vcp->vc_font.height = v.v_clin;
>
> Hi Tetsuo!
>
>> from vt_resizex(), I guess that some variables are getting confused by change
>> of vc->vc_font.height ...
>
> Yes, see bit_putcs():
>
> (drivers/video/fbdev/core/bitblit.c)
> static void bit_putcs(struct vc_data *vc, struct fb_info *info,
> const unsigned short *s, int count, int yy, int xx,
> int fg, int bg)
> {
> struct fb_image image;
> u32 width = DIV_ROUND_UP(vc->vc_font.width, 8);
> u32 cellsize = width * vc->vc_font.height;
> ^^^^^^^^ ^^^^^^^^^^^^^^
>
> `cellsize` is now too large. Later, in bit_putcs_aligned():
>
> while (cnt--) {
> src = vc->vc_font.data + (scr_readw(s++)&
> charmask)*cellsize;
> ^^^^^^^^
>
> `src` goes out of bounds of the data buffer. At first glance I guess
> this is an out-of-bound read reported as a use-after-free read? The
> crashlog says:
How this OOB access is reported varies.
>
> To resolve this out-of-bound issue for now, I think the easiest way
> is to add a range check in bit_putcs(), or bit_putcs_aligned().
>
> ...but yeah, that `VT_RESIZEX` ioctl looks really buggy, and is already
> causing more issues:
At least, since not all fonts have height == 32 (e.g. font_vga_8x8 is height == 8),
allow changing vc->vc_font.height with only
if (v.v_clin > 32)
return -EINVAL;
validation in VT_RESIZEX looks wrong. This needs more validations.
By the way, can we find a user of VT_RESIZEX? As far as I googled with "VT_RESIZEX",
I couldn't find a userspace program; only explanation of VT_RESIZEX and kernel patches
related to VT_RESIZEX are found. Also, while console_ioctl(4) man page says
Any parameter may be set to zero, indicating "no change"
, the assignment
if (!v.v_vlin)
v.v_vlin = vc->vc_scan_lines;
changes the meaning to
If v_vlin parameter is set to zero, the value for associated console is copied
to each console (instead of preserving current value for that console)
. Maybe for now we can try this (effectively making VT_RESIZEX == VT_RESIZE) ?
vt_ioctl.c | 57 ++++++++++-----------------------------------------------
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520bdd521..bc33938e2f20 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -773,58 +773,21 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
if (copy_from_user(&v, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
-
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
- }
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
- }
- }
-
- if (v.v_clin > 32)
- return -EINVAL;
+ if (v.v_vlin)
+ pr_info_once("\"struct vt_consize\"->v_vlin is ignored. Please report if you need this.\n");
+ if (v.v_clin)
+ pr_info_once("\"struct vt_consize\"->v_clin is ignored. Please report if you need this.\n");
+ console_lock();
for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
+ vc = vc_cons[i].d;
- if (!vc_cons[i].d)
- continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
- vcp->vc_scan_lines = v.v_vlin;
- if (v.v_clin)
- vcp->vc_font.height = v.v_clin;
- vcp->vc_resize_user = 1;
- ret = vc_resize(vcp, v.v_cols, v.v_rows);
- if (ret) {
- vcp->vc_scan_lines = save_scan_lines;
- vcp->vc_font.height = save_font_height;
- console_unlock();
- return ret;
- }
+ if (vc) {
+ vc->vc_resize_user = 1;
+ vc_resize(vc, v.v_cols, v.v_rows);
}
- console_unlock();
}
+ console_unlock();
return 0;
}
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-27 0:25 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-27 0:25 UTC (permalink / raw)
To: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs, Linus Torvalds
Cc: linux-fbdev, linux-kernel, dri-devel
On 2020/09/27 4:39, Peilin Ye wrote:
> On Sun, Sep 27, 2020 at 01:25:17AM +0900, Tetsuo Handa wrote:
>> Since I don't know the meaning of "struct vt_consize"->v_clin (which is commented
>> with "/* number of pixel rows per character */" but does it mean font size ?),
>> I don't know why we can assign that value to vcp->vc_font.height via
>>
>> if (v.v_clin)
>> vcp->vc_font.height = v.v_clin;
>>
>> in vt_resizex(). While ioctl(PIO_FONT) needs to pass vc->vc_sw->con_font_set()
>> check in con_font_set(), ioctl(VT_RESIZEX) does not pass it in vt_resizex()...
>>
>> Since this problem does not happen if I remove
>>
>> if (v.v_clin)
>> vcp->vc_font.height = v.v_clin;
>
> Hi Tetsuo!
>
>> from vt_resizex(), I guess that some variables are getting confused by change
>> of vc->vc_font.height ...
>
> Yes, see bit_putcs():
>
> (drivers/video/fbdev/core/bitblit.c)
> static void bit_putcs(struct vc_data *vc, struct fb_info *info,
> const unsigned short *s, int count, int yy, int xx,
> int fg, int bg)
> {
> struct fb_image image;
> u32 width = DIV_ROUND_UP(vc->vc_font.width, 8);
> u32 cellsize = width * vc->vc_font.height;
> ^^^^^^^^ ^^^^^^^^^^^^^^
>
> `cellsize` is now too large. Later, in bit_putcs_aligned():
>
> while (cnt--) {
> src = vc->vc_font.data + (scr_readw(s++)&
> charmask)*cellsize;
> ^^^^^^^^
>
> `src` goes out of bounds of the data buffer. At first glance I guess
> this is an out-of-bound read reported as a use-after-free read? The
> crashlog says:
How this OOB access is reported varies.
>
> To resolve this out-of-bound issue for now, I think the easiest way
> is to add a range check in bit_putcs(), or bit_putcs_aligned().
>
> ...but yeah, that `VT_RESIZEX` ioctl looks really buggy, and is already
> causing more issues:
At least, since not all fonts have height == 32 (e.g. font_vga_8x8 is height == 8),
allow changing vc->vc_font.height with only
if (v.v_clin > 32)
return -EINVAL;
validation in VT_RESIZEX looks wrong. This needs more validations.
By the way, can we find a user of VT_RESIZEX? As far as I googled with "VT_RESIZEX",
I couldn't find a userspace program; only explanation of VT_RESIZEX and kernel patches
related to VT_RESIZEX are found. Also, while console_ioctl(4) man page says
Any parameter may be set to zero, indicating "no change"
, the assignment
if (!v.v_vlin)
v.v_vlin = vc->vc_scan_lines;
changes the meaning to
If v_vlin parameter is set to zero, the value for associated console is copied
to each console (instead of preserving current value for that console)
. Maybe for now we can try this (effectively making VT_RESIZEX == VT_RESIZE) ?
vt_ioctl.c | 57 ++++++++++-----------------------------------------------
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520bdd521..bc33938e2f20 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -773,58 +773,21 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
if (copy_from_user(&v, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
-
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
- }
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
- }
- }
-
- if (v.v_clin > 32)
- return -EINVAL;
+ if (v.v_vlin)
+ pr_info_once("\"struct vt_consize\"->v_vlin is ignored. Please report if you need this.\n");
+ if (v.v_clin)
+ pr_info_once("\"struct vt_consize\"->v_clin is ignored. Please report if you need this.\n");
+ console_lock();
for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
+ vc = vc_cons[i].d;
- if (!vc_cons[i].d)
- continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
- vcp->vc_scan_lines = v.v_vlin;
- if (v.v_clin)
- vcp->vc_font.height = v.v_clin;
- vcp->vc_resize_user = 1;
- ret = vc_resize(vcp, v.v_cols, v.v_rows);
- if (ret) {
- vcp->vc_scan_lines = save_scan_lines;
- vcp->vc_font.height = save_font_height;
- console_unlock();
- return ret;
- }
+ if (vc) {
+ vc->vc_resize_user = 1;
+ vc_resize(vc, v.v_cols, v.v_rows);
}
- console_unlock();
}
+ console_unlock();
return 0;
}
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-27 0:25 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-27 0:25 UTC (permalink / raw)
To: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs, Linus Torvalds
Cc: linux-fbdev, linux-kernel, dri-devel
On 2020/09/27 4:39, Peilin Ye wrote:
> On Sun, Sep 27, 2020 at 01:25:17AM +0900, Tetsuo Handa wrote:
>> Since I don't know the meaning of "struct vt_consize"->v_clin (which is commented
>> with "/* number of pixel rows per character */" but does it mean font size ?),
>> I don't know why we can assign that value to vcp->vc_font.height via
>>
>> if (v.v_clin)
>> vcp->vc_font.height = v.v_clin;
>>
>> in vt_resizex(). While ioctl(PIO_FONT) needs to pass vc->vc_sw->con_font_set()
>> check in con_font_set(), ioctl(VT_RESIZEX) does not pass it in vt_resizex()...
>>
>> Since this problem does not happen if I remove
>>
>> if (v.v_clin)
>> vcp->vc_font.height = v.v_clin;
>
> Hi Tetsuo!
>
>> from vt_resizex(), I guess that some variables are getting confused by change
>> of vc->vc_font.height ...
>
> Yes, see bit_putcs():
>
> (drivers/video/fbdev/core/bitblit.c)
> static void bit_putcs(struct vc_data *vc, struct fb_info *info,
> const unsigned short *s, int count, int yy, int xx,
> int fg, int bg)
> {
> struct fb_image image;
> u32 width = DIV_ROUND_UP(vc->vc_font.width, 8);
> u32 cellsize = width * vc->vc_font.height;
> ^^^^^^^^ ^^^^^^^^^^^^^^
>
> `cellsize` is now too large. Later, in bit_putcs_aligned():
>
> while (cnt--) {
> src = vc->vc_font.data + (scr_readw(s++)&
> charmask)*cellsize;
> ^^^^^^^^
>
> `src` goes out of bounds of the data buffer. At first glance I guess
> this is an out-of-bound read reported as a use-after-free read? The
> crashlog says:
How this OOB access is reported varies.
>
> To resolve this out-of-bound issue for now, I think the easiest way
> is to add a range check in bit_putcs(), or bit_putcs_aligned().
>
> ...but yeah, that `VT_RESIZEX` ioctl looks really buggy, and is already
> causing more issues:
At least, since not all fonts have height = 32 (e.g. font_vga_8x8 is height = 8),
allow changing vc->vc_font.height with only
if (v.v_clin > 32)
return -EINVAL;
validation in VT_RESIZEX looks wrong. This needs more validations.
By the way, can we find a user of VT_RESIZEX? As far as I googled with "VT_RESIZEX",
I couldn't find a userspace program; only explanation of VT_RESIZEX and kernel patches
related to VT_RESIZEX are found. Also, while console_ioctl(4) man page says
Any parameter may be set to zero, indicating "no change"
, the assignment
if (!v.v_vlin)
v.v_vlin = vc->vc_scan_lines;
changes the meaning to
If v_vlin parameter is set to zero, the value for associated console is copied
to each console (instead of preserving current value for that console)
. Maybe for now we can try this (effectively making VT_RESIZEX = VT_RESIZE) ?
vt_ioctl.c | 57 ++++++++++-----------------------------------------------
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520bdd521..bc33938e2f20 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -773,58 +773,21 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
if (copy_from_user(&v, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
-
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
- }
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
- }
- }
-
- if (v.v_clin > 32)
- return -EINVAL;
+ if (v.v_vlin)
+ pr_info_once("\"struct vt_consize\"->v_vlin is ignored. Please report if you need this.\n");
+ if (v.v_clin)
+ pr_info_once("\"struct vt_consize\"->v_clin is ignored. Please report if you need this.\n");
+ console_lock();
for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
+ vc = vc_cons[i].d;
- if (!vc_cons[i].d)
- continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
- vcp->vc_scan_lines = v.v_vlin;
- if (v.v_clin)
- vcp->vc_font.height = v.v_clin;
- vcp->vc_resize_user = 1;
- ret = vc_resize(vcp, v.v_cols, v.v_rows);
- if (ret) {
- vcp->vc_scan_lines = save_scan_lines;
- vcp->vc_font.height = save_font_height;
- console_unlock();
- return ret;
- }
+ if (vc) {
+ vc->vc_resize_user = 1;
+ vc_resize(vc, v.v_cols, v.v_rows);
}
- console_unlock();
}
+ console_unlock();
return 0;
}
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
2020-09-27 0:25 ` Tetsuo Handa
(?)
@ 2020-09-27 8:28 ` Tetsuo Handa
-1 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-27 8:28 UTC (permalink / raw)
To: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs, Linus Torvalds, Peilin Ye
Cc: dri-devel, linux-fbdev, linux-kernel
Well, vt_io_ioctl(PIO_FONT) initializes "struct console_font_op op;" with
op.width = 8;
op.height = 0;
op.charcount = 256;
and calls con_font_set() from con_font_op(). But the "/* Need to guess font height [compat] */"
chunk in con_font_set() guesses font's height due to being initialized with op.height = 0.
Then, con_font_set() calls fbcon_set_font() via vc->vc_sw->con_font_set(), and fbcon_set_font()
allocates minimal amount of memory for font data based on font's height calcllated by con_font_set().
Therefore, any attempt to change font's height (like vt_resizex()) larger than font's height
calculated by con_font_set() can cause OOB read of memory block for font data. If we allocate
maximal amount of memory for any font, OOB read of memory block for font data should not happen.
----------------------------------------
static char fontdata[8192] = { 2 };
[ 227.065369] bit_putcs: width=1 cellsize=1 count=80 maxcnt=8192 scan_align=0 buf_align=0 image.height=1
[ 227.066254] bit_putcs: width=1 cellsize=1 count=80 maxcnt=8192 scan_align=0 buf_align=0 image.height=1
[ 227.067642] vc=ffff8880d69b4000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
[ 227.067699] vcp=ffff8880d69b4000 before: ->vc_rows=480 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=1
[ 227.067774] vcp=ffff8880d69b4000 after: ->vc_rows=480 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=1 ret=0
[ 227.067831] vcp=ffff8880cac4b000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.067891] vcp=ffff8880cac4b000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.067947] vcp=ffff8880c6180000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.068007] vcp=ffff8880c6180000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.068063] vcp=ffff8880d6b84000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.068123] vcp=ffff8880d6b84000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.068179] vcp=ffff8880ca8c0000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.068255] vcp=ffff8880ca8c0000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.068455] vcp=ffff8880cbd5d000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.068515] vcp=ffff8880cbd5d000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.084709] ==================================================================
[ 227.084729] BUG: KASAN: slab-out-of-bounds in soft_cursor+0x34e/0x4a0
[ 227.084748] Read of size 9 at addr ffff8880c98d5930 by task a.out/1662
[ 227.084786] CPU: 3 PID: 1662 Comm: a.out Not tainted 5.9.0-rc6+ #639
[ 227.084810] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 227.084818] Call Trace:
[ 227.084830] dump_stack+0x161/0x1c3
[ 227.084842] ? soft_cursor+0x34e/0x4a0
[ 227.084854] ? soft_cursor+0x34e/0x4a0
[ 227.084871] print_address_description.constprop.0.cold+0xd3/0x4c5
[ 227.084884] ? lock_is_held_type+0xbf/0xf0
[ 227.084896] ? vprintk_func+0xe2/0x155
[ 227.084908] ? soft_cursor+0x34e/0x4a0
[ 227.084920] ? soft_cursor+0x34e/0x4a0
[ 227.084932] kasan_report.cold+0x1f/0x42
[ 227.084944] ? soft_cursor+0x34e/0x4a0
[ 227.084957] check_memory_region+0x152/0x1b0
[ 227.084967] memcpy+0x24/0x60
[ 227.084979] soft_cursor+0x34e/0x4a0
[ 227.084990] bit_cursor+0x7f6/0xce6
[ 227.085001] ? bit_putcs+0x320/0x320
[ 227.085016] ? fb_get_color_depth.part.0+0x57/0xe0
[ 227.085031] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 227.085042] ? bit_putcs+0x320/0x320
[ 227.085054] fbcon_cursor+0x241/0x2c0
[ 227.085065] hide_cursor+0x58/0x150
[ 227.085077] vt_console_print+0x865/0x870
[ 227.085089] ? lock_release+0x480/0x480
[ 227.085102] ? lock_downgrade+0x3e0/0x3e0
[ 227.085115] ? do_raw_spin_lock+0x110/0x1f0
[ 227.085128] ? screen_glyph_unicode+0x140/0x140
[ 227.085141] ? rwlock_bug.part.0+0x50/0x50
[ 227.085156] ? check_preemption_disabled+0x50/0x130
[ 227.085169] ? screen_glyph_unicode+0x140/0x140
[ 227.085181] console_unlock+0x92c/0xb30
[ 227.085193] vt_ioctl.cold+0x182/0x3a2
[ 227.085207] ? complete_change_console+0x1e0/0x1e0
[ 227.085219] ? find_held_lock+0x85/0xa0
[ 227.085234] ? debug_check_no_obj_freed+0x18d/0x276
[ 227.085246] ? lock_downgrade+0x3e0/0x3e0
[ 227.085258] ? find_held_lock+0x85/0xa0
[ 227.085271] ? lock_is_held_type+0xbf/0xf0
[ 227.085281] ? putname+0xa7/0xc0
[ 227.085292] ? putname+0xa7/0xc0
[ 227.085302] ? putname+0xa7/0xc0
[ 227.085317] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 227.085332] ? complete_change_console+0x1e0/0x1e0
[ 227.085343] ? tty_ioctl+0x7c4/0xec0
[ 227.085368] tty_ioctl+0x7c4/0xec0
[ 227.085383] ? kmem_cache_free.part.0+0x1b0/0x1e0
[ 227.085394] ? tty_vhangup+0x30/0x30
[ 227.085409] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 227.085421] ? do_vfs_ioctl+0x224/0xc50
[ 227.085434] ? ioctl_file_clone+0x140/0x140
[ 227.085449] ? rcu_read_lock_sched_held+0xa0/0xd0
[ 227.085464] ? rcu_read_lock_any_held.part.0+0x30/0x30
[ 227.085479] ? check_preemption_disabled+0x50/0x130
[ 227.085491] ? lock_is_held_type+0xbf/0xf0
[ 227.085506] ? syscall_enter_from_user_mode+0x1c/0x60
[ 227.085520] ? rcu_read_lock_sched_held+0xa0/0xd0
[ 227.085533] ? mark_held_locks+0x24/0x90
[ 227.085544] ? tty_vhangup+0x30/0x30
[ 227.085556] __x64_sys_ioctl+0xec/0x140
[ 227.085568] do_syscall_64+0x31/0x70
[ 227.085583] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 227.085594] RIP: 0033:0x7f261b69e50b
[ 227.085643] 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
[ 227.085658] RSP: 002b:00007fff1894fb98 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 227.085691] RAX: ffffffffffffffda RBX: 0000560076c54220 RCX: 00007f261b69e50b
[ 227.085711] RDX: 00007fff1894fbac RSI: 000000000000560a RDI: 0000000000000003
[ 227.085731] RBP: 0000000000000003 R08: 0000000000000000 R09: 00007f261b794d50
[ 227.085751] R10: 0000000000000000 R11: 0000000000000246 R12: 0000560076c54130
[ 227.085771] R13: 00007fff1894fcb0 R14: 0000000000000000 R15: 0000000000000000
[ 227.085791] Allocated by task 1662:
[ 227.085803] kasan_save_stack+0x1f/0x40
[ 227.085817] __kasan_kmalloc.constprop.0+0xbf/0xd0
[ 227.085828] __kmalloc+0x57d/0x9d0
[ 227.085840] fbcon_set_font+0x1a6/0x4a0
[ 227.085852] con_font_op+0x8e2/0xac0
[ 227.085863] vt_ioctl+0x1186/0x21a0
[ 227.085874] tty_ioctl+0x7c4/0xec0
[ 227.085886] __x64_sys_ioctl+0xec/0x140
[ 227.085898] do_syscall_64+0x31/0x70
[ 227.085913] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 227.085940] The buggy address belongs to the object at ffff8880c98d5800
[ 227.085957] which belongs to the cache kmalloc-512 of size 512
[ 227.085974] The buggy address is located 304 bytes inside of
[ 227.085992] 512-byte region [ffff8880c98d5800, ffff8880c98d5a00)
[ 227.086007] The buggy address belongs to the page:
[ 227.086033] page:00000000022668f3 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0xc98d5
[ 227.086047] flags: 0xfffe0000000200(slab)
[ 227.086081] raw: 00fffe0000000200 ffffea00032c4b08 ffffea0003205748 ffff8880d6840600
[ 227.086104] raw: 0000000000000000 ffff8880c98d5000 0000000100000004 ffff8880d2419241
[ 227.086121] page dumped because: kasan: bad access detected
[ 227.086136] page->mem_cgroup:ffff8880d2419241
[ 227.086158] Memory state around the buggy address:
[ 227.086179] ffff8880c98d5800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 227.086201] ffff8880c98d5880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 227.086222] >ffff8880c98d5900: 00 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 227.086237] ^
[ 227.086258] ffff8880c98d5980: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 227.086279] ffff8880c98d5a00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 227.086301] ==================================================================
----------------------------------------
static char fontdata[8192] = { 2, 3 };
[ 464.415205] vcp=ffff8880d69b4000 before: ->vc_rows=240 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=1 ->vc_font.height=9 save_font_height=2
[ 464.415265] vcp=ffff8880d69b4000 after: ->vc_rows=240 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=1 ->vc_font.height=9 save_font_height=2 ret=0
[ 464.431257] bit_putcs: width=1 cellsize=9 count=68 maxcnt=910 scan_align=0 buf_align=0 image.height=9
[ 464.431279] ==================================================================
[ 464.431299] BUG: KASAN: slab-out-of-bounds in bit_putcs.cold+0x570/0x5aa
[ 464.431319] Read of size 1 at addr ffff8880ca0ccb43 by task a.out/1757
----------------------------------------
static char fontdata[8192] = "0123456789abcdef0123456789abcdef";
[ 300.610119] bit_putcs: width=1 cellsize=32 count=80 maxcnt=256 scan_align=0 buf_align=0 image.height=32
[ 300.630932] bit_putcs: width=1 cellsize=32 count=80 maxcnt=256 scan_align=0 buf_align=0 image.height=32
[ 300.652194] vc=ffff8880d69b4000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
[ 300.652249] vcp=ffff8880d69b4000 before: ->vc_rows=15 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=32
[ 300.652308] vcp=ffff8880d69b4000 after: ->vc_rows=15 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=32 ret=0
[ 300.652500] vcp=ffff8880d55ba000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.652559] vcp=ffff8880d55ba000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.652613] vcp=ffff8880d4d87000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.652690] vcp=ffff8880d4d87000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.652767] vcp=ffff8880d546b000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.652849] vcp=ffff8880d546b000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.652926] vcp=ffff8880c8f85000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.653008] vcp=ffff8880c8f85000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.653085] vcp=ffff8880d55db000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.653167] vcp=ffff8880d55db000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.665421] bit_putcs: width=1 cellsize=9 count=68 maxcnt=910 scan_align=0 buf_align=0 image.height=9
[ 300.665450] bit_putcs: width=1 cellsize=9 count=46 maxcnt=910 scan_align=0 buf_align=0 image.height=9
----------------------------------------
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-27 8:28 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-27 8:28 UTC (permalink / raw)
To: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs, Linus Torvalds, Peilin Ye
Cc: linux-fbdev, linux-kernel, dri-devel
Well, vt_io_ioctl(PIO_FONT) initializes "struct console_font_op op;" with
op.width = 8;
op.height = 0;
op.charcount = 256;
and calls con_font_set() from con_font_op(). But the "/* Need to guess font height [compat] */"
chunk in con_font_set() guesses font's height due to being initialized with op.height = 0.
Then, con_font_set() calls fbcon_set_font() via vc->vc_sw->con_font_set(), and fbcon_set_font()
allocates minimal amount of memory for font data based on font's height calcllated by con_font_set().
Therefore, any attempt to change font's height (like vt_resizex()) larger than font's height
calculated by con_font_set() can cause OOB read of memory block for font data. If we allocate
maximal amount of memory for any font, OOB read of memory block for font data should not happen.
----------------------------------------
static char fontdata[8192] = { 2 };
[ 227.065369] bit_putcs: width=1 cellsize=1 count=80 maxcnt=8192 scan_align=0 buf_align=0 image.height=1
[ 227.066254] bit_putcs: width=1 cellsize=1 count=80 maxcnt=8192 scan_align=0 buf_align=0 image.height=1
[ 227.067642] vc=ffff8880d69b4000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
[ 227.067699] vcp=ffff8880d69b4000 before: ->vc_rows=480 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=1
[ 227.067774] vcp=ffff8880d69b4000 after: ->vc_rows=480 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=1 ret=0
[ 227.067831] vcp=ffff8880cac4b000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.067891] vcp=ffff8880cac4b000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.067947] vcp=ffff8880c6180000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.068007] vcp=ffff8880c6180000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.068063] vcp=ffff8880d6b84000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.068123] vcp=ffff8880d6b84000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.068179] vcp=ffff8880ca8c0000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.068255] vcp=ffff8880ca8c0000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.068455] vcp=ffff8880cbd5d000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 227.068515] vcp=ffff8880cbd5d000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 227.084709] ==================================================================
[ 227.084729] BUG: KASAN: slab-out-of-bounds in soft_cursor+0x34e/0x4a0
[ 227.084748] Read of size 9 at addr ffff8880c98d5930 by task a.out/1662
[ 227.084786] CPU: 3 PID: 1662 Comm: a.out Not tainted 5.9.0-rc6+ #639
[ 227.084810] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 227.084818] Call Trace:
[ 227.084830] dump_stack+0x161/0x1c3
[ 227.084842] ? soft_cursor+0x34e/0x4a0
[ 227.084854] ? soft_cursor+0x34e/0x4a0
[ 227.084871] print_address_description.constprop.0.cold+0xd3/0x4c5
[ 227.084884] ? lock_is_held_type+0xbf/0xf0
[ 227.084896] ? vprintk_func+0xe2/0x155
[ 227.084908] ? soft_cursor+0x34e/0x4a0
[ 227.084920] ? soft_cursor+0x34e/0x4a0
[ 227.084932] kasan_report.cold+0x1f/0x42
[ 227.084944] ? soft_cursor+0x34e/0x4a0
[ 227.084957] check_memory_region+0x152/0x1b0
[ 227.084967] memcpy+0x24/0x60
[ 227.084979] soft_cursor+0x34e/0x4a0
[ 227.084990] bit_cursor+0x7f6/0xce6
[ 227.085001] ? bit_putcs+0x320/0x320
[ 227.085016] ? fb_get_color_depth.part.0+0x57/0xe0
[ 227.085031] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 227.085042] ? bit_putcs+0x320/0x320
[ 227.085054] fbcon_cursor+0x241/0x2c0
[ 227.085065] hide_cursor+0x58/0x150
[ 227.085077] vt_console_print+0x865/0x870
[ 227.085089] ? lock_release+0x480/0x480
[ 227.085102] ? lock_downgrade+0x3e0/0x3e0
[ 227.085115] ? do_raw_spin_lock+0x110/0x1f0
[ 227.085128] ? screen_glyph_unicode+0x140/0x140
[ 227.085141] ? rwlock_bug.part.0+0x50/0x50
[ 227.085156] ? check_preemption_disabled+0x50/0x130
[ 227.085169] ? screen_glyph_unicode+0x140/0x140
[ 227.085181] console_unlock+0x92c/0xb30
[ 227.085193] vt_ioctl.cold+0x182/0x3a2
[ 227.085207] ? complete_change_console+0x1e0/0x1e0
[ 227.085219] ? find_held_lock+0x85/0xa0
[ 227.085234] ? debug_check_no_obj_freed+0x18d/0x276
[ 227.085246] ? lock_downgrade+0x3e0/0x3e0
[ 227.085258] ? find_held_lock+0x85/0xa0
[ 227.085271] ? lock_is_held_type+0xbf/0xf0
[ 227.085281] ? putname+0xa7/0xc0
[ 227.085292] ? putname+0xa7/0xc0
[ 227.085302] ? putname+0xa7/0xc0
[ 227.085317] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 227.085332] ? complete_change_console+0x1e0/0x1e0
[ 227.085343] ? tty_ioctl+0x7c4/0xec0
[ 227.085368] tty_ioctl+0x7c4/0xec0
[ 227.085383] ? kmem_cache_free.part.0+0x1b0/0x1e0
[ 227.085394] ? tty_vhangup+0x30/0x30
[ 227.085409] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 227.085421] ? do_vfs_ioctl+0x224/0xc50
[ 227.085434] ? ioctl_file_clone+0x140/0x140
[ 227.085449] ? rcu_read_lock_sched_held+0xa0/0xd0
[ 227.085464] ? rcu_read_lock_any_held.part.0+0x30/0x30
[ 227.085479] ? check_preemption_disabled+0x50/0x130
[ 227.085491] ? lock_is_held_type+0xbf/0xf0
[ 227.085506] ? syscall_enter_from_user_mode+0x1c/0x60
[ 227.085520] ? rcu_read_lock_sched_held+0xa0/0xd0
[ 227.085533] ? mark_held_locks+0x24/0x90
[ 227.085544] ? tty_vhangup+0x30/0x30
[ 227.085556] __x64_sys_ioctl+0xec/0x140
[ 227.085568] do_syscall_64+0x31/0x70
[ 227.085583] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 227.085594] RIP: 0033:0x7f261b69e50b
[ 227.085643] 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
[ 227.085658] RSP: 002b:00007fff1894fb98 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 227.085691] RAX: ffffffffffffffda RBX: 0000560076c54220 RCX: 00007f261b69e50b
[ 227.085711] RDX: 00007fff1894fbac RSI: 000000000000560a RDI: 0000000000000003
[ 227.085731] RBP: 0000000000000003 R08: 0000000000000000 R09: 00007f261b794d50
[ 227.085751] R10: 0000000000000000 R11: 0000000000000246 R12: 0000560076c54130
[ 227.085771] R13: 00007fff1894fcb0 R14: 0000000000000000 R15: 0000000000000000
[ 227.085791] Allocated by task 1662:
[ 227.085803] kasan_save_stack+0x1f/0x40
[ 227.085817] __kasan_kmalloc.constprop.0+0xbf/0xd0
[ 227.085828] __kmalloc+0x57d/0x9d0
[ 227.085840] fbcon_set_font+0x1a6/0x4a0
[ 227.085852] con_font_op+0x8e2/0xac0
[ 227.085863] vt_ioctl+0x1186/0x21a0
[ 227.085874] tty_ioctl+0x7c4/0xec0
[ 227.085886] __x64_sys_ioctl+0xec/0x140
[ 227.085898] do_syscall_64+0x31/0x70
[ 227.085913] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 227.085940] The buggy address belongs to the object at ffff8880c98d5800
[ 227.085957] which belongs to the cache kmalloc-512 of size 512
[ 227.085974] The buggy address is located 304 bytes inside of
[ 227.085992] 512-byte region [ffff8880c98d5800, ffff8880c98d5a00)
[ 227.086007] The buggy address belongs to the page:
[ 227.086033] page:00000000022668f3 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0xc98d5
[ 227.086047] flags: 0xfffe0000000200(slab)
[ 227.086081] raw: 00fffe0000000200 ffffea00032c4b08 ffffea0003205748 ffff8880d6840600
[ 227.086104] raw: 0000000000000000 ffff8880c98d5000 0000000100000004 ffff8880d2419241
[ 227.086121] page dumped because: kasan: bad access detected
[ 227.086136] page->mem_cgroup:ffff8880d2419241
[ 227.086158] Memory state around the buggy address:
[ 227.086179] ffff8880c98d5800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 227.086201] ffff8880c98d5880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 227.086222] >ffff8880c98d5900: 00 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 227.086237] ^
[ 227.086258] ffff8880c98d5980: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 227.086279] ffff8880c98d5a00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 227.086301] ==================================================================
----------------------------------------
static char fontdata[8192] = { 2, 3 };
[ 464.415205] vcp=ffff8880d69b4000 before: ->vc_rows=240 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=1 ->vc_font.height=9 save_font_height=2
[ 464.415265] vcp=ffff8880d69b4000 after: ->vc_rows=240 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=1 ->vc_font.height=9 save_font_height=2 ret=0
[ 464.431257] bit_putcs: width=1 cellsize=9 count=68 maxcnt=910 scan_align=0 buf_align=0 image.height=9
[ 464.431279] ==================================================================
[ 464.431299] BUG: KASAN: slab-out-of-bounds in bit_putcs.cold+0x570/0x5aa
[ 464.431319] Read of size 1 at addr ffff8880ca0ccb43 by task a.out/1757
----------------------------------------
static char fontdata[8192] = "0123456789abcdef0123456789abcdef";
[ 300.610119] bit_putcs: width=1 cellsize=32 count=80 maxcnt=256 scan_align=0 buf_align=0 image.height=32
[ 300.630932] bit_putcs: width=1 cellsize=32 count=80 maxcnt=256 scan_align=0 buf_align=0 image.height=32
[ 300.652194] vc=ffff8880d69b4000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
[ 300.652249] vcp=ffff8880d69b4000 before: ->vc_rows=15 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=32
[ 300.652308] vcp=ffff8880d69b4000 after: ->vc_rows=15 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=32 ret=0
[ 300.652500] vcp=ffff8880d55ba000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.652559] vcp=ffff8880d55ba000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.652613] vcp=ffff8880d4d87000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.652690] vcp=ffff8880d4d87000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.652767] vcp=ffff8880d546b000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.652849] vcp=ffff8880d546b000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.652926] vcp=ffff8880c8f85000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.653008] vcp=ffff8880c8f85000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.653085] vcp=ffff8880d55db000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
[ 300.653167] vcp=ffff8880d55db000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
[ 300.665421] bit_putcs: width=1 cellsize=9 count=68 maxcnt=910 scan_align=0 buf_align=0 image.height=9
[ 300.665450] bit_putcs: width=1 cellsize=9 count=46 maxcnt=910 scan_align=0 buf_align=0 image.height=9
----------------------------------------
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-27 8:28 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-27 8:28 UTC (permalink / raw)
To: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs, Linus Torvalds, Peilin Ye
Cc: linux-fbdev, linux-kernel, dri-devel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 11531 bytes --]
Well, vt_io_ioctl(PIO_FONT) initializes "struct console_font_op op;" with
op.width = 8;
op.height = 0;
op.charcount = 256;
and calls con_font_set() from con_font_op(). But the "/* Need to guess font height [compat] */"
chunk in con_font_set() guesses font's height due to being initialized with op.height = 0.
Then, con_font_set() calls fbcon_set_font() via vc->vc_sw->con_font_set(), and fbcon_set_font()
allocates minimal amount of memory for font data based on font's height calcllated by con_font_set().
Therefore, any attempt to change font's height (like vt_resizex()) larger than font's height
calculated by con_font_set() can cause OOB read of memory block for font data. If we allocate
maximal amount of memory for any font, OOB read of memory block for font data should not happen.
----------------------------------------
static char fontdata[8192] = { 2 };
[ 227.065369] bit_putcs: width=1 cellsize=1 count€ maxcnt92 scan_align=0 buf_align=0 image.height=1
[ 227.066254] bit_putcs: width=1 cellsize=1 count€ maxcnt92 scan_align=0 buf_align=0 image.height=1
[ 227.067642] vcÿff8880d69b4000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
[ 227.067699] vcpÿff8880d69b4000 before: ->vc_rowsH0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines@0 ->vc_font.height=9 save_font_height=1
[ 227.067774] vcpÿff8880d69b4000 after: ->vc_rowsH0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines@0 ->vc_font.height=9 save_font_height=1 ret=0
[ 227.067831] vcpÿff8880cac4b000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 227.067891] vcpÿff8880cac4b000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 227.067947] vcpÿff8880c6180000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 227.068007] vcpÿff8880c6180000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 227.068063] vcpÿff8880d6b84000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 227.068123] vcpÿff8880d6b84000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 227.068179] vcpÿff8880ca8c0000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 227.068255] vcpÿff8880ca8c0000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 227.068455] vcpÿff8880cbd5d000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 227.068515] vcpÿff8880cbd5d000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 227.084709] =================================
[ 227.084729] BUG: KASAN: slab-out-of-bounds in soft_cursor+0x34e/0x4a0
[ 227.084748] Read of size 9 at addr ffff8880c98d5930 by task a.out/1662
[ 227.084786] CPU: 3 PID: 1662 Comm: a.out Not tainted 5.9.0-rc6+ #639
[ 227.084810] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 227.084818] Call Trace:
[ 227.084830] dump_stack+0x161/0x1c3
[ 227.084842] ? soft_cursor+0x34e/0x4a0
[ 227.084854] ? soft_cursor+0x34e/0x4a0
[ 227.084871] print_address_description.constprop.0.cold+0xd3/0x4c5
[ 227.084884] ? lock_is_held_type+0xbf/0xf0
[ 227.084896] ? vprintk_func+0xe2/0x155
[ 227.084908] ? soft_cursor+0x34e/0x4a0
[ 227.084920] ? soft_cursor+0x34e/0x4a0
[ 227.084932] kasan_report.cold+0x1f/0x42
[ 227.084944] ? soft_cursor+0x34e/0x4a0
[ 227.084957] check_memory_region+0x152/0x1b0
[ 227.084967] memcpy+0x24/0x60
[ 227.084979] soft_cursor+0x34e/0x4a0
[ 227.084990] bit_cursor+0x7f6/0xce6
[ 227.085001] ? bit_putcs+0x320/0x320
[ 227.085016] ? fb_get_color_depth.part.0+0x57/0xe0
[ 227.085031] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 227.085042] ? bit_putcs+0x320/0x320
[ 227.085054] fbcon_cursor+0x241/0x2c0
[ 227.085065] hide_cursor+0x58/0x150
[ 227.085077] vt_console_print+0x865/0x870
[ 227.085089] ? lock_release+0x480/0x480
[ 227.085102] ? lock_downgrade+0x3e0/0x3e0
[ 227.085115] ? do_raw_spin_lock+0x110/0x1f0
[ 227.085128] ? screen_glyph_unicode+0x140/0x140
[ 227.085141] ? rwlock_bug.part.0+0x50/0x50
[ 227.085156] ? check_preemption_disabled+0x50/0x130
[ 227.085169] ? screen_glyph_unicode+0x140/0x140
[ 227.085181] console_unlock+0x92c/0xb30
[ 227.085193] vt_ioctl.cold+0x182/0x3a2
[ 227.085207] ? complete_change_console+0x1e0/0x1e0
[ 227.085219] ? find_held_lock+0x85/0xa0
[ 227.085234] ? debug_check_no_obj_freed+0x18d/0x276
[ 227.085246] ? lock_downgrade+0x3e0/0x3e0
[ 227.085258] ? find_held_lock+0x85/0xa0
[ 227.085271] ? lock_is_held_type+0xbf/0xf0
[ 227.085281] ? putname+0xa7/0xc0
[ 227.085292] ? putname+0xa7/0xc0
[ 227.085302] ? putname+0xa7/0xc0
[ 227.085317] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 227.085332] ? complete_change_console+0x1e0/0x1e0
[ 227.085343] ? tty_ioctl+0x7c4/0xec0
[ 227.085368] tty_ioctl+0x7c4/0xec0
[ 227.085383] ? kmem_cache_free.part.0+0x1b0/0x1e0
[ 227.085394] ? tty_vhangup+0x30/0x30
[ 227.085409] ? __sanitizer_cov_trace_switch+0x49/0x80
[ 227.085421] ? do_vfs_ioctl+0x224/0xc50
[ 227.085434] ? ioctl_file_clone+0x140/0x140
[ 227.085449] ? rcu_read_lock_sched_held+0xa0/0xd0
[ 227.085464] ? rcu_read_lock_any_held.part.0+0x30/0x30
[ 227.085479] ? check_preemption_disabled+0x50/0x130
[ 227.085491] ? lock_is_held_type+0xbf/0xf0
[ 227.085506] ? syscall_enter_from_user_mode+0x1c/0x60
[ 227.085520] ? rcu_read_lock_sched_held+0xa0/0xd0
[ 227.085533] ? mark_held_locks+0x24/0x90
[ 227.085544] ? tty_vhangup+0x30/0x30
[ 227.085556] __x64_sys_ioctl+0xec/0x140
[ 227.085568] do_syscall_64+0x31/0x70
[ 227.085583] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 227.085594] RIP: 0033:0x7f261b69e50b
[ 227.085643] 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
[ 227.085658] RSP: 002b:00007fff1894fb98 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 227.085691] RAX: ffffffffffffffda RBX: 0000560076c54220 RCX: 00007f261b69e50b
[ 227.085711] RDX: 00007fff1894fbac RSI: 000000000000560a RDI: 0000000000000003
[ 227.085731] RBP: 0000000000000003 R08: 0000000000000000 R09: 00007f261b794d50
[ 227.085751] R10: 0000000000000000 R11: 0000000000000246 R12: 0000560076c54130
[ 227.085771] R13: 00007fff1894fcb0 R14: 0000000000000000 R15: 0000000000000000
[ 227.085791] Allocated by task 1662:
[ 227.085803] kasan_save_stack+0x1f/0x40
[ 227.085817] __kasan_kmalloc.constprop.0+0xbf/0xd0
[ 227.085828] __kmalloc+0x57d/0x9d0
[ 227.085840] fbcon_set_font+0x1a6/0x4a0
[ 227.085852] con_font_op+0x8e2/0xac0
[ 227.085863] vt_ioctl+0x1186/0x21a0
[ 227.085874] tty_ioctl+0x7c4/0xec0
[ 227.085886] __x64_sys_ioctl+0xec/0x140
[ 227.085898] do_syscall_64+0x31/0x70
[ 227.085913] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 227.085940] The buggy address belongs to the object at ffff8880c98d5800
[ 227.085957] which belongs to the cache kmalloc-512 of size 512
[ 227.085974] The buggy address is located 304 bytes inside of
[ 227.085992] 512-byte region [ffff8880c98d5800, ffff8880c98d5a00)
[ 227.086007] The buggy address belongs to the page:
[ 227.086033] page:00000000022668f3 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0xc98d5
[ 227.086047] flags: 0xfffe0000000200(slab)
[ 227.086081] raw: 00fffe0000000200 ffffea00032c4b08 ffffea0003205748 ffff8880d6840600
[ 227.086104] raw: 0000000000000000 ffff8880c98d5000 0000000100000004 ffff8880d2419241
[ 227.086121] page dumped because: kasan: bad access detected
[ 227.086136] page->mem_cgroup:ffff8880d2419241
[ 227.086158] Memory state around the buggy address:
[ 227.086179] ffff8880c98d5800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 227.086201] ffff8880c98d5880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 227.086222] >ffff8880c98d5900: 00 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 227.086237] ^
[ 227.086258] ffff8880c98d5980: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 227.086279] ffff8880c98d5a00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 227.086301] =================================
----------------------------------------
static char fontdata[8192] = { 2, 3 };
[ 464.415205] vcpÿff8880d69b4000 before: ->vc_rows$0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=1 ->vc_font.height=9 save_font_height=2
[ 464.415265] vcpÿff8880d69b4000 after: ->vc_rows$0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=1 ->vc_font.height=9 save_font_height=2 ret=0
[ 464.431257] bit_putcs: width=1 cellsize=9 counth maxcnt‘0 scan_align=0 buf_align=0 image.height=9
[ 464.431279] =================================
[ 464.431299] BUG: KASAN: slab-out-of-bounds in bit_putcs.cold+0x570/0x5aa
[ 464.431319] Read of size 1 at addr ffff8880ca0ccb43 by task a.out/1757
----------------------------------------
static char fontdata[8192] = "0123456789abcdef0123456789abcdef";
[ 300.610119] bit_putcs: width=1 cellsize2 count€ maxcnt%6 scan_align=0 buf_align=0 image.height2
[ 300.630932] bit_putcs: width=1 cellsize2 count€ maxcnt%6 scan_align=0 buf_align=0 image.height2
[ 300.652194] vcÿff8880d69b4000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
[ 300.652249] vcpÿff8880d69b4000 before: ->vc_rows\x15 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines@0 ->vc_font.height=9 save_font_height2
[ 300.652308] vcpÿff8880d69b4000 after: ->vc_rows\x15 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines@0 ->vc_font.height=9 save_font_height2 ret=0
[ 300.652500] vcpÿff8880d55ba000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 300.652559] vcpÿff8880d55ba000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 300.652613] vcpÿff8880d4d87000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 300.652690] vcpÿff8880d4d87000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 300.652767] vcpÿff8880d546b000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 300.652849] vcpÿff8880d546b000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 300.652926] vcpÿff8880c8f85000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 300.653008] vcpÿff8880c8f85000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 300.653085] vcpÿff8880d55db000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
[ 300.653167] vcpÿff8880d55db000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
[ 300.665421] bit_putcs: width=1 cellsize=9 counth maxcnt‘0 scan_align=0 buf_align=0 image.height=9
[ 300.665450] bit_putcs: width=1 cellsize=9 countF maxcnt‘0 scan_align=0 buf_align=0 image.height=9
----------------------------------------
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
2020-09-27 8:28 ` Tetsuo Handa
(?)
@ 2020-09-27 9:27 ` Peilin Ye
-1 siblings, 0 replies; 51+ messages in thread
From: Peilin Ye @ 2020-09-27 9:27 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, b.zolnierkie, daniel.vetter, deller, gregkh, jirislaby,
syzkaller-bugs, Linus Torvalds, Peilin Ye, dri-devel,
linux-fbdev, linux-kernel
On Sun, Sep 27, 2020 at 05:28:12PM +0900, Tetsuo Handa wrote:
> Well, vt_io_ioctl(PIO_FONT) initializes "struct console_font_op op;" with
>
> op.width = 8;
> op.height = 0;
> op.charcount = 256;
>
> and calls con_font_set() from con_font_op(). But the "/* Need to guess font height [compat] */"
> chunk in con_font_set() guesses font's height due to being initialized with op.height = 0.
> Then, con_font_set() calls fbcon_set_font() via vc->vc_sw->con_font_set(), and fbcon_set_font()
> allocates minimal amount of memory for font data based on font's height calcllated by con_font_set().
>
> Therefore, any attempt to change font's height (like vt_resizex()) larger than font's height
> calculated by con_font_set() can cause OOB read of memory block for font data. If we allocate
> maximal amount of memory for any font, OOB read of memory block for font data should not happen.
>
> ----------------------------------------
>
> static char fontdata[8192] = { 2 };
>
> [ 227.065369] bit_putcs: width=1 cellsize=1 count=80 maxcnt=8192 scan_align=0 buf_align=0 image.height=1
> [ 227.066254] bit_putcs: width=1 cellsize=1 count=80 maxcnt=8192 scan_align=0 buf_align=0 image.height=1
> [ 227.067642] vc=ffff8880d69b4000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
> [ 227.067699] vcp=ffff8880d69b4000 before: ->vc_rows=480 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=1
> [ 227.067774] vcp=ffff8880d69b4000 after: ->vc_rows=480 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=1 ret=0
> [ 227.067831] vcp=ffff8880cac4b000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.067891] vcp=ffff8880cac4b000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.067947] vcp=ffff8880c6180000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.068007] vcp=ffff8880c6180000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.068063] vcp=ffff8880d6b84000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.068123] vcp=ffff8880d6b84000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.068179] vcp=ffff8880ca8c0000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.068255] vcp=ffff8880ca8c0000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.068455] vcp=ffff8880cbd5d000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.068515] vcp=ffff8880cbd5d000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.084709] ==================================================================
> [ 227.084729] BUG: KASAN: slab-out-of-bounds in soft_cursor+0x34e/0x4a0
> [ 227.084748] Read of size 9 at addr ffff8880c98d5930 by task a.out/1662
Very interesting, I remember seeing this on the syzbot dashboard...
Yes, I guess it is this one:
KASAN: slab-out-of-bounds Read in soft_cursor
https://syzkaller.appspot.com/bug?id=6b8355d27b2b94fb5cedf4655e3a59162d9e48e3
There is a `0x560aul` ioctl() in the reproducer, which is `VT_RESIZEX`.
Thank you,
Peilin Ye
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-27 9:27 ` Peilin Ye
0 siblings, 0 replies; 51+ messages in thread
From: Peilin Ye @ 2020-09-27 9:27 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, b.zolnierkie, daniel.vetter, deller,
syzkaller-bugs, linux-kernel, dri-devel, Linus Torvalds, gregkh,
jirislaby, Peilin Ye
On Sun, Sep 27, 2020 at 05:28:12PM +0900, Tetsuo Handa wrote:
> Well, vt_io_ioctl(PIO_FONT) initializes "struct console_font_op op;" with
>
> op.width = 8;
> op.height = 0;
> op.charcount = 256;
>
> and calls con_font_set() from con_font_op(). But the "/* Need to guess font height [compat] */"
> chunk in con_font_set() guesses font's height due to being initialized with op.height = 0.
> Then, con_font_set() calls fbcon_set_font() via vc->vc_sw->con_font_set(), and fbcon_set_font()
> allocates minimal amount of memory for font data based on font's height calcllated by con_font_set().
>
> Therefore, any attempt to change font's height (like vt_resizex()) larger than font's height
> calculated by con_font_set() can cause OOB read of memory block for font data. If we allocate
> maximal amount of memory for any font, OOB read of memory block for font data should not happen.
>
> ----------------------------------------
>
> static char fontdata[8192] = { 2 };
>
> [ 227.065369] bit_putcs: width=1 cellsize=1 count=80 maxcnt=8192 scan_align=0 buf_align=0 image.height=1
> [ 227.066254] bit_putcs: width=1 cellsize=1 count=80 maxcnt=8192 scan_align=0 buf_align=0 image.height=1
> [ 227.067642] vc=ffff8880d69b4000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
> [ 227.067699] vcp=ffff8880d69b4000 before: ->vc_rows=480 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=1
> [ 227.067774] vcp=ffff8880d69b4000 after: ->vc_rows=480 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=400 ->vc_font.height=9 save_font_height=1 ret=0
> [ 227.067831] vcp=ffff8880cac4b000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.067891] vcp=ffff8880cac4b000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.067947] vcp=ffff8880c6180000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.068007] vcp=ffff8880c6180000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.068063] vcp=ffff8880d6b84000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.068123] vcp=ffff8880d6b84000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.068179] vcp=ffff8880ca8c0000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.068255] vcp=ffff8880ca8c0000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.068455] vcp=ffff8880cbd5d000 before: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16
> [ 227.068515] vcp=ffff8880cbd5d000 after: ->vc_rows=30 ->vc_cols=80 ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height=16 ret=0
> [ 227.084709] ==================================================================
> [ 227.084729] BUG: KASAN: slab-out-of-bounds in soft_cursor+0x34e/0x4a0
> [ 227.084748] Read of size 9 at addr ffff8880c98d5930 by task a.out/1662
Very interesting, I remember seeing this on the syzbot dashboard...
Yes, I guess it is this one:
KASAN: slab-out-of-bounds Read in soft_cursor
https://syzkaller.appspot.com/bug?id=6b8355d27b2b94fb5cedf4655e3a59162d9e48e3
There is a `0x560aul` ioctl() in the reproducer, which is `VT_RESIZEX`.
Thank you,
Peilin Ye
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: KASAN: use-after-free Read in bit_putcs
@ 2020-09-27 9:27 ` Peilin Ye
0 siblings, 0 replies; 51+ messages in thread
From: Peilin Ye @ 2020-09-27 9:27 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, b.zolnierkie, daniel.vetter, deller,
syzkaller-bugs, linux-kernel, dri-devel, Linus Torvalds, gregkh,
jirislaby, Peilin Ye
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 3518 bytes --]
On Sun, Sep 27, 2020 at 05:28:12PM +0900, Tetsuo Handa wrote:
> Well, vt_io_ioctl(PIO_FONT) initializes "struct console_font_op op;" with
>
> op.width = 8;
> op.height = 0;
> op.charcount = 256;
>
> and calls con_font_set() from con_font_op(). But the "/* Need to guess font height [compat] */"
> chunk in con_font_set() guesses font's height due to being initialized with op.height = 0.
> Then, con_font_set() calls fbcon_set_font() via vc->vc_sw->con_font_set(), and fbcon_set_font()
> allocates minimal amount of memory for font data based on font's height calcllated by con_font_set().
>
> Therefore, any attempt to change font's height (like vt_resizex()) larger than font's height
> calculated by con_font_set() can cause OOB read of memory block for font data. If we allocate
> maximal amount of memory for any font, OOB read of memory block for font data should not happen.
>
> ----------------------------------------
>
> static char fontdata[8192] = { 2 };
>
> [ 227.065369] bit_putcs: width=1 cellsize=1 count€ maxcnt92 scan_align=0 buf_align=0 image.height=1
> [ 227.066254] bit_putcs: width=1 cellsize=1 count€ maxcnt92 scan_align=0 buf_align=0 image.height=1
> [ 227.067642] vcÿff8880d69b4000 v.v_rows=0 v.v_cols=0 v.v_vlin=1 v.v_clin=9 v.v_vcol=0 v.v_ccol=0 ret=0
> [ 227.067699] vcpÿff8880d69b4000 before: ->vc_rowsH0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines@0 ->vc_font.height=9 save_font_height=1
> [ 227.067774] vcpÿff8880d69b4000 after: ->vc_rowsH0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines@0 ->vc_font.height=9 save_font_height=1 ret=0
> [ 227.067831] vcpÿff8880cac4b000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
> [ 227.067891] vcpÿff8880cac4b000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
> [ 227.067947] vcpÿff8880c6180000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
> [ 227.068007] vcpÿff8880c6180000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
> [ 227.068063] vcpÿff8880d6b84000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
> [ 227.068123] vcpÿff8880d6b84000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
> [ 227.068179] vcpÿff8880ca8c0000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
> [ 227.068255] vcpÿff8880ca8c0000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
> [ 227.068455] vcpÿff8880cbd5d000 before: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16
> [ 227.068515] vcpÿff8880cbd5d000 after: ->vc_rows0 ->vc_cols€ ->vc_scan_lines=1 save_scan_lines=0 ->vc_font.height=9 save_font_height\x16 ret=0
> [ 227.084709] =================================
> [ 227.084729] BUG: KASAN: slab-out-of-bounds in soft_cursor+0x34e/0x4a0
> [ 227.084748] Read of size 9 at addr ffff8880c98d5930 by task a.out/1662
Very interesting, I remember seeing this on the syzbot dashboard...
Yes, I guess it is this one:
KASAN: slab-out-of-bounds Read in soft_cursor
https://syzkaller.appspot.com/bug?idk8355d27b2b94fb5cedf4655e3a59162d9e48e3
There is a `0x560aul` ioctl() in the reproducer, which is `VT_RESIZEX`.
Thank you,
Peilin Ye
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2020-09-27 9:27 ` Peilin Ye
(?)
@ 2020-09-27 11:46 ` Tetsuo Handa
-1 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-27 11:46 UTC (permalink / raw)
To: gregkh, jirislaby
Cc: Peilin Ye, syzbot, b.zolnierkie, daniel.vetter, deller,
syzkaller-bugs, Linus Torvalds, dri-devel, linux-fbdev,
linux-kernel, George Kennedy
syzbot is reporting UAF/OOB read at bit_putcs()/soft_cursor() [1][2], for
vt_resizex() from ioctl(VT_RESIZEX) allows setting font height larger than
actual font height calculated by con_font_set() from ioctl(PIO_FONT).
Since fbcon_set_font() from con_font_set() allocates minimal amount of
memory based on actual font height calculated by con_font_set(),
use of vt_resizex() can cause UAF/OOB read for font data.
VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
#define VT_RESIZE 0x5609 /* set kernel's idea of screensize */
#define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */
So far we are not aware of syzbot reports caused by setting non-zero value
to v_vlin parameter. But given that it is possible that nobody is using
VT_RESIZEX, we can try removing support for v_clin and v_vlin parameters.
Therefore, this patch effectively makes VT_RESIZEX behave like VT_RESIZE,
with emitting a message if somebody is still using v_clin and/or v_vlin
parameters.
[1] https://syzkaller.appspot.com/bug?id=32577e96d88447ded2d3b76d71254fb855245837
[2] https://syzkaller.appspot.com/bug?id=6b8355d27b2b94fb5cedf4655e3a59162d9e48e3
Reported-by: syzbot <syzbot+b308f5fd049fbbc6e74f@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+16469b5e8e5a72e9131e@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
drivers/tty/vt/vt_ioctl.c | 57 +++++++--------------------------------
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520bdd521..bc33938e2f20 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -773,58 +773,21 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
if (copy_from_user(&v, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
-
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
- }
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
- }
- }
-
- if (v.v_clin > 32)
- return -EINVAL;
+ if (v.v_vlin)
+ pr_info_once("\"struct vt_consize\"->v_vlin is ignored. Please report if you need this.\n");
+ if (v.v_clin)
+ pr_info_once("\"struct vt_consize\"->v_clin is ignored. Please report if you need this.\n");
+ console_lock();
for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
+ vc = vc_cons[i].d;
- if (!vc_cons[i].d)
- continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
- vcp->vc_scan_lines = v.v_vlin;
- if (v.v_clin)
- vcp->vc_font.height = v.v_clin;
- vcp->vc_resize_user = 1;
- ret = vc_resize(vcp, v.v_cols, v.v_rows);
- if (ret) {
- vcp->vc_scan_lines = save_scan_lines;
- vcp->vc_font.height = save_font_height;
- console_unlock();
- return ret;
- }
+ if (vc) {
+ vc->vc_resize_user = 1;
+ vc_resize(vc, v.v_cols, v.v_rows);
}
- console_unlock();
}
+ console_unlock();
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-27 11:46 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-27 11:46 UTC (permalink / raw)
To: gregkh, jirislaby
Cc: syzbot, linux-fbdev, b.zolnierkie, daniel.vetter, deller,
syzkaller-bugs, linux-kernel, dri-devel, George Kennedy,
Linus Torvalds, Peilin Ye
syzbot is reporting UAF/OOB read at bit_putcs()/soft_cursor() [1][2], for
vt_resizex() from ioctl(VT_RESIZEX) allows setting font height larger than
actual font height calculated by con_font_set() from ioctl(PIO_FONT).
Since fbcon_set_font() from con_font_set() allocates minimal amount of
memory based on actual font height calculated by con_font_set(),
use of vt_resizex() can cause UAF/OOB read for font data.
VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
#define VT_RESIZE 0x5609 /* set kernel's idea of screensize */
#define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */
So far we are not aware of syzbot reports caused by setting non-zero value
to v_vlin parameter. But given that it is possible that nobody is using
VT_RESIZEX, we can try removing support for v_clin and v_vlin parameters.
Therefore, this patch effectively makes VT_RESIZEX behave like VT_RESIZE,
with emitting a message if somebody is still using v_clin and/or v_vlin
parameters.
[1] https://syzkaller.appspot.com/bug?id=32577e96d88447ded2d3b76d71254fb855245837
[2] https://syzkaller.appspot.com/bug?id=6b8355d27b2b94fb5cedf4655e3a59162d9e48e3
Reported-by: syzbot <syzbot+b308f5fd049fbbc6e74f@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+16469b5e8e5a72e9131e@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
drivers/tty/vt/vt_ioctl.c | 57 +++++++--------------------------------
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520bdd521..bc33938e2f20 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -773,58 +773,21 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
if (copy_from_user(&v, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
-
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
- }
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
- }
- }
-
- if (v.v_clin > 32)
- return -EINVAL;
+ if (v.v_vlin)
+ pr_info_once("\"struct vt_consize\"->v_vlin is ignored. Please report if you need this.\n");
+ if (v.v_clin)
+ pr_info_once("\"struct vt_consize\"->v_clin is ignored. Please report if you need this.\n");
+ console_lock();
for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
+ vc = vc_cons[i].d;
- if (!vc_cons[i].d)
- continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
- vcp->vc_scan_lines = v.v_vlin;
- if (v.v_clin)
- vcp->vc_font.height = v.v_clin;
- vcp->vc_resize_user = 1;
- ret = vc_resize(vcp, v.v_cols, v.v_rows);
- if (ret) {
- vcp->vc_scan_lines = save_scan_lines;
- vcp->vc_font.height = save_font_height;
- console_unlock();
- return ret;
- }
+ if (vc) {
+ vc->vc_resize_user = 1;
+ vc_resize(vc, v.v_cols, v.v_rows);
}
- console_unlock();
}
+ console_unlock();
return 0;
}
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-27 11:46 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-27 11:46 UTC (permalink / raw)
To: gregkh, jirislaby
Cc: syzbot, linux-fbdev, b.zolnierkie, daniel.vetter, deller,
syzkaller-bugs, linux-kernel, dri-devel, George Kennedy,
Linus Torvalds, Peilin Ye
syzbot is reporting UAF/OOB read at bit_putcs()/soft_cursor() [1][2], for
vt_resizex() from ioctl(VT_RESIZEX) allows setting font height larger than
actual font height calculated by con_font_set() from ioctl(PIO_FONT).
Since fbcon_set_font() from con_font_set() allocates minimal amount of
memory based on actual font height calculated by con_font_set(),
use of vt_resizex() can cause UAF/OOB read for font data.
VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
#define VT_RESIZE 0x5609 /* set kernel's idea of screensize */
#define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */
So far we are not aware of syzbot reports caused by setting non-zero value
to v_vlin parameter. But given that it is possible that nobody is using
VT_RESIZEX, we can try removing support for v_clin and v_vlin parameters.
Therefore, this patch effectively makes VT_RESIZEX behave like VT_RESIZE,
with emitting a message if somebody is still using v_clin and/or v_vlin
parameters.
[1] https://syzkaller.appspot.com/bug?id2577e96d88447ded2d3b76d71254fb855245837
[2] https://syzkaller.appspot.com/bug?idk8355d27b2b94fb5cedf4655e3a59162d9e48e3
Reported-by: syzbot <syzbot+b308f5fd049fbbc6e74f@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+16469b5e8e5a72e9131e@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
drivers/tty/vt/vt_ioctl.c | 57 +++++++--------------------------------
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520bdd521..bc33938e2f20 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -773,58 +773,21 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
if (copy_from_user(&v, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
-
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
- }
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
- }
- }
-
- if (v.v_clin > 32)
- return -EINVAL;
+ if (v.v_vlin)
+ pr_info_once("\"struct vt_consize\"->v_vlin is ignored. Please report if you need this.\n");
+ if (v.v_clin)
+ pr_info_once("\"struct vt_consize\"->v_clin is ignored. Please report if you need this.\n");
+ console_lock();
for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
+ vc = vc_cons[i].d;
- if (!vc_cons[i].d)
- continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
- vcp->vc_scan_lines = v.v_vlin;
- if (v.v_clin)
- vcp->vc_font.height = v.v_clin;
- vcp->vc_resize_user = 1;
- ret = vc_resize(vcp, v.v_cols, v.v_rows);
- if (ret) {
- vcp->vc_scan_lines = save_scan_lines;
- vcp->vc_font.height = save_font_height;
- console_unlock();
- return ret;
- }
+ if (vc) {
+ vc->vc_resize_user = 1;
+ vc_resize(vc, v.v_cols, v.v_rows);
}
- console_unlock();
}
+ console_unlock();
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2020-09-27 11:46 ` Tetsuo Handa
(?)
@ 2020-09-27 12:06 ` Greg KH
-1 siblings, 0 replies; 51+ messages in thread
From: Greg KH @ 2020-09-27 12:06 UTC (permalink / raw)
To: Tetsuo Handa
Cc: jirislaby, Peilin Ye, syzbot, b.zolnierkie, daniel.vetter,
deller, syzkaller-bugs, Linus Torvalds, dri-devel, linux-fbdev,
linux-kernel, George Kennedy
On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> syzbot is reporting UAF/OOB read at bit_putcs()/soft_cursor() [1][2], for
> vt_resizex() from ioctl(VT_RESIZEX) allows setting font height larger than
> actual font height calculated by con_font_set() from ioctl(PIO_FONT).
> Since fbcon_set_font() from con_font_set() allocates minimal amount of
> memory based on actual font height calculated by con_font_set(),
> use of vt_resizex() can cause UAF/OOB read for font data.
>
> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
>
> #define VT_RESIZE 0x5609 /* set kernel's idea of screensize */
> #define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */
>
> So far we are not aware of syzbot reports caused by setting non-zero value
> to v_vlin parameter. But given that it is possible that nobody is using
> VT_RESIZEX, we can try removing support for v_clin and v_vlin parameters.
Debian code search doesn't show any users, and that's usually a good
indication of what userspace ioctls for old things like this, are being
used for.
So this makes sense to me, I'll queue it up, thanks!
greg k-h
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-27 12:06 ` Greg KH
0 siblings, 0 replies; 51+ messages in thread
From: Greg KH @ 2020-09-27 12:06 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, daniel.vetter,
deller, syzkaller-bugs, linux-kernel, dri-devel, Linus Torvalds,
jirislaby, Peilin Ye
On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> syzbot is reporting UAF/OOB read at bit_putcs()/soft_cursor() [1][2], for
> vt_resizex() from ioctl(VT_RESIZEX) allows setting font height larger than
> actual font height calculated by con_font_set() from ioctl(PIO_FONT).
> Since fbcon_set_font() from con_font_set() allocates minimal amount of
> memory based on actual font height calculated by con_font_set(),
> use of vt_resizex() can cause UAF/OOB read for font data.
>
> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
>
> #define VT_RESIZE 0x5609 /* set kernel's idea of screensize */
> #define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */
>
> So far we are not aware of syzbot reports caused by setting non-zero value
> to v_vlin parameter. But given that it is possible that nobody is using
> VT_RESIZEX, we can try removing support for v_clin and v_vlin parameters.
Debian code search doesn't show any users, and that's usually a good
indication of what userspace ioctls for old things like this, are being
used for.
So this makes sense to me, I'll queue it up, thanks!
greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-27 12:06 ` Greg KH
0 siblings, 0 replies; 51+ messages in thread
From: Greg KH @ 2020-09-27 12:06 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, daniel.vetter,
deller, syzkaller-bugs, linux-kernel, dri-devel, Linus Torvalds,
jirislaby, Peilin Ye
On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> syzbot is reporting UAF/OOB read at bit_putcs()/soft_cursor() [1][2], for
> vt_resizex() from ioctl(VT_RESIZEX) allows setting font height larger than
> actual font height calculated by con_font_set() from ioctl(PIO_FONT).
> Since fbcon_set_font() from con_font_set() allocates minimal amount of
> memory based on actual font height calculated by con_font_set(),
> use of vt_resizex() can cause UAF/OOB read for font data.
>
> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
>
> #define VT_RESIZE 0x5609 /* set kernel's idea of screensize */
> #define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */
>
> So far we are not aware of syzbot reports caused by setting non-zero value
> to v_vlin parameter. But given that it is possible that nobody is using
> VT_RESIZEX, we can try removing support for v_clin and v_vlin parameters.
Debian code search doesn't show any users, and that's usually a good
indication of what userspace ioctls for old things like this, are being
used for.
So this makes sense to me, I'll queue it up, thanks!
greg k-h
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2020-09-27 11:46 ` Tetsuo Handa
(?)
@ 2020-09-28 17:59 ` Martin Hostettler
-1 siblings, 0 replies; 51+ messages in thread
From: Martin Hostettler @ 2020-09-28 17:59 UTC (permalink / raw)
To: Tetsuo Handa
Cc: gregkh, jirislaby, Peilin Ye, syzbot, b.zolnierkie,
daniel.vetter, deller, syzkaller-bugs, Linus Torvalds, dri-devel,
linux-fbdev, linux-kernel, George Kennedy
On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
>
It seems this is/was used by "svgatextmode" which seems to be at
http://www.ibiblio.org/pub/Linux/utils/console/
Not sure if that kind of software still has a chance to work nowadays.
- Martin Hostettler
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-28 17:59 ` Martin Hostettler
0 siblings, 0 replies; 51+ messages in thread
From: Martin Hostettler @ 2020-09-28 17:59 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, daniel.vetter,
deller, syzkaller-bugs, linux-kernel, dri-devel, Linus Torvalds,
gregkh, jirislaby, Peilin Ye
On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
>
It seems this is/was used by "svgatextmode" which seems to be at
http://www.ibiblio.org/pub/Linux/utils/console/
Not sure if that kind of software still has a chance to work nowadays.
- Martin Hostettler
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-28 17:59 ` Martin Hostettler
0 siblings, 0 replies; 51+ messages in thread
From: Martin Hostettler @ 2020-09-28 17:59 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, daniel.vetter,
deller, syzkaller-bugs, linux-kernel, dri-devel, Linus Torvalds,
gregkh, jirislaby, Peilin Ye
On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
>
It seems this is/was used by "svgatextmode" which seems to be at
http://www.ibiblio.org/pub/Linux/utils/console/
Not sure if that kind of software still has a chance to work nowadays.
- Martin Hostettler
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2020-09-28 17:59 ` Martin Hostettler
(?)
@ 2020-09-29 1:12 ` Tetsuo Handa
-1 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-29 1:12 UTC (permalink / raw)
To: Martin Hostettler
Cc: gregkh, jirislaby, Peilin Ye, syzbot, b.zolnierkie,
daniel.vetter, deller, syzkaller-bugs, Linus Torvalds, dri-devel,
linux-fbdev, linux-kernel, George Kennedy
On 2020/09/29 2:59, Martin Hostettler wrote:
> On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
>> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
>> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
>>
>
> It seems this is/was used by "svgatextmode" which seems to be at
> http://www.ibiblio.org/pub/Linux/utils/console/
>
> Not sure if that kind of software still has a chance to work nowadays.
>
Thanks for the information.
It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
---------- SVGATextMode-1.10/SVGATextMode.c ----------
/*
* Resize the screen. Still needs LOTS more error checking to avoid dropping out in the middle, leaving
* the user with a garbled screen.
*
* sresize will be TRUE when resizing tty's should be forced (due to the 2nd attempt do_VT_RESIZE will do
* when not enough memory is free).
*
*/
/*
* ALWAYS do a VT_RESIZE, even if we already did a VT_RESIZEX on a 1.3.3 or higher kernel,
* until those kernel programmers make this unambiguous
*/
if (do_VT_RESIZE(curr_textmode->cols, curr_textmode->rows, resize1x1)) sresize=TRUE;
if (check_kernel_version(1,3,3, "VT_RESIZEX"))
{
/*
* VDisplay must de divided by 2 for DoubleScan modes,
* or VT_RESIZEX will fail -- until someone fixes the kernel
* so it understands about doublescan modes.
*/
if (do_VT_RESIZEX(curr_textmode->cols,
curr_textmode->rows,
curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1),
curr_textmode->FontHeight,
curr_textmode->HDisplay/8*curr_textmode->FontWidth,
curr_textmode->FontWidth, resize1x1)) sresize=TRUE;
}
---------- SVGATextMode-1.10/ttyresize.c ----------
/*
* if VT_RESIZEX not supported (i.e. when compiling on < 1.3.3 kernels), define it.
* this is just te keep the compiler happy
*/
#ifndef VT_RESIZEX
# define VT_RESIZEX 0x560A
typedef struct vt_consize {
ushort v_rows; ushort v_cols; ushort v_vlin; ushort v_clin; ushort v_vcol; ushort v_ccol;
} vt_consize;
#endif
int do_VT_RESIZEX(int cols, int rows, int vlin, int clin, int vcol, int ccol, int allow1x1)
{
struct vt_consize my_vt_size; /* passes the new screen size on to the kernel */
struct vt_consize dummy_vt_size = { 1 , 1 , 1 , 1 , 1 , 1 };
int ram_needed = cols * rows * 2 * MAX_NR_CONSOLES;
my_vt_size.v_rows = rows;
my_vt_size.v_cols = cols;
my_vt_size.v_vlin = vlin;
my_vt_size.v_clin = clin;
my_vt_size.v_vcol = vcol;
my_vt_size.v_ccol = ccol;
PDEBUG(("VT_RESIZEX(cols=%d,rows=%d,vlin=%d,clin=%d,vcol=%d,ccol=%d)\n",cols, rows, vlin, clin, vcol, ccol));
return(generic_VT_RESIZE(&my_vt_size, &dummy_vt_size, allow1x1, ram_needed, VT_RESIZEX, "VT_RESIZEX"));
}
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-29 1:12 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-29 1:12 UTC (permalink / raw)
To: Martin Hostettler
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, daniel.vetter,
deller, syzkaller-bugs, linux-kernel, dri-devel, Linus Torvalds,
gregkh, jirislaby, Peilin Ye
On 2020/09/29 2:59, Martin Hostettler wrote:
> On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
>> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
>> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
>>
>
> It seems this is/was used by "svgatextmode" which seems to be at
> http://www.ibiblio.org/pub/Linux/utils/console/
>
> Not sure if that kind of software still has a chance to work nowadays.
>
Thanks for the information.
It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
---------- SVGATextMode-1.10/SVGATextMode.c ----------
/*
* Resize the screen. Still needs LOTS more error checking to avoid dropping out in the middle, leaving
* the user with a garbled screen.
*
* sresize will be TRUE when resizing tty's should be forced (due to the 2nd attempt do_VT_RESIZE will do
* when not enough memory is free).
*
*/
/*
* ALWAYS do a VT_RESIZE, even if we already did a VT_RESIZEX on a 1.3.3 or higher kernel,
* until those kernel programmers make this unambiguous
*/
if (do_VT_RESIZE(curr_textmode->cols, curr_textmode->rows, resize1x1)) sresize=TRUE;
if (check_kernel_version(1,3,3, "VT_RESIZEX"))
{
/*
* VDisplay must de divided by 2 for DoubleScan modes,
* or VT_RESIZEX will fail -- until someone fixes the kernel
* so it understands about doublescan modes.
*/
if (do_VT_RESIZEX(curr_textmode->cols,
curr_textmode->rows,
curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1),
curr_textmode->FontHeight,
curr_textmode->HDisplay/8*curr_textmode->FontWidth,
curr_textmode->FontWidth, resize1x1)) sresize=TRUE;
}
---------- SVGATextMode-1.10/ttyresize.c ----------
/*
* if VT_RESIZEX not supported (i.e. when compiling on < 1.3.3 kernels), define it.
* this is just te keep the compiler happy
*/
#ifndef VT_RESIZEX
# define VT_RESIZEX 0x560A
typedef struct vt_consize {
ushort v_rows; ushort v_cols; ushort v_vlin; ushort v_clin; ushort v_vcol; ushort v_ccol;
} vt_consize;
#endif
int do_VT_RESIZEX(int cols, int rows, int vlin, int clin, int vcol, int ccol, int allow1x1)
{
struct vt_consize my_vt_size; /* passes the new screen size on to the kernel */
struct vt_consize dummy_vt_size = { 1 , 1 , 1 , 1 , 1 , 1 };
int ram_needed = cols * rows * 2 * MAX_NR_CONSOLES;
my_vt_size.v_rows = rows;
my_vt_size.v_cols = cols;
my_vt_size.v_vlin = vlin;
my_vt_size.v_clin = clin;
my_vt_size.v_vcol = vcol;
my_vt_size.v_ccol = ccol;
PDEBUG(("VT_RESIZEX(cols=%d,rows=%d,vlin=%d,clin=%d,vcol=%d,ccol=%d)\n",cols, rows, vlin, clin, vcol, ccol));
return(generic_VT_RESIZE(&my_vt_size, &dummy_vt_size, allow1x1, ram_needed, VT_RESIZEX, "VT_RESIZEX"));
}
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-29 1:12 ` Tetsuo Handa
0 siblings, 0 replies; 51+ messages in thread
From: Tetsuo Handa @ 2020-09-29 1:12 UTC (permalink / raw)
To: Martin Hostettler
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, daniel.vetter,
deller, syzkaller-bugs, linux-kernel, dri-devel, Linus Torvalds,
gregkh, jirislaby, Peilin Ye
On 2020/09/29 2:59, Martin Hostettler wrote:
> On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
>> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
>> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
>>
>
> It seems this is/was used by "svgatextmode" which seems to be at
> http://www.ibiblio.org/pub/Linux/utils/console/
>
> Not sure if that kind of software still has a chance to work nowadays.
>
Thanks for the information.
It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
---------- SVGATextMode-1.10/SVGATextMode.c ----------
/*
* Resize the screen. Still needs LOTS more error checking to avoid dropping out in the middle, leaving
* the user with a garbled screen.
*
* sresize will be TRUE when resizing tty's should be forced (due to the 2nd attempt do_VT_RESIZE will do
* when not enough memory is free).
*
*/
/*
* ALWAYS do a VT_RESIZE, even if we already did a VT_RESIZEX on a 1.3.3 or higher kernel,
* until those kernel programmers make this unambiguous
*/
if (do_VT_RESIZE(curr_textmode->cols, curr_textmode->rows, resize1x1)) sresize=TRUE;
if (check_kernel_version(1,3,3, "VT_RESIZEX"))
{
/*
* VDisplay must de divided by 2 for DoubleScan modes,
* or VT_RESIZEX will fail -- until someone fixes the kernel
* so it understands about doublescan modes.
*/
if (do_VT_RESIZEX(curr_textmode->cols,
curr_textmode->rows,
curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1),
curr_textmode->FontHeight,
curr_textmode->HDisplay/8*curr_textmode->FontWidth,
curr_textmode->FontWidth, resize1x1)) sresize=TRUE;
}
---------- SVGATextMode-1.10/ttyresize.c ----------
/*
* if VT_RESIZEX not supported (i.e. when compiling on < 1.3.3 kernels), define it.
* this is just te keep the compiler happy
*/
#ifndef VT_RESIZEX
# define VT_RESIZEX 0x560A
typedef struct vt_consize {
ushort v_rows; ushort v_cols; ushort v_vlin; ushort v_clin; ushort v_vcol; ushort v_ccol;
} vt_consize;
#endif
int do_VT_RESIZEX(int cols, int rows, int vlin, int clin, int vcol, int ccol, int allow1x1)
{
struct vt_consize my_vt_size; /* passes the new screen size on to the kernel */
struct vt_consize dummy_vt_size = { 1 , 1 , 1 , 1 , 1 , 1 };
int ram_needed = cols * rows * 2 * MAX_NR_CONSOLES;
my_vt_size.v_rows = rows;
my_vt_size.v_cols = cols;
my_vt_size.v_vlin = vlin;
my_vt_size.v_clin = clin;
my_vt_size.v_vcol = vcol;
my_vt_size.v_ccol = ccol;
PDEBUG(("VT_RESIZEX(cols=%d,rows=%d,vlin=%d,clin=%d,vcol=%d,ccol=%d)\n",cols, rows, vlin, clin, vcol, ccol));
return(generic_VT_RESIZE(&my_vt_size, &dummy_vt_size, allow1x1, ram_needed, VT_RESIZEX, "VT_RESIZEX"));
}
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2020-09-29 1:12 ` Tetsuo Handa
(?)
@ 2020-09-29 10:52 ` Martin Hostettler
-1 siblings, 0 replies; 51+ messages in thread
From: Martin Hostettler @ 2020-09-29 10:52 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Martin Hostettler, gregkh, jirislaby, Peilin Ye, syzbot,
b.zolnierkie, daniel.vetter, deller, syzkaller-bugs,
Linus Torvalds, dri-devel, linux-fbdev, linux-kernel,
George Kennedy
On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote:
> On 2020/09/29 2:59, Martin Hostettler wrote:
> > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> >>
> >
> > It seems this is/was used by "svgatextmode" which seems to be at
> > http://www.ibiblio.org/pub/Linux/utils/console/
> >
> > Not sure if that kind of software still has a chance to work nowadays.
> >
>
> Thanks for the information.
>
> It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
>
Yes, this seems to be from pre framebuffer times.
Back in the days "svga" was the wording you got for "pokes svga card
hardware registers from userspace drivers". And textmode means font
rendering is done via (fixed function in those times) hardware scanout
engine. Of course having only to update 2 bytes per character was a huge
saving early on. Likely this is also before vesa VBE was reliable.
So i guess the point where this all starts going wrong allowing the X parts
of the api to be combined with FB based rendering at all? Sounds the only
user didn't use that combination and so it was never tested?
Then again, this all relates to hardware from 20 years ago...
- Martin Hostettler
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-29 10:52 ` Martin Hostettler
0 siblings, 0 replies; 51+ messages in thread
From: Martin Hostettler @ 2020-09-29 10:52 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, gregkh,
deller, syzkaller-bugs, linux-kernel, dri-devel,
Martin Hostettler, Linus Torvalds, daniel.vetter, jirislaby,
Peilin Ye
On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote:
> On 2020/09/29 2:59, Martin Hostettler wrote:
> > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> >>
> >
> > It seems this is/was used by "svgatextmode" which seems to be at
> > http://www.ibiblio.org/pub/Linux/utils/console/
> >
> > Not sure if that kind of software still has a chance to work nowadays.
> >
>
> Thanks for the information.
>
> It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
>
Yes, this seems to be from pre framebuffer times.
Back in the days "svga" was the wording you got for "pokes svga card
hardware registers from userspace drivers". And textmode means font
rendering is done via (fixed function in those times) hardware scanout
engine. Of course having only to update 2 bytes per character was a huge
saving early on. Likely this is also before vesa VBE was reliable.
So i guess the point where this all starts going wrong allowing the X parts
of the api to be combined with FB based rendering at all? Sounds the only
user didn't use that combination and so it was never tested?
Then again, this all relates to hardware from 20 years ago...
- Martin Hostettler
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-29 10:52 ` Martin Hostettler
0 siblings, 0 replies; 51+ messages in thread
From: Martin Hostettler @ 2020-09-29 10:52 UTC (permalink / raw)
To: Tetsuo Handa
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, gregkh,
deller, syzkaller-bugs, linux-kernel, dri-devel,
Martin Hostettler, Linus Torvalds, daniel.vetter, jirislaby,
Peilin Ye
On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote:
> On 2020/09/29 2:59, Martin Hostettler wrote:
> > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> >>
> >
> > It seems this is/was used by "svgatextmode" which seems to be at
> > http://www.ibiblio.org/pub/Linux/utils/console/
> >
> > Not sure if that kind of software still has a chance to work nowadays.
> >
>
> Thanks for the information.
>
> It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
>
Yes, this seems to be from pre framebuffer times.
Back in the days "svga" was the wording you got for "pokes svga card
hardware registers from userspace drivers". And textmode means font
rendering is done via (fixed function in those times) hardware scanout
engine. Of course having only to update 2 bytes per character was a huge
saving early on. Likely this is also before vesa VBE was reliable.
So i guess the point where this all starts going wrong allowing the X parts
of the api to be combined with FB based rendering at all? Sounds the only
user didn't use that combination and so it was never tested?
Then again, this all relates to hardware from 20 years ago...
- Martin Hostettler
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2020-09-29 10:52 ` Martin Hostettler
(?)
@ 2020-09-29 16:56 ` Daniel Vetter
-1 siblings, 0 replies; 51+ messages in thread
From: Daniel Vetter @ 2020-09-29 16:56 UTC (permalink / raw)
To: Martin Hostettler
Cc: Tetsuo Handa, gregkh, jirislaby, Peilin Ye, syzbot, b.zolnierkie,
daniel.vetter, deller, syzkaller-bugs, Linus Torvalds, dri-devel,
linux-fbdev, linux-kernel, George Kennedy
On Tue, Sep 29, 2020 at 12:52:03PM +0200, Martin Hostettler wrote:
> On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote:
> > On 2020/09/29 2:59, Martin Hostettler wrote:
> > > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> > >>
> > >
> > > It seems this is/was used by "svgatextmode" which seems to be at
> > > http://www.ibiblio.org/pub/Linux/utils/console/
> > >
> > > Not sure if that kind of software still has a chance to work nowadays.
> > >
> >
> > Thanks for the information.
> >
> > It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> > But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
> >
>
> Yes, this seems to be from pre framebuffer times.
>
> Back in the days "svga" was the wording you got for "pokes svga card
> hardware registers from userspace drivers". And textmode means font
> rendering is done via (fixed function in those times) hardware scanout
> engine. Of course having only to update 2 bytes per character was a huge
> saving early on. Likely this is also before vesa VBE was reliable.
>
> So i guess the point where this all starts going wrong allowing the X parts
> of the api to be combined with FB based rendering at all? Sounds the only
> user didn't use that combination and so it was never tested?
>
> Then again, this all relates to hardware from 20 years ago...
Imo userspace modesetting should be burned down anywhere we can. We've
gotten away with this in drivers/gpu by just seamlessly transitioning to
kernel drivers.
Since th only userspace we've found seems to be able to cope if this ioctl
doesn't do anything, my vote goes towards ripping it out completely and
doing nothing in there. Only question is whether we should error or fail
with a silent success: Former is safer, latter can avoid a few regression
reports since the userspace tools keep "working", and usually people don't
notice for stuff this old. It worked in drivers/gpu :-)
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-29 16:56 ` Daniel Vetter
0 siblings, 0 replies; 51+ messages in thread
From: Daniel Vetter @ 2020-09-29 16:56 UTC (permalink / raw)
To: Martin Hostettler
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, Tetsuo Handa,
gregkh, deller, syzkaller-bugs, linux-kernel, dri-devel,
Linus Torvalds, daniel.vetter, jirislaby, Peilin Ye
On Tue, Sep 29, 2020 at 12:52:03PM +0200, Martin Hostettler wrote:
> On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote:
> > On 2020/09/29 2:59, Martin Hostettler wrote:
> > > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> > >>
> > >
> > > It seems this is/was used by "svgatextmode" which seems to be at
> > > http://www.ibiblio.org/pub/Linux/utils/console/
> > >
> > > Not sure if that kind of software still has a chance to work nowadays.
> > >
> >
> > Thanks for the information.
> >
> > It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> > But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
> >
>
> Yes, this seems to be from pre framebuffer times.
>
> Back in the days "svga" was the wording you got for "pokes svga card
> hardware registers from userspace drivers". And textmode means font
> rendering is done via (fixed function in those times) hardware scanout
> engine. Of course having only to update 2 bytes per character was a huge
> saving early on. Likely this is also before vesa VBE was reliable.
>
> So i guess the point where this all starts going wrong allowing the X parts
> of the api to be combined with FB based rendering at all? Sounds the only
> user didn't use that combination and so it was never tested?
>
> Then again, this all relates to hardware from 20 years ago...
Imo userspace modesetting should be burned down anywhere we can. We've
gotten away with this in drivers/gpu by just seamlessly transitioning to
kernel drivers.
Since th only userspace we've found seems to be able to cope if this ioctl
doesn't do anything, my vote goes towards ripping it out completely and
doing nothing in there. Only question is whether we should error or fail
with a silent success: Former is safer, latter can avoid a few regression
reports since the userspace tools keep "working", and usually people don't
notice for stuff this old. It worked in drivers/gpu :-)
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-29 16:56 ` Daniel Vetter
0 siblings, 0 replies; 51+ messages in thread
From: Daniel Vetter @ 2020-09-29 16:56 UTC (permalink / raw)
To: Martin Hostettler
Cc: syzbot, linux-fbdev, George Kennedy, b.zolnierkie, Tetsuo Handa,
gregkh, deller, syzkaller-bugs, linux-kernel, dri-devel,
Linus Torvalds, daniel.vetter, jirislaby, Peilin Ye
On Tue, Sep 29, 2020 at 12:52:03PM +0200, Martin Hostettler wrote:
> On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote:
> > On 2020/09/29 2:59, Martin Hostettler wrote:
> > > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> > >>
> > >
> > > It seems this is/was used by "svgatextmode" which seems to be at
> > > http://www.ibiblio.org/pub/Linux/utils/console/
> > >
> > > Not sure if that kind of software still has a chance to work nowadays.
> > >
> >
> > Thanks for the information.
> >
> > It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> > But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
> >
>
> Yes, this seems to be from pre framebuffer times.
>
> Back in the days "svga" was the wording you got for "pokes svga card
> hardware registers from userspace drivers". And textmode means font
> rendering is done via (fixed function in those times) hardware scanout
> engine. Of course having only to update 2 bytes per character was a huge
> saving early on. Likely this is also before vesa VBE was reliable.
>
> So i guess the point where this all starts going wrong allowing the X parts
> of the api to be combined with FB based rendering at all? Sounds the only
> user didn't use that combination and so it was never tested?
>
> Then again, this all relates to hardware from 20 years ago...
Imo userspace modesetting should be burned down anywhere we can. We've
gotten away with this in drivers/gpu by just seamlessly transitioning to
kernel drivers.
Since th only userspace we've found seems to be able to cope if this ioctl
doesn't do anything, my vote goes towards ripping it out completely and
doing nothing in there. Only question is whether we should error or fail
with a silent success: Former is safer, latter can avoid a few regression
reports since the userspace tools keep "working", and usually people don't
notice for stuff this old. It worked in drivers/gpu :-)
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2020-09-29 16:56 ` Daniel Vetter
(?)
@ 2020-09-29 17:10 ` Greg KH
-1 siblings, 0 replies; 51+ messages in thread
From: Greg KH @ 2020-09-29 17:10 UTC (permalink / raw)
To: Martin Hostettler, Tetsuo Handa, jirislaby, Peilin Ye, syzbot,
b.zolnierkie, deller, syzkaller-bugs, Linus Torvalds, dri-devel,
linux-fbdev, linux-kernel, George Kennedy
On Tue, Sep 29, 2020 at 06:56:57PM +0200, Daniel Vetter wrote:
> On Tue, Sep 29, 2020 at 12:52:03PM +0200, Martin Hostettler wrote:
> > On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote:
> > > On 2020/09/29 2:59, Martin Hostettler wrote:
> > > > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> > > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> > > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> > > >>
> > > >
> > > > It seems this is/was used by "svgatextmode" which seems to be at
> > > > http://www.ibiblio.org/pub/Linux/utils/console/
> > > >
> > > > Not sure if that kind of software still has a chance to work nowadays.
> > > >
> > >
> > > Thanks for the information.
> > >
> > > It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> > > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> > > But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
> > >
> >
> > Yes, this seems to be from pre framebuffer times.
> >
> > Back in the days "svga" was the wording you got for "pokes svga card
> > hardware registers from userspace drivers". And textmode means font
> > rendering is done via (fixed function in those times) hardware scanout
> > engine. Of course having only to update 2 bytes per character was a huge
> > saving early on. Likely this is also before vesa VBE was reliable.
> >
> > So i guess the point where this all starts going wrong allowing the X parts
> > of the api to be combined with FB based rendering at all? Sounds the only
> > user didn't use that combination and so it was never tested?
> >
> > Then again, this all relates to hardware from 20 years ago...
>
> Imo userspace modesetting should be burned down anywhere we can. We've
> gotten away with this in drivers/gpu by just seamlessly transitioning to
> kernel drivers.
>
> Since th only userspace we've found seems to be able to cope if this ioctl
> doesn't do anything, my vote goes towards ripping it out completely and
> doing nothing in there. Only question is whether we should error or fail
> with a silent success: Former is safer, latter can avoid a few regression
> reports since the userspace tools keep "working", and usually people don't
> notice for stuff this old. It worked in drivers/gpu :-)
This patch just ignores the ioctl and keeps on going, so userspace
"shouldn't" notice it :)
And it's in linux-next now, so all should be good.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-29 17:10 ` Greg KH
0 siblings, 0 replies; 51+ messages in thread
From: Greg KH @ 2020-09-29 17:10 UTC (permalink / raw)
To: Martin Hostettler, Tetsuo Handa, jirislaby, Peilin Ye, syzbot,
b.zolnierkie, deller, syzkaller-bugs, Linus Torvalds, dri-devel,
linux-fbdev, linux-kernel, George Kennedy
On Tue, Sep 29, 2020 at 06:56:57PM +0200, Daniel Vetter wrote:
> On Tue, Sep 29, 2020 at 12:52:03PM +0200, Martin Hostettler wrote:
> > On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote:
> > > On 2020/09/29 2:59, Martin Hostettler wrote:
> > > > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> > > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> > > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> > > >>
> > > >
> > > > It seems this is/was used by "svgatextmode" which seems to be at
> > > > http://www.ibiblio.org/pub/Linux/utils/console/
> > > >
> > > > Not sure if that kind of software still has a chance to work nowadays.
> > > >
> > >
> > > Thanks for the information.
> > >
> > > It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> > > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> > > But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
> > >
> >
> > Yes, this seems to be from pre framebuffer times.
> >
> > Back in the days "svga" was the wording you got for "pokes svga card
> > hardware registers from userspace drivers". And textmode means font
> > rendering is done via (fixed function in those times) hardware scanout
> > engine. Of course having only to update 2 bytes per character was a huge
> > saving early on. Likely this is also before vesa VBE was reliable.
> >
> > So i guess the point where this all starts going wrong allowing the X parts
> > of the api to be combined with FB based rendering at all? Sounds the only
> > user didn't use that combination and so it was never tested?
> >
> > Then again, this all relates to hardware from 20 years ago...
>
> Imo userspace modesetting should be burned down anywhere we can. We've
> gotten away with this in drivers/gpu by just seamlessly transitioning to
> kernel drivers.
>
> Since th only userspace we've found seems to be able to cope if this ioctl
> doesn't do anything, my vote goes towards ripping it out completely and
> doing nothing in there. Only question is whether we should error or fail
> with a silent success: Former is safer, latter can avoid a few regression
> reports since the userspace tools keep "working", and usually people don't
> notice for stuff this old. It worked in drivers/gpu :-)
This patch just ignores the ioctl and keeps on going, so userspace
"shouldn't" notice it :)
And it's in linux-next now, so all should be good.
thanks,
greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2020-09-29 17:10 ` Greg KH
0 siblings, 0 replies; 51+ messages in thread
From: Greg KH @ 2020-09-29 17:10 UTC (permalink / raw)
To: Martin Hostettler, Tetsuo Handa, jirislaby, Peilin Ye, syzbot,
b.zolnierkie, deller, syzkaller-bugs, Linus Torvalds, dri-devel,
linux-fbdev, linux-kernel, George Kennedy
On Tue, Sep 29, 2020 at 06:56:57PM +0200, Daniel Vetter wrote:
> On Tue, Sep 29, 2020 at 12:52:03PM +0200, Martin Hostettler wrote:
> > On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote:
> > > On 2020/09/29 2:59, Martin Hostettler wrote:
> > > > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote:
> > > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> > > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> > > >>
> > > >
> > > > It seems this is/was used by "svgatextmode" which seems to be at
> > > > http://www.ibiblio.org/pub/Linux/utils/console/
> > > >
> > > > Not sure if that kind of software still has a chance to work nowadays.
> > > >
> > >
> > > Thanks for the information.
> > >
> > > It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> > > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> > > But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
> > >
> >
> > Yes, this seems to be from pre framebuffer times.
> >
> > Back in the days "svga" was the wording you got for "pokes svga card
> > hardware registers from userspace drivers". And textmode means font
> > rendering is done via (fixed function in those times) hardware scanout
> > engine. Of course having only to update 2 bytes per character was a huge
> > saving early on. Likely this is also before vesa VBE was reliable.
> >
> > So i guess the point where this all starts going wrong allowing the X parts
> > of the api to be combined with FB based rendering at all? Sounds the only
> > user didn't use that combination and so it was never tested?
> >
> > Then again, this all relates to hardware from 20 years ago...
>
> Imo userspace modesetting should be burned down anywhere we can. We've
> gotten away with this in drivers/gpu by just seamlessly transitioning to
> kernel drivers.
>
> Since th only userspace we've found seems to be able to cope if this ioctl
> doesn't do anything, my vote goes towards ripping it out completely and
> doing nothing in there. Only question is whether we should error or fail
> with a silent success: Former is safer, latter can avoid a few regression
> reports since the userspace tools keep "working", and usually people don't
> notice for stuff this old. It worked in drivers/gpu :-)
This patch just ignores the ioctl and keeps on going, so userspace
"shouldn't" notice it :)
And it's in linux-next now, so all should be good.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2020-09-29 17:10 ` Greg KH
@ 2021-04-11 21:43 ` Maciej W. Rozycki
-1 siblings, 0 replies; 51+ messages in thread
From: Maciej W. Rozycki @ 2021-04-11 21:43 UTC (permalink / raw)
To: Greg KH
Cc: Martin Hostettler, Tetsuo Handa, jirislaby, Peilin Ye, syzbot,
b.zolnierkie, deller, syzkaller-bugs, Linus Torvalds, dri-devel,
linux-fbdev, linux-kernel, George Kennedy
On Tue, 29 Sep 2020, Greg KH wrote:
> > > > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> > > > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> > > > >>
> > > > >
> > > > > It seems this is/was used by "svgatextmode" which seems to be at
> > > > > http://www.ibiblio.org/pub/Linux/utils/console/
> > > > >
> > > > > Not sure if that kind of software still has a chance to work nowadays.
> > > > >
> > > >
> > > > Thanks for the information.
> > > >
> > > > It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> > > > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> > > > But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
> > > >
> > >
> > > Yes, this seems to be from pre framebuffer times.
> > >
> > > Back in the days "svga" was the wording you got for "pokes svga card
> > > hardware registers from userspace drivers". And textmode means font
> > > rendering is done via (fixed function in those times) hardware scanout
> > > engine. Of course having only to update 2 bytes per character was a huge
> > > saving early on. Likely this is also before vesa VBE was reliable.
> > >
> > > So i guess the point where this all starts going wrong allowing the X parts
> > > of the api to be combined with FB based rendering at all? Sounds the only
> > > user didn't use that combination and so it was never tested?
> > >
> > > Then again, this all relates to hardware from 20 years ago...
> >
> > Imo userspace modesetting should be burned down anywhere we can. We've
> > gotten away with this in drivers/gpu by just seamlessly transitioning to
> > kernel drivers.
> >
> > Since th only userspace we've found seems to be able to cope if this ioctl
> > doesn't do anything, my vote goes towards ripping it out completely and
> > doing nothing in there. Only question is whether we should error or fail
> > with a silent success: Former is safer, latter can avoid a few regression
> > reports since the userspace tools keep "working", and usually people don't
> > notice for stuff this old. It worked in drivers/gpu :-)
>
> This patch just ignores the ioctl and keeps on going, so userspace
> "shouldn't" notice it :)
>
> And it's in linux-next now, so all should be good.
So it does trigger with vgacon and my old server, which I have started
experimenting with and for a start I have switched to a new kernel for an
unrelated purpose (now that I have relieved it from all its usual tasks
except for the last remaining one for which I haven't got the required
user software ported to the new system yet):
"struct vt_consize"->v_vlin is ignored. Please report if you need this.
"struct vt_consize"->v_clin is ignored. Please report if you need this.
It continues using svgatextmode with its glass (CRT) VT to set my usual
80x37 text mode (720x576 pixel resolution) by manipulating the VGA clock
chip and the CRT controller appropriately for a nice refresh rate of 85Hz:
Chipset = `TVGA8900', Textmode clock = 44.90 MHz, 80x37 chars, CharCell = 9x16. Refresh = 52.51kHz/84.7Hz.
Indeed the piece of software became less usable around Y2K as clock chip
support stopped being added to svgatextmode for new video adapters, but
with the advent of LCD technology and its disregard for the refresh rate
previously driven by the pixel clock the program got its second life and I
have used it ever since with its plain VGA driver by just manipulating the
CRTC for the resolution required:
Chipset = `VGA', Textmode clock = 28.32 MHz, 80x37 chars, CharCell = 9x16. Refresh = 31.47kHz/49.0Hz.
(that would still work with a standard 800x600 SVGA CRT, but the refresh
rate would make anyone's eyes cry soon; with LCD it's just awesome, and
the VGA emulation of the actual graphics adapter turns it at the video
output into a 1600x1200 picture at the horizontal and vertical rates of
75KHz and 60Hz respectively, making the text produced on LCD outstanding
while showing about the right amount of it).
But I'm currently ~160km/100mi away from the server I have triggered this
message with, so I cannot easily check what's going on with its VT. And I
can't fiddle with my production laptop (ThinkPad P51) I have with me that
I also use svgatextmode with so much as to reboot it with a new kernel
(plain Debian 4.19.16-1~bpo9+1 still here).
So what's the supposed impact of this change that prompted the inclusion
of the messages? I can port svgatextmode (it's my own compilation anyway)
if that is required for it to continue working correctly, but I need to
understand the circumstances here. I have failed to find a satisfactory
alternative solution to vgacon and svgatextmode; the main showstopper is
the IBM's hardware trick for a 9x16 character cell that I rely on.
Maciej
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2021-04-11 21:43 ` Maciej W. Rozycki
0 siblings, 0 replies; 51+ messages in thread
From: Maciej W. Rozycki @ 2021-04-11 21:43 UTC (permalink / raw)
To: Greg KH
Cc: syzbot, linux-fbdev, b.zolnierkie, Tetsuo Handa, Linus Torvalds,
deller, syzkaller-bugs, linux-kernel, dri-devel,
Martin Hostettler, George Kennedy, jirislaby, Peilin Ye
On Tue, 29 Sep 2020, Greg KH wrote:
> > > > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
> > > > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
> > > > >>
> > > > >
> > > > > It seems this is/was used by "svgatextmode" which seems to be at
> > > > > http://www.ibiblio.org/pub/Linux/utils/console/
> > > > >
> > > > > Not sure if that kind of software still has a chance to work nowadays.
> > > > >
> > > >
> > > > Thanks for the information.
> > > >
> > > > It seems that v.v_vlin = curr_textmode->VDisplay / (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1)
> > > > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height and seems to be non-zero.
> > > > But according to https://bugs.gentoo.org/19485 , people are using kernel framebuffer instead.
> > > >
> > >
> > > Yes, this seems to be from pre framebuffer times.
> > >
> > > Back in the days "svga" was the wording you got for "pokes svga card
> > > hardware registers from userspace drivers". And textmode means font
> > > rendering is done via (fixed function in those times) hardware scanout
> > > engine. Of course having only to update 2 bytes per character was a huge
> > > saving early on. Likely this is also before vesa VBE was reliable.
> > >
> > > So i guess the point where this all starts going wrong allowing the X parts
> > > of the api to be combined with FB based rendering at all? Sounds the only
> > > user didn't use that combination and so it was never tested?
> > >
> > > Then again, this all relates to hardware from 20 years ago...
> >
> > Imo userspace modesetting should be burned down anywhere we can. We've
> > gotten away with this in drivers/gpu by just seamlessly transitioning to
> > kernel drivers.
> >
> > Since th only userspace we've found seems to be able to cope if this ioctl
> > doesn't do anything, my vote goes towards ripping it out completely and
> > doing nothing in there. Only question is whether we should error or fail
> > with a silent success: Former is safer, latter can avoid a few regression
> > reports since the userspace tools keep "working", and usually people don't
> > notice for stuff this old. It worked in drivers/gpu :-)
>
> This patch just ignores the ioctl and keeps on going, so userspace
> "shouldn't" notice it :)
>
> And it's in linux-next now, so all should be good.
So it does trigger with vgacon and my old server, which I have started
experimenting with and for a start I have switched to a new kernel for an
unrelated purpose (now that I have relieved it from all its usual tasks
except for the last remaining one for which I haven't got the required
user software ported to the new system yet):
"struct vt_consize"->v_vlin is ignored. Please report if you need this.
"struct vt_consize"->v_clin is ignored. Please report if you need this.
It continues using svgatextmode with its glass (CRT) VT to set my usual
80x37 text mode (720x576 pixel resolution) by manipulating the VGA clock
chip and the CRT controller appropriately for a nice refresh rate of 85Hz:
Chipset = `TVGA8900', Textmode clock = 44.90 MHz, 80x37 chars, CharCell = 9x16. Refresh = 52.51kHz/84.7Hz.
Indeed the piece of software became less usable around Y2K as clock chip
support stopped being added to svgatextmode for new video adapters, but
with the advent of LCD technology and its disregard for the refresh rate
previously driven by the pixel clock the program got its second life and I
have used it ever since with its plain VGA driver by just manipulating the
CRTC for the resolution required:
Chipset = `VGA', Textmode clock = 28.32 MHz, 80x37 chars, CharCell = 9x16. Refresh = 31.47kHz/49.0Hz.
(that would still work with a standard 800x600 SVGA CRT, but the refresh
rate would make anyone's eyes cry soon; with LCD it's just awesome, and
the VGA emulation of the actual graphics adapter turns it at the video
output into a 1600x1200 picture at the horizontal and vertical rates of
75KHz and 60Hz respectively, making the text produced on LCD outstanding
while showing about the right amount of it).
But I'm currently ~160km/100mi away from the server I have triggered this
message with, so I cannot easily check what's going on with its VT. And I
can't fiddle with my production laptop (ThinkPad P51) I have with me that
I also use svgatextmode with so much as to reboot it with a new kernel
(plain Debian 4.19.16-1~bpo9+1 still here).
So what's the supposed impact of this change that prompted the inclusion
of the messages? I can port svgatextmode (it's my own compilation anyway)
if that is required for it to continue working correctly, but I need to
understand the circumstances here. I have failed to find a satisfactory
alternative solution to vgacon and svgatextmode; the main showstopper is
the IBM's hardware trick for a 9x16 character cell that I rely on.
Maciej
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2021-04-11 21:43 ` Maciej W. Rozycki
@ 2021-04-11 22:15 ` Linus Torvalds
-1 siblings, 0 replies; 51+ messages in thread
From: Linus Torvalds @ 2021-04-11 22:15 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Greg KH, Martin Hostettler, Tetsuo Handa, Jiri Slaby, Peilin Ye,
syzbot, Bartlomiej Zolnierkiewicz, Helge Deller, syzkaller-bugs,
dri-devel, Linux Fbdev development list,
Linux Kernel Mailing List, George Kennedy
On Sun, Apr 11, 2021 at 2:43 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> So it does trigger with vgacon and my old server, which I have started
> experimenting with and for a start I have switched to a new kernel for an
> unrelated purpose (now that I have relieved it from all its usual tasks
> except for the last remaining one for which I haven't got the required
> user software ported to the new system yet):
>
> "struct vt_consize"->v_vlin is ignored. Please report if you need this.
> "struct vt_consize"->v_clin is ignored. Please report if you need this.
Note that it's entirely possible that things continue to work well
despite this warning. It's unclear to me from your email if you
actually see any difference (and apparently you're not able to see it
right now due to not being close to the machine).
The fact that v_vlin/v_clin are ignored doesn't necessarily mean that
they are different from what they were before, so the warning may be a
non-issue.
> It continues using svgatextmode with its glass (CRT) VT to set my usual
> 80x37 text mode (720x576 pixel resolution) by manipulating the VGA clock
> chip and the CRT controller appropriately for a nice refresh rate of 85Hz:
>
> Chipset = `TVGA8900', Textmode clock = 44.90 MHz, 80x37 chars, CharCell = 9x16. Refresh = 52.51kHz/84.7Hz.
That doesn't seem necessarily wrong to me.
> So what's the supposed impact of this change that prompted the inclusion
> of the messages?
There _may_ be no impact at all apart from the messages.
The code _used_ to set the scan lines (v_vlin) and font height
(v_clin) from those numbers if they were non-zero, and now it just
ignores them and warns instead.
The code now just sets the font height from the actual font size when
the font is set. Which is honestly the only thing that ever made
sense. Trying to set it with v_clin is ignored, but it's entirely
possible - maybe even likely - that your user of VT_RESIZEX sets it to
the same values it already has.
Exactly because setting a font line number to anything else than the
font size isn't exactly sensible.
But if your screen looks different than it used to, that is obviously
interesting and says something actually changed (outside of the
message itself).
Linus
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2021-04-11 22:15 ` Linus Torvalds
0 siblings, 0 replies; 51+ messages in thread
From: Linus Torvalds @ 2021-04-11 22:15 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: syzbot, Linux Fbdev development list, Bartlomiej Zolnierkiewicz,
Tetsuo Handa, Greg KH, Helge Deller, syzkaller-bugs,
Linux Kernel Mailing List, dri-devel, Martin Hostettler,
George Kennedy, Jiri Slaby, Peilin Ye
On Sun, Apr 11, 2021 at 2:43 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> So it does trigger with vgacon and my old server, which I have started
> experimenting with and for a start I have switched to a new kernel for an
> unrelated purpose (now that I have relieved it from all its usual tasks
> except for the last remaining one for which I haven't got the required
> user software ported to the new system yet):
>
> "struct vt_consize"->v_vlin is ignored. Please report if you need this.
> "struct vt_consize"->v_clin is ignored. Please report if you need this.
Note that it's entirely possible that things continue to work well
despite this warning. It's unclear to me from your email if you
actually see any difference (and apparently you're not able to see it
right now due to not being close to the machine).
The fact that v_vlin/v_clin are ignored doesn't necessarily mean that
they are different from what they were before, so the warning may be a
non-issue.
> It continues using svgatextmode with its glass (CRT) VT to set my usual
> 80x37 text mode (720x576 pixel resolution) by manipulating the VGA clock
> chip and the CRT controller appropriately for a nice refresh rate of 85Hz:
>
> Chipset = `TVGA8900', Textmode clock = 44.90 MHz, 80x37 chars, CharCell = 9x16. Refresh = 52.51kHz/84.7Hz.
That doesn't seem necessarily wrong to me.
> So what's the supposed impact of this change that prompted the inclusion
> of the messages?
There _may_ be no impact at all apart from the messages.
The code _used_ to set the scan lines (v_vlin) and font height
(v_clin) from those numbers if they were non-zero, and now it just
ignores them and warns instead.
The code now just sets the font height from the actual font size when
the font is set. Which is honestly the only thing that ever made
sense. Trying to set it with v_clin is ignored, but it's entirely
possible - maybe even likely - that your user of VT_RESIZEX sets it to
the same values it already has.
Exactly because setting a font line number to anything else than the
font size isn't exactly sensible.
But if your screen looks different than it used to, that is obviously
interesting and says something actually changed (outside of the
message itself).
Linus
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2021-04-11 22:15 ` Linus Torvalds
@ 2021-04-12 7:01 ` Daniel Vetter
-1 siblings, 0 replies; 51+ messages in thread
From: Daniel Vetter @ 2021-04-12 7:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: Maciej W. Rozycki, syzbot, Linux Fbdev development list,
Bartlomiej Zolnierkiewicz, Tetsuo Handa, Greg KH, Helge Deller,
syzkaller-bugs, Linux Kernel Mailing List, dri-devel,
Martin Hostettler, George Kennedy, Jiri Slaby, Peilin Ye
On Mon, Apr 12, 2021 at 12:15 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sun, Apr 11, 2021 at 2:43 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
> >
> > So it does trigger with vgacon and my old server, which I have started
> > experimenting with and for a start I have switched to a new kernel for an
> > unrelated purpose (now that I have relieved it from all its usual tasks
> > except for the last remaining one for which I haven't got the required
> > user software ported to the new system yet):
> >
> > "struct vt_consize"->v_vlin is ignored. Please report if you need this.
> > "struct vt_consize"->v_clin is ignored. Please report if you need this.
>
> Note that it's entirely possible that things continue to work well
> despite this warning. It's unclear to me from your email if you
> actually see any difference (and apparently you're not able to see it
> right now due to not being close to the machine).
Original search didn't turn up any users of VT_RESIZEX, this is the
first. And looking at the source code I think we could outright remove
support for VT_RESIZEX (but make it silent) and everything should keep
working:
/*
* ALWAYS do a VT_RESIZE, even if we already did a VT_RESIZEX
on a 1.3.3 or higher kernel,
* until those kernel programmers make this unambiguous
*/
if (do_VT_RESIZE(curr_textmode->cols, curr_textmode->rows,
resize1x1)) sresize=TRUE;
if (check_kernel_version(1,3,3, "VT_RESIZEX"))
{
/*
* VDisplay must de divided by 2 for DoubleScan modes,
* or VT_RESIZEX will fail -- until someone fixes the kernel
* so it understands about doublescan modes.
*/
if (do_VT_RESIZEX(curr_textmode->cols,
curr_textmode->rows,
curr_textmode->VDisplay /
(MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1),
curr_textmode->FontHeight,
curr_textmode->HDisplay/8*curr_textmode->FontWidth,
curr_textmode->FontWidth, resize1x1)) sresize=TRUE;
}
The functions are just straightforward wrappers. There's also no cvs
repo, changelog or old releases before 2000 that would shed some light
on why this code even exists.
I think we can just tune down the pr_info_once to pr_debug and done.
Maybe a comment about where the single user we're aware of is.
-Daniel
>
> The fact that v_vlin/v_clin are ignored doesn't necessarily mean that
> they are different from what they were before, so the warning may be a
> non-issue.
>
> > It continues using svgatextmode with its glass (CRT) VT to set my usual
> > 80x37 text mode (720x576 pixel resolution) by manipulating the VGA clock
> > chip and the CRT controller appropriately for a nice refresh rate of 85Hz:
> >
> > Chipset = `TVGA8900', Textmode clock = 44.90 MHz, 80x37 chars, CharCell = 9x16. Refresh = 52.51kHz/84.7Hz.
>
> That doesn't seem necessarily wrong to me.
>
> > So what's the supposed impact of this change that prompted the inclusion
> > of the messages?
>
> There _may_ be no impact at all apart from the messages.
>
> The code _used_ to set the scan lines (v_vlin) and font height
> (v_clin) from those numbers if they were non-zero, and now it just
> ignores them and warns instead.
>
> The code now just sets the font height from the actual font size when
> the font is set. Which is honestly the only thing that ever made
> sense. Trying to set it with v_clin is ignored, but it's entirely
> possible - maybe even likely - that your user of VT_RESIZEX sets it to
> the same values it already has.
>
> Exactly because setting a font line number to anything else than the
> font size isn't exactly sensible.
>
> But if your screen looks different than it used to, that is obviously
> interesting and says something actually changed (outside of the
> message itself).
>
> Linus
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2021-04-12 7:01 ` Daniel Vetter
0 siblings, 0 replies; 51+ messages in thread
From: Daniel Vetter @ 2021-04-12 7:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: syzbot, Linux Fbdev development list, Bartlomiej Zolnierkiewicz,
Tetsuo Handa, Greg KH, Helge Deller, syzkaller-bugs,
Linux Kernel Mailing List, dri-devel, Martin Hostettler,
George Kennedy, Jiri Slaby, Peilin Ye, Maciej W. Rozycki
On Mon, Apr 12, 2021 at 12:15 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sun, Apr 11, 2021 at 2:43 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
> >
> > So it does trigger with vgacon and my old server, which I have started
> > experimenting with and for a start I have switched to a new kernel for an
> > unrelated purpose (now that I have relieved it from all its usual tasks
> > except for the last remaining one for which I haven't got the required
> > user software ported to the new system yet):
> >
> > "struct vt_consize"->v_vlin is ignored. Please report if you need this.
> > "struct vt_consize"->v_clin is ignored. Please report if you need this.
>
> Note that it's entirely possible that things continue to work well
> despite this warning. It's unclear to me from your email if you
> actually see any difference (and apparently you're not able to see it
> right now due to not being close to the machine).
Original search didn't turn up any users of VT_RESIZEX, this is the
first. And looking at the source code I think we could outright remove
support for VT_RESIZEX (but make it silent) and everything should keep
working:
/*
* ALWAYS do a VT_RESIZE, even if we already did a VT_RESIZEX
on a 1.3.3 or higher kernel,
* until those kernel programmers make this unambiguous
*/
if (do_VT_RESIZE(curr_textmode->cols, curr_textmode->rows,
resize1x1)) sresize=TRUE;
if (check_kernel_version(1,3,3, "VT_RESIZEX"))
{
/*
* VDisplay must de divided by 2 for DoubleScan modes,
* or VT_RESIZEX will fail -- until someone fixes the kernel
* so it understands about doublescan modes.
*/
if (do_VT_RESIZEX(curr_textmode->cols,
curr_textmode->rows,
curr_textmode->VDisplay /
(MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1),
curr_textmode->FontHeight,
curr_textmode->HDisplay/8*curr_textmode->FontWidth,
curr_textmode->FontWidth, resize1x1)) sresize=TRUE;
}
The functions are just straightforward wrappers. There's also no cvs
repo, changelog or old releases before 2000 that would shed some light
on why this code even exists.
I think we can just tune down the pr_info_once to pr_debug and done.
Maybe a comment about where the single user we're aware of is.
-Daniel
>
> The fact that v_vlin/v_clin are ignored doesn't necessarily mean that
> they are different from what they were before, so the warning may be a
> non-issue.
>
> > It continues using svgatextmode with its glass (CRT) VT to set my usual
> > 80x37 text mode (720x576 pixel resolution) by manipulating the VGA clock
> > chip and the CRT controller appropriately for a nice refresh rate of 85Hz:
> >
> > Chipset = `TVGA8900', Textmode clock = 44.90 MHz, 80x37 chars, CharCell = 9x16. Refresh = 52.51kHz/84.7Hz.
>
> That doesn't seem necessarily wrong to me.
>
> > So what's the supposed impact of this change that prompted the inclusion
> > of the messages?
>
> There _may_ be no impact at all apart from the messages.
>
> The code _used_ to set the scan lines (v_vlin) and font height
> (v_clin) from those numbers if they were non-zero, and now it just
> ignores them and warns instead.
>
> The code now just sets the font height from the actual font size when
> the font is set. Which is honestly the only thing that ever made
> sense. Trying to set it with v_clin is ignored, but it's entirely
> possible - maybe even likely - that your user of VT_RESIZEX sets it to
> the same values it already has.
>
> Exactly because setting a font line number to anything else than the
> font size isn't exactly sensible.
>
> But if your screen looks different than it used to, that is obviously
> interesting and says something actually changed (outside of the
> message itself).
>
> Linus
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2021-04-12 7:01 ` Daniel Vetter
@ 2021-04-12 13:30 ` Maciej W. Rozycki
-1 siblings, 0 replies; 51+ messages in thread
From: Maciej W. Rozycki @ 2021-04-12 13:30 UTC (permalink / raw)
To: Daniel Vetter
Cc: Linus Torvalds, syzbot, Linux Fbdev development list,
Bartlomiej Zolnierkiewicz, Tetsuo Handa, Greg KH, Helge Deller,
syzkaller-bugs, Linux Kernel Mailing List, dri-devel,
Martin Hostettler, George Kennedy, Jiri Slaby, Peilin Ye
On Mon, 12 Apr 2021, Daniel Vetter wrote:
> > Note that it's entirely possible that things continue to work well
> > despite this warning. It's unclear to me from your email if you
> > actually see any difference (and apparently you're not able to see it
> > right now due to not being close to the machine).
>
> Original search didn't turn up any users of VT_RESIZEX, this is the
> first. And looking at the source code I think we could outright remove
> support for VT_RESIZEX (but make it silent) and everything should keep
> working:
>
> /*
> * ALWAYS do a VT_RESIZE, even if we already did a VT_RESIZEX
> on a 1.3.3 or higher kernel,
> * until those kernel programmers make this unambiguous
> */
>
> if (do_VT_RESIZE(curr_textmode->cols, curr_textmode->rows,
> resize1x1)) sresize=TRUE;
>
> if (check_kernel_version(1,3,3, "VT_RESIZEX"))
> {
> /*
> * VDisplay must de divided by 2 for DoubleScan modes,
> * or VT_RESIZEX will fail -- until someone fixes the kernel
> * so it understands about doublescan modes.
> */
> if (do_VT_RESIZEX(curr_textmode->cols,
> curr_textmode->rows,
> curr_textmode->VDisplay /
> (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1),
> curr_textmode->FontHeight,
> curr_textmode->HDisplay/8*curr_textmode->FontWidth,
> curr_textmode->FontWidth, resize1x1)) sresize=TRUE;
> }
>
> The functions are just straightforward wrappers. There's also no cvs
> repo, changelog or old releases before 2000 that would shed some light
> on why this code even exists.
I did some archaeology then, using a local copy of the linux-mips.org
Linux tree that has historic information imported from the old oss.sgi.com
MIPS/Linux CVS repo. According to that the ioctl was added with or
shortly before 2.1.14:
commit beb116954b9b7f3bb56412b2494b562f02b864b1
Author: Ralf Baechle <ralf@linux-mips.org>
Date: Tue Jan 7 02:33:00 1997 +0000
Import of Linux/MIPS 2.1.14
and, importantly, it was used to set some parameters:
if ( vlin )
video_scan_lines = vlin;
if ( clin )
video_font_height = clin;
used by `con_adjust_height' in drivers/char/vga.c: `video_scan_lines' to
set the vertical display limit (so that it is a whole multiple of the new
font height) and `video_font_height' to set the cursor scan lines in the
CRTC. The function was used by the PIO_FONTX and PIO_FONTRESET VT ioctls
at that point.
That code was moved to `vgacon_adjust_height' in drivers/video/vgacon.c
and then drivers/video/console/vgacon.c. The code is still there, serving
the KDFONTOP ioctl.
With:
commit 9736a3546de7b6a2b16ad93539e4b3ac72b385bb
Author: Ralf Baechle <ralf@linux-mips.org>
Date: Thu Jun 5 10:06:35 2003 +0000
Merge with Linux 2.5.66.
the parameters were moved into `struct vc_data':
if (vlin)
- video_scan_lines = vlin;
+ vc->vc_scan_lines = vlin;
if (clin)
- video_font_height = clin;
+ vc->vc_font.height = clin;
and this piece of code to set them only removed with the change discussed
here.
So without even looking at the VT, which I'll surely get to eventually, I
conclude this change regresses font resizing (KD_FONT_OP_SET) once a new
resolution has been set with svgatextmode. I think this change needs to
be reverted, especially as the problematic PIO_FONT ioctl referred has
been since removed with commit ff2047fb755d ("vt: drop old FONT ioctls").
Maciej
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
@ 2021-04-12 13:30 ` Maciej W. Rozycki
0 siblings, 0 replies; 51+ messages in thread
From: Maciej W. Rozycki @ 2021-04-12 13:30 UTC (permalink / raw)
To: Daniel Vetter
Cc: syzbot, Linux Fbdev development list, Jiri Slaby,
Bartlomiej Zolnierkiewicz, Tetsuo Handa, Greg KH, Helge Deller,
syzkaller-bugs, Linux Kernel Mailing List, dri-devel,
Martin Hostettler, George Kennedy, Linus Torvalds, Peilin Ye
On Mon, 12 Apr 2021, Daniel Vetter wrote:
> > Note that it's entirely possible that things continue to work well
> > despite this warning. It's unclear to me from your email if you
> > actually see any difference (and apparently you're not able to see it
> > right now due to not being close to the machine).
>
> Original search didn't turn up any users of VT_RESIZEX, this is the
> first. And looking at the source code I think we could outright remove
> support for VT_RESIZEX (but make it silent) and everything should keep
> working:
>
> /*
> * ALWAYS do a VT_RESIZE, even if we already did a VT_RESIZEX
> on a 1.3.3 or higher kernel,
> * until those kernel programmers make this unambiguous
> */
>
> if (do_VT_RESIZE(curr_textmode->cols, curr_textmode->rows,
> resize1x1)) sresize=TRUE;
>
> if (check_kernel_version(1,3,3, "VT_RESIZEX"))
> {
> /*
> * VDisplay must de divided by 2 for DoubleScan modes,
> * or VT_RESIZEX will fail -- until someone fixes the kernel
> * so it understands about doublescan modes.
> */
> if (do_VT_RESIZEX(curr_textmode->cols,
> curr_textmode->rows,
> curr_textmode->VDisplay /
> (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1),
> curr_textmode->FontHeight,
> curr_textmode->HDisplay/8*curr_textmode->FontWidth,
> curr_textmode->FontWidth, resize1x1)) sresize=TRUE;
> }
>
> The functions are just straightforward wrappers. There's also no cvs
> repo, changelog or old releases before 2000 that would shed some light
> on why this code even exists.
I did some archaeology then, using a local copy of the linux-mips.org
Linux tree that has historic information imported from the old oss.sgi.com
MIPS/Linux CVS repo. According to that the ioctl was added with or
shortly before 2.1.14:
commit beb116954b9b7f3bb56412b2494b562f02b864b1
Author: Ralf Baechle <ralf@linux-mips.org>
Date: Tue Jan 7 02:33:00 1997 +0000
Import of Linux/MIPS 2.1.14
and, importantly, it was used to set some parameters:
if ( vlin )
video_scan_lines = vlin;
if ( clin )
video_font_height = clin;
used by `con_adjust_height' in drivers/char/vga.c: `video_scan_lines' to
set the vertical display limit (so that it is a whole multiple of the new
font height) and `video_font_height' to set the cursor scan lines in the
CRTC. The function was used by the PIO_FONTX and PIO_FONTRESET VT ioctls
at that point.
That code was moved to `vgacon_adjust_height' in drivers/video/vgacon.c
and then drivers/video/console/vgacon.c. The code is still there, serving
the KDFONTOP ioctl.
With:
commit 9736a3546de7b6a2b16ad93539e4b3ac72b385bb
Author: Ralf Baechle <ralf@linux-mips.org>
Date: Thu Jun 5 10:06:35 2003 +0000
Merge with Linux 2.5.66.
the parameters were moved into `struct vc_data':
if (vlin)
- video_scan_lines = vlin;
+ vc->vc_scan_lines = vlin;
if (clin)
- video_font_height = clin;
+ vc->vc_font.height = clin;
and this piece of code to set them only removed with the change discussed
here.
So without even looking at the VT, which I'll surely get to eventually, I
conclude this change regresses font resizing (KD_FONT_OP_SET) once a new
resolution has been set with svgatextmode. I think this change needs to
be reverted, especially as the problematic PIO_FONT ioctl referred has
been since removed with commit ff2047fb755d ("vt: drop old FONT ioctls").
Maciej
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 51+ messages in thread
* [tip: perf/urgent] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
2020-09-27 11:46 ` Tetsuo Handa
` (3 preceding siblings ...)
(?)
@ 2020-10-19 17:02 ` tip-bot2 for Tetsuo Handa
-1 siblings, 0 replies; 51+ messages in thread
From: tip-bot2 for Tetsuo Handa @ 2020-10-19 17:02 UTC (permalink / raw)
To: linux-tip-commits
Cc: syzbot, syzbot, Tetsuo Handa, stable, Greg Kroah-Hartman, x86, LKML
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: 6d389a66ccfc743699aa0274801654b6c7f9753b
Gitweb: https://git.kernel.org/tip/6d389a66ccfc743699aa0274801654b6c7f9753b
Author: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
AuthorDate: Sun, 27 Sep 2020 20:46:30 +09:00
Committer: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CommitterDate: Sat, 17 Oct 2020 08:31:22 +02:00
vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
commit 988d0763361bb65690d60e2bc53a6b72777040c3 upstream.
syzbot is reporting UAF/OOB read at bit_putcs()/soft_cursor() [1][2], for
vt_resizex() from ioctl(VT_RESIZEX) allows setting font height larger than
actual font height calculated by con_font_set() from ioctl(PIO_FONT).
Since fbcon_set_font() from con_font_set() allocates minimal amount of
memory based on actual font height calculated by con_font_set(),
use of vt_resizex() can cause UAF/OOB read for font data.
VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.
#define VT_RESIZE 0x5609 /* set kernel's idea of screensize */
#define VT_RESIZEX 0x560A /* set kernel's idea of screensize + more */
So far we are not aware of syzbot reports caused by setting non-zero value
to v_vlin parameter. But given that it is possible that nobody is using
VT_RESIZEX, we can try removing support for v_clin and v_vlin parameters.
Therefore, this patch effectively makes VT_RESIZEX behave like VT_RESIZE,
with emitting a message if somebody is still using v_clin and/or v_vlin
parameters.
[1] https://syzkaller.appspot.com/bug?id=32577e96d88447ded2d3b76d71254fb855245837
[2] https://syzkaller.appspot.com/bug?id=6b8355d27b2b94fb5cedf4655e3a59162d9e48e3
Reported-by: syzbot <syzbot+b308f5fd049fbbc6e74f@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+16469b5e8e5a72e9131e@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/4933b81b-9b1a-355b-df0e-9b31e8280ab9@i-love.sakura.ne.jp
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/tty/vt/vt_ioctl.c | 57 ++++++--------------------------------
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520b..bc33938 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -773,58 +773,21 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
if (copy_from_user(&v, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
-
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
- }
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
- }
- }
-
- if (v.v_clin > 32)
- return -EINVAL;
+ if (v.v_vlin)
+ pr_info_once("\"struct vt_consize\"->v_vlin is ignored. Please report if you need this.\n");
+ if (v.v_clin)
+ pr_info_once("\"struct vt_consize\"->v_clin is ignored. Please report if you need this.\n");
+ console_lock();
for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
+ vc = vc_cons[i].d;
- if (!vc_cons[i].d)
- continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
- vcp->vc_scan_lines = v.v_vlin;
- if (v.v_clin)
- vcp->vc_font.height = v.v_clin;
- vcp->vc_resize_user = 1;
- ret = vc_resize(vcp, v.v_cols, v.v_rows);
- if (ret) {
- vcp->vc_scan_lines = save_scan_lines;
- vcp->vc_font.height = save_font_height;
- console_unlock();
- return ret;
- }
+ if (vc) {
+ vc->vc_resize_user = 1;
+ vc_resize(vc, v.v_cols, v.v_rows);
}
- console_unlock();
}
+ console_unlock();
return 0;
}
^ permalink raw reply related [flat|nested] 51+ messages in thread