Re: [PATCH] tty: plug a use-after-free in TIOCGETD ioctl

Mateusz Guzik
2016-01-07T16:21:26Z

On Thu, Jan 07, 2016 at 07:33:10AM -0800, Greg Kroah-Hartman wrote:
> On Thu, Jan 07, 2016 at 03:58:00PM +0100, Mateusz Guzik wrote:
> > When the line discipline is being changed, the old one is freed.
> > However, the handler for TIOCGETD would dereference it without taking
> > any locks, in effect possibly reading freed memory.
> >
> > Line discipline changes are protected with tty lock. Use it on reader
> > side as well.
> >
> > CVE: CVE-2016-0723
>
> Why a cve tag?
>

Red Hat SRT assigned a CVE and asked me to included in the commit
message. I did a quick check how people mark such stuff and found the
tag. I definitely don't insist on having it mentioned.

> > Found-by: Milos Vyletel <milos@redhat.com>
> > Signed-off-by: Mateusz Guzik <mguzik@redhat.com>
> > ---
> > drivers/tty/tty_io.c | 23 ++++++++++++++++++++++-
> > 1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> > index 892c923..1b10469 100644
> > --- a/drivers/tty/tty_io.c
> > +++ b/drivers/tty/tty_io.c
> > @@ -2626,6 +2626,27 @@ static int tiocgsid(struct tty_struct *tty, struct tty_struct *real_tty, pid_t _
> > }
> >
> > /**
> > + * tiocgetd - get line discipline
> > + * @tty: tty device
> > + * @p: pointer to returned line discipline
> > + *
> > + * Get the line discipline associated with the tty.
> > + *
> > + * Locking: none
> > + */
> > +
> > +static int tiocgetd(struct tty_struct *tty, int __user *p)
> > +{
> > + int ldisc;
> > +
> > + tty_lock(tty);
> > + ldisc = tty->ldisc->ops->num;
> > + tty_unlock(tty);
> > +
> > + return put_user(ldisc, p);
>
> Does this really protect anything? What is preventing ldisc from going
> away right after the tty_unlock call?

I guess I should have elaborated, sorry.

Yes, ldisc can be freed just after tty_unlock, but it does not matter.
There is only a need to store the number (which is done with the lock
held) and line discipline is not touched afterwards.

>
> And how are you able to trigger the tty to go away while the file is
> still held open and this ioctl is being called?
>

It's not the tty going away, but the memory pointed to by previous value
of tty->ldisc.

tty_set_ldisc will reassign tty->ldisc to a new value, and will later
free the old one with tty_ldisc_put.

In the current code TIOCGETD is:
return put_user(tty->ldisc->ops->num, (int __user *)p);

A thread doing this ioctl can load tty->ldisc's value, but memory
pointed to it can be freed before it loads ops's address.

The race can be triggered in few seconds on bare metal:

[ 428.520966] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
[ 428.529728] IP: [<ffffffff813b04cb>] tty_ioctl+0x89b/0xbc0
[ 428.535864] PGD 35a25067 PUD 36662067 PMD 0
[ 428.540659] Oops: 0000 [#1] SMP
[ 428.544277] Modules linked in: ppp_async ppp_generic slhc crc_ccitt intel_powerclamp coretemp intel_rapl kvm_intel kvm crc32_pclmul g
hash_clmulni_intel aesni_intel iTCO_wdt lrw gf128mul glue_helper ablk_helper ipmi_ssif cryptd iTCO_vendor_support sg ipmi_devintf dcdbas
sb_edac lpc_ich edac_core pcspkr mfd_core ipmi_si ipmi_msghandler mei_me mei wmi shpchp acpi_power_meter nfsd auth_rpcgss nfs_acl lockd
grace sunrpc ip_tables xfs sd_mod crc_t10dif crct10dif_generic mgag200 syscopyarea sysfillrect sysimgblt i2c_algo_bit drm_kms_helper tt
m bnx2x drm ahci mdio libahci crct10dif_pclmul crct10dif_common ptp libata i2c_core crc32c_intel megaraid_sas pps_core libcrc32c dm_mirr
or dm_region_hash dm_log dm_mod
[ 428.614136] CPU: 29 PID: 3149 Comm: a.out Not tainted 3.10.0-327.el7.x86_64 #1
[ 428.622201] Hardware name: Dell Inc. PowerEdge M630/0JXJPT, BIOS 1.2.5 05/04/2015
[ 428.630557] task: ffff8804654aae00 ti: ffff880036720000 task.ti: ffff880036720000
[ 428.638912] RIP: 0010:[<ffffffff813b04cb>] [<ffffffff813b04cb>] tty_ioctl+0x89b/0xbc0
[ 428.647762] RSP: 0018:ffff880036723e18 EFLAGS: 00010246
[ 428.653690] RAX: 0000000000000000 RBX: 0000000000005424 RCX: 00007ffcafb2209c
[ 428.661658] RDX: ffffffff818a4231 RSI: ffff8808663762a0 RDI: ffff88046804dc00
[ 428.669625] RBP: ffff880036723eb0 R08: 0000000000000000 R09: 0000000000000000
[ 428.677593] R10: 00007ffcafb21e00 R11: 0000000000000000 R12: ffff88046804dc00
[ 428.685560] R13: ffff880461449700 R14: 00007ffcafb2209c R15: ffff88046804ec00
[ 428.693529] FS: 00007fabcdefa740(0000) GS:ffff88086edc0000(0000) knlGS:0000000000000000
[ 428.702563] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 428.708978] CR2: 0000000000000010 CR3: 0000000035b10000 CR4: 00000000001407e0
[ 428.716945] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 428.724912] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 428.732880] Stack:
[ 428.735122] ffffffff812881d6 0000000000000010 0000000000000246 ffff880036723e40
[ 428.743420] 0000000000000018 00007ffcafb21e00 0000000000000000 ffff880036723ec8
[ 428.751718] ffff880036723ec8 0007ffff00000001 fffdffef00000000 0000000000000001
[ 428.760018] Call Trace:
[ 428.762749] [<ffffffff812881d6>] ? avc_has_perm_flags+0x96/0x1a0
[ 428.769557] [<ffffffff811f1ef5>] do_vfs_ioctl+0x2e5/0x4c0
[ 428.775685] [<ffffffff8128bc6e>] ? file_has_perm+0xae/0xc0
[ 428.781907] [<ffffffff811f2171>] SyS_ioctl+0xa1/0xc0
[ 428.787550] [<ffffffff81645909>] system_call_fastpath+0x16/0x1b
[ 428.794256] Code: ff ff ff 84 d2 0f 84 05 fc ff ff 31 f6 4c 89 e7 e8 fb d3 ff ff 31 c0 e9 f4 fb ff ff 0f 1f 40 00 49 8b 44 24 50 4c 8
9 f1 48 8b 00 <8b> 40 10 e8 4d 12 f5 ff 48 98 e9 d6 fb ff ff 66 0f 1f 44 00 00
[ 428.815957] RIP [<ffffffff813b04cb>] tty_ioctl+0x89b/0xbc0
[ 428.822188] RSP <ffff880036723e18>

To confirm the kernel really crashed while dereferncing tty->ldisc:

0xffffffff813b04bc <tty_ioctl+0x88c>: nopl 0x0(%rax)
0xffffffff813b04c0 <tty_ioctl+0x890>: mov 0x50(%r12),%rax
0xffffffff813b04c5 <tty_ioctl+0x895>: mov %r14,%rcx
0xffffffff813b04c8 <tty_ioctl+0x898>: mov (%rax),%rax
0xffffffff813b04cb <tty_ioctl+0x89b>: mov 0x10(%rax),%eax

tty_ldisc is at offset 0x50 in tty_struct and ops is at offset 0x0 in
tty_ldisc.

--
Mateusz Guzik