Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755287AbbLaKCY (ORCPT ); Thu, 31 Dec 2015 05:02:24 -0500 Received: from mail-wm0-f41.google.com ([74.125.82.41]:34070 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751247AbbLaKCU (ORCPT ); Thu, 31 Dec 2015 05:02:20 -0500 MIME-Version: 1.0 In-Reply-To: <565714AD.8010906@hurleysoftware.com> References: <56570280.5030505@hurleysoftware.com> <5657091D.5010500@hurleysoftware.com> <565714AD.8010906@hurleysoftware.com> From: Dmitry Vyukov Date: Thu, 31 Dec 2015 11:01:59 +0100 Message-ID: Subject: Re: use-after-free in tty_check_change To: Peter Hurley , Greg Kroah-Hartman Cc: Jiri Slaby , LKML , syzkaller , Kostya Serebryany , Alexander Potapenko , Sasha Levin , Eric Dumazet Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 14986 Lines: 343 On Thu, Nov 26, 2015 at 3:18 PM, Peter Hurley wrote: > On 11/26/2015 09:00 AM, Dmitry Vyukov wrote: >> On Thu, Nov 26, 2015 at 2:29 PM, Peter Hurley wrote: >>> On 11/26/2015 08:09 AM, Dmitry Vyukov wrote: >>>> On Thu, Nov 26, 2015 at 2:00 PM, Peter Hurley wrote: >>>>> Hi Dmitry, >>>>> >>>>> On 11/26/2015 05:31 AM, Dmitry Vyukov wrote: >>>>>> Hello, >>>>>> >>>>>> The following program causes a use-after-free in tty_check_change: >>>>>> >>>>>> // autogenerated by syzkaller (http://github.com/google/syzkaller) >>>>>> #include >>>>>> #include >>>>>> #include >>>>>> #include >>>>>> >>>>>> void *thr(void *arg) >>>>>> { >>>>>> int x = 0; >>>>>> ioctl((int)(long)arg, TIOCGETD, &x); >>>>>> return 0; >>>>>> } >>>>>> >>>>>> void tty() >>>>>> { >>>>>> int fd = open("/dev/ptmx", O_RDONLY|O_SYNC); >>>>>> pthread_t th; >>>>>> pthread_create(&th, 0, thr, (void*)(long)fd); >>>>>> int x = 16; >>>>>> ioctl(fd, TIOCSETD, &x); >>>>>> pthread_join(th, 0); >>>>>> } >>>>>> >>>>>> int main() >>>>>> { >>>>>> int i; >>>>>> >>>>>> for (i = 0; i < 10; i++) { >>>>>> if (fork() == 0) { >>>>>> tty(); >>>>>> return 0; >>>>>> } >>>>>> } >>>>>> return 0; >>>>>> } >>>>>> >>>>>> >>>>>> ================================================================== >>>>>> BUG: KASAN: use-after-free in tty_ioctl+0x1f06/0x2140 at addr ffff880061aa0968 >>>>>> Read of size 8 by task a.out/6241 >>>>>> ============================================================================= >>>>>> BUG kmalloc-16 (Not tainted): kasan: bad access detected >>>>>> ----------------------------------------------------------------------------- >>>>> >>>>> Thanks for the report. I think I have the patch that fixes this in >>>>> my out-queue as part of a larger patchset. >>>>> >>>>> However, the stack backtrace below and the reported eip don't correspond >>>>> to the what I think the problem is: >>>>> >>>>> >>>>>> BUG: KASAN: use-after-free in tty_ioctl+0x1f06/0x2140 at addr ffff880061aa0968 >>>>> ^^^^^^^^^^^^^^ >>>>> this offset is off the end of compilation unit for me with gcc 4.8.4 >>>>> >>>>> Could you generate a mixed listing file of drivers/tty/tty_io.c and attach >>>>> it please? >>>> >>>> >>>> The source is plain 6ffeba9607343f15303a399bc402a538800d89d9, but I >>>> use CONFIG_KASAN=y, CONFIG_KASAN_INLINE=y and gcc version 6.0.0 >>>> 20151014. KASAN does affect generated code significantly. >>>> Line numbers in the report should be correct. >>> >>> current->signal? I doubt it. Also note the stack trace pulls the wrong >>> symbol: should be __tty_check_change on line 399, not tty_check_change(). >>> >>>>>> Call Trace: >>>>>> [] __asan_report_load8_noabort+0x3e/0x40 >>>>>> mm/kasan/report.c:280 >>>>>> [< inline >] tty_check_change drivers/tty/tty_io.c:399 >>> ^^^^^^^^^^^^ >>> | >>> +------------------------------------------+ >>> | | >>> $ git blame -L393,409 -s 6ffeba960734 -- drivers/tty/tty_io.c >>> 2812d9e9 drivers/tty/tty_io.c 393) int __tty_check_change(struct tty_struct *tty, int sig) >>> ^1da177e drivers/char/tty_io.c 394) { >>> 47f86834 drivers/char/tty_io.c 395) unsigned long flags; >>> 2812d9e9 drivers/tty/tty_io.c 396) struct pid *pgrp, *tty_pgrp; >>> 47f86834 drivers/char/tty_io.c 397) int ret = 0; >>> 47f86834 drivers/char/tty_io.c 398) >>> ^1da177e drivers/char/tty_io.c 399) if (current->signal->tty != tty) <<<==== ??? >>> ^1da177e drivers/char/tty_io.c 400) return 0; >>> 47f86834 drivers/char/tty_io.c 401) >>> 6719693c drivers/tty/tty_io.c 402) rcu_read_lock(); >>> 6719693c drivers/tty/tty_io.c 403) pgrp = task_pgrp(current); >>> 6719693c drivers/tty/tty_io.c 404) >>> 47f86834 drivers/char/tty_io.c 405) spin_lock_irqsave(&tty->ctrl_lock, flags); >>> 2812d9e9 drivers/tty/tty_io.c 406) tty_pgrp = tty->pgrp; >>> 9ffee4cb drivers/char/tty_io.c 407) spin_unlock_irqrestore(&tty->ctrl_lock, flags); >>> 6719693c drivers/tty/tty_io.c 408) >>> 2812d9e9 drivers/tty/tty_io.c 409) if (tty_pgrp && pgrp != tty->pgrp) { >>> >>> >>> The disassembly would really help. Feel free to send it to me off-list. >>> >>> Regards, >>> Peter Hurley >>> >>>>>> Disabling lock debugging due to kernel taint >>>>>> INFO: Allocated in tty_ldisc_get.part.3+0x66/0x140 age=6 cpu=3 pid=6230 >>>>>> [< none >] kmem_cache_alloc_trace+0x1cf/0x220 mm/slub.c:2589 >>>>>> [< none >] tty_ldisc_get.part.3+0x66/0x140 drivers/tty/tty_ldisc.c:172 >>>>>> [< none >] tty_set_ldisc+0x83d/0xa70 drivers/tty/tty_ldisc.c:574 >>>>>> [< none >] tty_ioctl+0xb2a/0x2140 drivers/tty/tty_io.c:2865 >>>>>> [< inline >] spin_unlock include/linux/spinlock.h:347 >>>>>> [< inline >] ioctl_fionbio fs/ioctl.c:492 >>>>>> [< none >] do_vfs_ioctl+0x681/0xe40 fs/ioctl.c:572 >>>>>> [< none >] SyS_ioctl+0x8f/0xc0 fs/readdir.c:25 >>>>>> [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a >>>>>> arch/x86/entry/entry_64.S:185 >>>>>> >>>>>> INFO: Freed in tty_set_ldisc+0x4c2/0xa70 age=12 cpu=3 pid=6230 >>>>>> [< none >] kfree+0x199/0x1b0 mm/slub.c:3632 >>>>>> [< inline >] tty_ldisc_restore drivers/tty/tty_ldisc.c:493 >>>>>> [< none >] tty_set_ldisc+0x4c2/0xa70 drivers/tty/tty_ldisc.c:571 >>>>>> [< none >] tty_ioctl+0xb2a/0x2140 drivers/tty/tty_io.c:2865 >>>>>> [< inline >] spin_unlock include/linux/spinlock.h:347 >>>>>> [< inline >] ioctl_fionbio fs/ioctl.c:492 >>>>>> [< none >] do_vfs_ioctl+0x681/0xe40 fs/ioctl.c:572 >>>>>> [< none >] SyS_ioctl+0x8f/0xc0 fs/readdir.c:25 >>>>>> [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a >>>>>> arch/x86/entry/entry_64.S:185 >>>>>> >>>>>> INFO: Slab 0xffffea000186a800 objects=23 used=19 fp=0xffff880061aa12d0 >>>>>> flags=0x5fffc0000004080 >>>>>> INFO: Object 0xffff880061aa0968 @offset=2408 fp=0xffff880061aa0810 >>>>>> CPU: 2 PID: 6241 Comm: a.out Tainted: G B 4.4.0-rc1+ #117 >>>>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >>>>>> 00000000ffffffff ffff88006db0fab8 ffffffff827450f6 ffff88003e807980 >>>>>> ffff880061aa0968 ffff880061aa0000 ffff88006db0fae8 ffffffff81629404 >>>>>> ffff88003e807980 ffffea000186a800 ffff880061aa0968 000000000000001b >>>>>> >>>>>> Call Trace: >>>>>> [] __asan_report_load8_noabort+0x3e/0x40 >>>>>> mm/kasan/report.c:280 >>>>>> [< inline >] tty_check_change drivers/tty/tty_io.c:399 >>>>>> [] tty_ioctl+0x1f06/0x2140 drivers/tty/tty_io.c:2831 >>>>>> [< inline >] spin_unlock include/linux/spinlock.h:347 >>>>>> [< inline >] ioctl_fionbio fs/ioctl.c:492 >>>>>> [] do_vfs_ioctl+0x681/0xe40 fs/ioctl.c:572 >>>>>> [] SyS_ioctl+0x8f/0xc0 fs/readdir.c:25 >>>>>> [] entry_SYSCALL_64_fastpath+0x16/0x7a >>>>>> arch/x86/entry/entry_64.S:185 >>>>>> ================================================================== >>>>>> >>>>>> >>>>>> On commit 6ffeba9607343f15303a399bc402a538800d89d9 (Nov 24). >> >> >> >> >> >> Uh, sorry, I used a wrong vmlinux for postprocessing. > > Ok; it is the problem I thought it was. > > I've attached the patch below from my out-queue for reference > (which I'll mark for -stable now). > > Regards, > Peter Hurley > >> Here is a proper report. >> drivers/tty/tty_io.c:2881 is: >> >> case TIOCGETD: >> return put_user(tty->ldisc->ops->num, (int __user *)p); >> >> >> >> >> BUG: KASAN: use-after-free in tty_ioctl+0x1d1c/0x1fd0 at addr ffff8800640bc810 >> Read of size 8 by task a.out/23957 >> ============================================================================= >> BUG kmalloc-16 (Tainted: G B ): kasan: bad access detected >> ----------------------------------------------------------------------------- >> >> INFO: Allocated in tty_ldisc_get.part.3+0x61/0x130 age=0 cpu=3 pid=23947 >> [< none >] ___slab_alloc+0x489/0x4e0 mm/slub.c:2438 >> [< none >] __slab_alloc+0x4c/0x90 mm/slub.c:2467 >> [< inline >] slab_alloc_node mm/slub.c:2530 >> [< inline >] slab_alloc mm/slub.c:2572 >> [< none >] kmem_cache_alloc_trace+0x1c6/0x210 mm/slub.c:2589 >> [< inline >] kmalloc include/linux/slab.h:458 >> [< none >] tty_ldisc_get.part.3+0x61/0x130 drivers/tty/tty_ldisc.c:171 >> [< inline >] tty_ldisc_get drivers/tty/tty_ldisc.c:190 >> [< inline >] tty_ldisc_restore drivers/tty/tty_ldisc.c:490 >> [< none >] tty_set_ldisc+0x74c/0x940 drivers/tty/tty_ldisc.c:571 >> [< inline >] tiocsetd drivers/tty/tty_io.c:2650 >> [< none >] tty_ioctl+0xace/0x1fd0 drivers/tty/tty_io.c:2883 >> [< inline >] vfs_ioctl fs/ioctl.c:43 >> [< none >] do_vfs_ioctl+0x57c/0xe60 fs/ioctl.c:607 >> [< inline >] SYSC_ioctl fs/ioctl.c:622 >> [< none >] SyS_ioctl+0x74/0x80 fs/ioctl.c:613 >> [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a >> arch/x86/entry/entry_64.S:185 >> >> INFO: Freed in tty_set_ldisc+0x423/0x940 age=0 cpu=3 pid=23947 >> [< none >] __slab_free+0x206/0x360 mm/slub.c:2648 (discriminator 1) >> [< inline >] slab_free mm/slub.c:2803 >> [< none >] kfree+0x1a2/0x1c0 mm/slub.c:3632 >> [< inline >] tty_ldisc_put drivers/tty/tty_ldisc.c:194 >> [< none >] tty_set_ldisc+0x423/0x940 drivers/tty/tty_ldisc.c:570 >> [< inline >] tiocsetd drivers/tty/tty_io.c:2650 >> [< none >] tty_ioctl+0xace/0x1fd0 drivers/tty/tty_io.c:2883 >> [< inline >] vfs_ioctl fs/ioctl.c:43 >> [< none >] do_vfs_ioctl+0x57c/0xe60 fs/ioctl.c:607 >> [< inline >] SYSC_ioctl fs/ioctl.c:622 >> [< none >] SyS_ioctl+0x74/0x80 fs/ioctl.c:613 >> [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a >> arch/x86/entry/entry_64.S:185 >> INFO: Slab 0xffffea0001902f00 objects=23 used=3 fp=0xffff8800640bc000 >> flags=0x5fffc0000004080 >> INFO: Object 0xffff8800640bc810 @offset=2064 fp=0xffff8800640bc000 >> >> Bytes b4 ffff8800640bc800: 15 1d 3b 00 01 00 00 00 5a 5a 5a 5a 5a 5a >> 5a 5a ..;.....ZZZZZZZZ >> Object ffff8800640bc810: a0 9b 3d 86 ff ff ff ff 60 ef 16 6d 00 88 ff >> ff ..=.....`..m.... >> Redzone ffff8800640bc820: cc cc cc cc cc cc cc cc >> ........ >> Padding ffff8800640bc960: 5a 5a 5a 5a 5a 5a 5a 5a >> ZZZZZZZZ >> CPU: 0 PID: 23957 Comm: a.out Tainted: G B 4.4.0-rc2+ #51 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 >> ffff8800640bc000 ffff88006127fab8 ffffffff826c8be0 ffff88003e807980 >> ffff88006127fae8 ffffffff815f1604 ffff88003e807980 ffffea0001902f00 >> ffff8800640bc810 0000000000005424 ffff88006127fb10 ffffffff815f775f >> Call Trace: >> [< inline >] __dump_stack lib/dump_stack.c:15 >> [] dump_stack+0x44/0x64 lib/dump_stack.c:50 >> [] print_trailer+0xf4/0x150 mm/slub.c:652 >> [] object_err+0x2f/0x40 mm/slub.c:659 >> [< inline >] print_address_description mm/kasan/report.c:138 >> [] kasan_report_error+0x210/0x520 mm/kasan/report.c:236 >> [< inline >] kasan_report mm/kasan/report.c:259 >> [] __asan_report_load8_noabort+0x3e/0x40 >> mm/kasan/report.c:280 >> [] tty_ioctl+0x1d1c/0x1fd0 drivers/tty/tty_io.c:2881 >> [< inline >] vfs_ioctl fs/ioctl.c:43 >> [] do_vfs_ioctl+0x57c/0xe60 fs/ioctl.c:607 >> [< inline >] SYSC_ioctl fs/ioctl.c:622 >> [] SyS_ioctl+0x74/0x80 fs/ioctl.c:613 >> [] entry_SYSCALL_64_fastpath+0x16/0x7a >> arch/x86/entry/entry_64.S:185 >> > > --- >% --- > Subject: [PATCH] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Hi Peter, Greg, I am testing on 8513342170278468bac126640a5d2d12ffbff106 (4.4-rc7) and still see this use-after-free. I think we need to merge this patch in main tree. It can crash kernel or leak arbitrary kernel data (we directly copy out data from freed object to user). > ioctl(TIOCGETD) retrieves the line discipline id directly from the > ldisc because the line discipline id (c_line) in termios is untrustworthy; > userspace may have set termios via ioctl(TCSETS*) without actually > changing the line discipline via ioctl(TIOCSETD). > > However, directly accessing the current ldisc via tty->ldisc is > unsafe; the ldisc ptr dereferenced may be stale if the line discipline > is changing via ioctl(TIOCSETD) or hangup. > > Wait for the line discipline reference (just like read() or write()) > to retrieve the "current" line discipline id. > > Signed-off-by: Peter Hurley > --- > drivers/tty/tty_io.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c > index 5f5928f..963d895 100644 > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -2659,6 +2659,28 @@ static int tiocsetd(struct tty_struct *tty, int __user *p) > } > > /** > + * tiocgetd - get line discipline > + * @tty: tty device > + * @p: pointer to user data > + * > + * Retrieves the line discipline id directly from the ldisc. > + * > + * Locking: waits for ldisc reference (in case the line discipline > + * is changing or the tty is being hungup) > + */ > + > +static int tiocgetd(struct tty_struct *tty, int __user *p) > +{ > + struct tty_ldisc *ld; > + int ret; > + > + ld = tty_ldisc_ref_wait(tty); > + ret = put_user(ld->ops->num, p); > + tty_ldisc_deref(ld); > + return ret; > +} > + > +/** > * send_break - performed time break > * @tty: device to break on > * @duration: timeout in mS > @@ -2884,7 +2906,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > case TIOCGSID: > return tiocgsid(tty, real_tty, p); > case TIOCGETD: > - return put_user(tty->ldisc->ops->num, (int __user *)p); > + return tiocgetd(tty, p); > case TIOCSETD: > return tiocsetd(tty, p); > case TIOCVHANGUP: > -- > 2.6.3 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/