Re: use-after-free in tty_check_change

From: Dmitry Vyukov
Date: Thu Dec 31 2015 - 05:02:27 EST

Next message: zhongjiang: "[PATCH] arm64: fix add kasan bug"
Previous message: James Liao: "Re: [PATCH 3/5] clk: mediatek: Add MT2701 clock support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, Nov 26, 2015 at 3:18 PM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote:
> On 11/26/2015 09:00 AM, Dmitry Vyukov wrote:
>> On Thu, Nov 26, 2015 at 2:29 PM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote:
>>> On 11/26/2015 08:09 AM, Dmitry Vyukov wrote:
>>>> On Thu, Nov 26, 2015 at 2:00 PM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> 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 <sys/types.h>
>>>>>> #include <sys/stat.h>
>>>>>> #include <fcntl.h>
>>>>>> #include <sys/ioctl.h>
>>>>>>
>>>>>> 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:
>>>>>> [<ffffffff81631fde>] __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:
>>>>>> [<ffffffff81631fde>] __asan_report_load8_noabort+0x3e/0x40
>>>>>> mm/kasan/report.c:280
>>>>>> [< inline >] tty_check_change drivers/tty/tty_io.c:399
>>>>>> [<ffffffff82a7b0a6>] 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
>>>>>> [<ffffffff816aea91>] do_vfs_ioctl+0x681/0xe40 fs/ioctl.c:572
>>>>>> [<ffffffff816af2df>] SyS_ioctl+0x8f/0xc0 fs/readdir.c:25
>>>>>> [<ffffffff85415cf6>] 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
>> [<ffffffff826c8be0>] dump_stack+0x44/0x64 lib/dump_stack.c:50
>> [<ffffffff815f1604>] print_trailer+0xf4/0x150 mm/slub.c:652
>> [<ffffffff815f775f>] object_err+0x2f/0x40 mm/slub.c:659
>> [< inline >] print_address_description mm/kasan/report.c:138
>> [<ffffffff815f9ee0>] kasan_report_error+0x210/0x520 mm/kasan/report.c:236
>> [< inline >] kasan_report mm/kasan/report.c:259
>> [<ffffffff815fa2ee>] __asan_report_load8_noabort+0x3e/0x40
>> mm/kasan/report.c:280
>> [<ffffffff829db6ec>] tty_ioctl+0x1d1c/0x1fd0 drivers/tty/tty_io.c:2881
>> [< inline >] vfs_ioctl fs/ioctl.c:43
>> [<ffffffff816708ac>] do_vfs_ioctl+0x57c/0xe60 fs/ioctl.c:607
>> [< inline >] SYSC_ioctl fs/ioctl.c:622
>> [<ffffffff81671204>] SyS_ioctl+0x74/0x80 fs/ioctl.c:613
>> [<ffffffff852a7876>] 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 <peter@xxxxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: zhongjiang: "[PATCH] arm64: fix add kasan bug"
Previous message: James Liao: "Re: [PATCH 3/5] clk: mediatek: Add MT2701 clock support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]