Hello,
syzbot found the following issue on:
HEAD commit: 2eb29d59ddf0 Merge tag 'drm-next-2023-03-03-1' of git://an..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=12e88ebcc80000
kernel config: https://syzkaller.appspot.com/x/.config?x=cab35c936731a347
dashboard link: https://syzkaller.appspot.com/bug?extid=3af17071816b61e807ed
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10b71504c80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16f02d9cc80000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/ea23ee201337/disk-2eb29d59.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/40a9db02dede/vmlinux-2eb29d59.xz
kernel image: https://storage.googleapis.com/syzbot-assets/8d4b574642d0/bzImage-2eb29d59.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]
usercopy: Kernel memory exposure attempt detected from page alloc (offset 0, size 4194560)!
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:102!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 5073 Comm: syz-executor309 Not tainted 6.2.0-syzkaller-13277-g2eb29d59ddf0 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/16/2023
RIP: 0010:usercopy_abort+0xb7/0xd0 mm/usercopy.c:102
Code: e8 fe e8 a1 ff 49 89 d9 4c 89 e1 48 89 ee 41 56 48 c7 c7 00 73 5b 8a 41 55 41 57 4c 8b 44 24 20 48 8b 54 24 18 e8 59 8b 85 ff <0f> 0b 48 c7 c3 00 71 5b 8a 49 89 df 49 89 d8 e9 71 ff ff ff 0f 1f
RSP: 0018:ffffc90003bcf9e0 EFLAGS: 00010286
RAX: 000000000000005b RBX: ffffffff8a5b7100 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff816931fc RDI: 0000000000000005
RBP: ffffffff8a5b72c0 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000080000000 R11: 0000000000000000 R12: ffffffff8a5b7500
R13: 0000000000000000 R14: 0000000000400100 R15: ffffffff8a5b7100
FS: 0000555555b1b300(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000000 CR3: 0000000075655000 CR4: 0000000000350ee0
Call Trace:
<TASK>
check_heap_object mm/usercopy.c:200 [inline]
__check_object_size mm/usercopy.c:251 [inline]
__check_object_size+0x50a/0x6e0 mm/usercopy.c:213
check_object_size include/linux/thread_info.h:215 [inline]
check_copy_size include/linux/thread_info.h:251 [inline]
copy_to_user include/linux/uaccess.h:168 [inline]
con_font_get drivers/tty/vt/vt.c:4580 [inline]
con_font_op+0x397/0xf10 drivers/tty/vt/vt.c:4674
vt_k_ioctl drivers/tty/vt/vt_ioctl.c:474 [inline]
vt_ioctl+0x620/0x2df0 drivers/tty/vt/vt_ioctl.c:752
tty_ioctl+0x773/0x16e0 drivers/tty/tty_io.c:2777
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:870 [inline]
__se_sys_ioctl fs/ioctl.c:856 [inline]
__x64_sys_ioctl+0x197/0x210 fs/ioctl.c:856
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f6672fd82d9
Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc66955e38 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f6672fd82d9
RDX: 0000000020000000 RSI: 0000000000004b72 RDI: 0000000000000003
RBP: 00007f6672f9c0c0 R08: 000000000000000d R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f6672f9c150
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:usercopy_abort+0xb7/0xd0 mm/usercopy.c:102
Code: e8 fe e8 a1 ff 49 89 d9 4c 89 e1 48 89 ee 41 56 48 c7 c7 00 73 5b 8a 41 55 41 57 4c 8b 44 24 20 48 8b 54 24 18 e8 59 8b 85 ff <0f> 0b 48 c7 c3 00 71 5b 8a 49 89 df 49 89 d8 e9 71 ff ff ff 0f 1f
RSP: 0018:ffffc90003bcf9e0 EFLAGS: 00010286
RAX: 000000000000005b RBX: ffffffff8a5b7100 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff816931fc RDI: 0000000000000005
RBP: ffffffff8a5b72c0 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000080000000 R11: 0000000000000000 R12: ffffffff8a5b7500
R13: 0000000000000000 R14: 0000000000400100 R15: ffffffff8a5b7100
FS: 0000555555b1b300(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000000 CR3: 0000000075655000 CR4: 0000000000350ee0
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at [email protected].
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
On Fri, Mar 03, 2023 at 01:37:55PM -0800, syzbot wrote:
> dashboard link: https://syzkaller.appspot.com/bug?extid=3af17071816b61e807ed
> [...]
> usercopy: Kernel memory exposure attempt detected from page alloc (offset 0, size 4194560)!
> [...]
> Call Trace:
> <TASK>
> check_heap_object mm/usercopy.c:200 [inline]
> __check_object_size mm/usercopy.c:251 [inline]
> __check_object_size+0x50a/0x6e0 mm/usercopy.c:213
> check_object_size include/linux/thread_info.h:215 [inline]
> check_copy_size include/linux/thread_info.h:251 [inline]
> copy_to_user include/linux/uaccess.h:168 [inline]
> con_font_get drivers/tty/vt/vt.c:4580 [inline]
> con_font_op+0x397/0xf10 drivers/tty/vt/vt.c:4674
This is coming from the folio checking:
} else if (folio_test_large(folio)) {
offset = ptr - folio_address(folio);
if (n > folio_size(folio) - offset)
usercopy_abort("page alloc", NULL, to_user, offset, n);
}
triggered by copy_to_user of the font.data allocation:
#define max_font_width 64
#define max_font_height 128
#define max_font_glyphs 512
#define max_font_size (max_font_glyphs*max_font_width*max_font_height)
...
font.data = kvmalloc(max_font_size, GFP_KERNEL);
...
if (op->data && copy_to_user(op->data, font.data, c))
rc = -EFAULT;
it is correctly seeing "c" (4194560 in the report) as larger than
"max_font_size" (4194304, seen reported by "folio_size(folio)"). The
"c" calculation comes from:
unsigned int vpitch = op->op == KD_FONT_OP_GET_TALL ? op->height : 32;
...
rc = vc->vc_sw->con_font_get(vc, &font, vpitch);
...
c = (font.width+7)/8 * vpitch * font.charcount;
So yes, 4194560 is larger than 4194304, and a memory exposure was,
in fact, blocked here.
Given the recent work in this area, I'm not sure which calculation is
wrong, max_font_size or c. Samuel?
-Kees
--
Kees Cook
syzbot has bisected this issue to:
commit 24d69384bcd34b9dcaf5dab744bf7096e84d1abd
Author: Samuel Thibault <[email protected]>
Date: Thu Jan 19 15:19:16 2023 +0000
VT: Add KD_FONT_OP_SET/GET_TALL operations
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=120b3232c80000
start commit: 2eb29d59ddf0 Merge tag 'drm-next-2023-03-03-1' of git://an..
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=110b3232c80000
console output: https://syzkaller.appspot.com/x/log.txt?x=160b3232c80000
kernel config: https://syzkaller.appspot.com/x/.config?x=cab35c936731a347
dashboard link: https://syzkaller.appspot.com/bug?extid=3af17071816b61e807ed
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10b71504c80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16f02d9cc80000
Reported-by: [email protected]
Fixes: 24d69384bcd3 ("VT: Add KD_FONT_OP_SET/GET_TALL operations")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
Kees Cook, le ven. 03 mars 2023 14:07:04 -0800, a ecrit:
> #define max_font_width 64
> #define max_font_height 128
> #define max_font_glyphs 512
> #define max_font_size (max_font_glyphs*max_font_width*max_font_height)
> ...
> font.data = kvmalloc(max_font_size, GFP_KERNEL);
> ...
> if (op->data && copy_to_user(op->data, font.data, c))
> rc = -EFAULT;
>
> it is correctly seeing "c" (4194560 in the report) as larger than
> "max_font_size" (4194304, seen reported by "folio_size(folio)"). The
> "c" calculation comes from:
>
> unsigned int vpitch = op->op == KD_FONT_OP_GET_TALL ? op->height : 32;
> ...
> rc = vc->vc_sw->con_font_get(vc, &font, vpitch);
> ...
> c = (font.width+7)/8 * vpitch * font.charcount;
>
> So yes, 4194560 is larger than 4194304, and a memory exposure was,
> in fact, blocked here.
>
> Given the recent work in this area, I'm not sure which calculation is
> wrong, max_font_size or c. Samuel?
They are not wrong. It's the vpitch value (coming from userland's
op.height) which is out of bound and missing a check.
The patch below should be fixing it, could you check?
I don't know how I am supposed to properly reference the syzbot report
etc., could somebody used to the process handle submitting the fix?
Samuel
VT: Protect KD_FONT_OP_GET_TALL from unbound access
In ioctl(KD_FONT_OP_GET_TALL), userland tells through op->height which
vpitch should be used to copy over the font. In con_font_get, we were
not checking that it is within the maximum height value, and thus
userland could make the vc->vc_sw->con_font_get(vc, &font, vpitch);
call possibly overflow the allocated max_font_size bytes, and the
copy_to_user(op->data, font.data, c) call possibly read out of that
allocated buffer.
By checking vpitch against max_font_height, the max_font_size buffer
will always be large enough for the vc->vc_sw->con_font_get(vc, &font,
vpitch) call (since we already prevent loading a font larger than that),
and c = (font.width+7)/8 * vpitch * font.charcount will always remain
below max_font_size.
Fixes: 24d69384bcd3 ("VT: Add KD_FONT_OP_SET/GET_TALL operations")
Signed-off-by: Samuel Thibault <[email protected]>
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 57a5c23b51d4..3c2ea9c098f7 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4545,6 +4545,9 @@ static int con_font_get(struct vc_data *vc, struct console_font_op *op)
int c;
unsigned int vpitch = op->op == KD_FONT_OP_GET_TALL ? op->height : 32;
+ if (vpitch > max_font_height)
+ return -EINVAL;
+
if (op->data) {
font.data = kvmalloc(max_font_size, GFP_KERNEL);
if (!font.data)
On 05. 03. 23, 18:54, Samuel Thibault wrote:
> Kees Cook, le ven. 03 mars 2023 14:07:04 -0800, a ecrit:
>> #define max_font_width 64
>> #define max_font_height 128
>> #define max_font_glyphs 512
>> #define max_font_size (max_font_glyphs*max_font_width*max_font_height)
>> ...
>> font.data = kvmalloc(max_font_size, GFP_KERNEL);
>> ...
>> if (op->data && copy_to_user(op->data, font.data, c))
>> rc = -EFAULT;
>>
>> it is correctly seeing "c" (4194560 in the report) as larger than
>> "max_font_size" (4194304, seen reported by "folio_size(folio)"). The
>> "c" calculation comes from:
>>
>> unsigned int vpitch = op->op == KD_FONT_OP_GET_TALL ? op->height : 32;
>> ...
>> rc = vc->vc_sw->con_font_get(vc, &font, vpitch);
>> ...
>> c = (font.width+7)/8 * vpitch * font.charcount;
>>
>> So yes, 4194560 is larger than 4194304, and a memory exposure was,
>> in fact, blocked here.
>>
>> Given the recent work in this area, I'm not sure which calculation is
>> wrong, max_font_size or c. Samuel?
>
> They are not wrong. It's the vpitch value (coming from userland's
> op.height) which is out of bound and missing a check.
>
> The patch below should be fixing it, could you check?
>
> I don't know how I am supposed to properly reference the syzbot report
> etc., could somebody used to the process handle submitting the fix?
It's as simple as adding:
Reported-by: [email protected]
to the tags.
> VT: Protect KD_FONT_OP_GET_TALL from unbound access
>
> In ioctl(KD_FONT_OP_GET_TALL), userland tells through op->height which
> vpitch should be used to copy over the font. In con_font_get, we were
> not checking that it is within the maximum height value, and thus
> userland could make the vc->vc_sw->con_font_get(vc, &font, vpitch);
> call possibly overflow the allocated max_font_size bytes, and the
> copy_to_user(op->data, font.data, c) call possibly read out of that
> allocated buffer.
>
> By checking vpitch against max_font_height, the max_font_size buffer
> will always be large enough for the vc->vc_sw->con_font_get(vc, &font,
> vpitch) call (since we already prevent loading a font larger than that),
> and c = (font.width+7)/8 * vpitch * font.charcount will always remain
> below max_font_size.
>
> Fixes: 24d69384bcd3 ("VT: Add KD_FONT_OP_SET/GET_TALL operations")
Reviewed-by: Jiri Slaby <[email protected]>
> Signed-off-by: Samuel Thibault <[email protected]>
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 57a5c23b51d4..3c2ea9c098f7 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -4545,6 +4545,9 @@ static int con_font_get(struct vc_data *vc, struct console_font_op *op)
> int c;
> unsigned int vpitch = op->op == KD_FONT_OP_GET_TALL ? op->height : 32;
>
> + if (vpitch > max_font_height)
> + return -EINVAL;
> +
> if (op->data) {
> font.data = kvmalloc(max_font_size, GFP_KERNEL);
> if (!font.data)
--
js
Hi Samuel,
On Mon, Mar 6, 2023 at 8:36 AM Samuel Thibault
<[email protected]> wrote:
>
> The patch below should be fixing it, could you check?
>
> I don't know how I am supposed to properly reference the syzbot report
> etc., could somebody used to the process handle submitting the fix?
As Jiri Slaby correctly said above, you just need to add the
`Reported-by` tag from the syzbot bug report to your patch so that the
bot can recognize the fix later.
If you just want syzbot to check whether the reproducer still triggers
the bug after your changes, you can send an email with the `syz test`
command and the raw diff patch. Here are the instructions:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches
and here are many examples:
https://groups.google.com/g/syzkaller-bugs/search?q=%22%23syz%20test%22
--
Aleksandr
Aleksandr Nogikh, le lun. 06 mars 2023 11:28:04 +0100, a ecrit:
> On Mon, Mar 6, 2023 at 8:36 AM Samuel Thibault
> <[email protected]> wrote:
> >
> > The patch below should be fixing it, could you check?
> >
> > I don't know how I am supposed to properly reference the syzbot report
> > etc., could somebody used to the process handle submitting the fix?
>
> As Jiri Slaby correctly said above, you just need to add the
> `Reported-by` tag from the syzbot bug report to your patch so that the
> bot can recognize the fix later.
>
> If you just want syzbot to check whether the reproducer still triggers
> the bug after your changes, you can send an email with the `syz test`
> command and the raw diff patch. Here are the instructions:
> https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches
> and here are many examples:
> https://groups.google.com/g/syzkaller-bugs/search?q=%22%23syz%20test%22
Thanks! The patch does fix the reproducer case.
Samuel