Commit c70dbb1e authored by Chen Tingjie's avatar Chen Tingjie Committed by Greg Kroah-Hartman

tty: fix memleak in alloc_pid

There is memleak in alloc_pid:
------------------------------
unreferenced object 0xd3453a80 (size 64):
  comm "adbd", pid 1730, jiffies 66363 (age 6586.950s)
  hex dump (first 32 bytes):
    01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 40 c2 f6 d5 00 d3 25 c1 59 28 00 00  ....@.....%.Y(..
  backtrace:
    [<c1a6f15c>] kmemleak_alloc+0x3c/0xa0
    [<c1320546>] kmem_cache_alloc+0xc6/0x190
    [<c125d51e>] alloc_pid+0x1e/0x400
    [<c123d344>] copy_process.part.39+0xad4/0x1120
    [<c123da59>] do_fork+0x99/0x330
    [<c123dd58>] sys_fork+0x28/0x30
    [<c1a89a08>] syscall_call+0x7/0xb
    [<ffffffff>] 0xffffffff

the leak is due to unreleased pid->count, which execute in function:
get_pid()(pid->count++) and put_pid()(pid->count--).

The race condition as following:
task[dumpsys]               task[adbd]
in disassociate_ctty()      in tty_signal_session_leader()
-----------------------     -------------------------
tty = get_current_tty();
// tty is not NULL
...
spin_lock_irq(&current->sighand->siglock);
put_pid(current->signal->tty_old_pgrp);
current->signal->tty_old_pgrp = NULL;
spin_unlock_irq(&current->sighand->siglock);

                            spin_lock_irq(&p->sighand->siglock);
                            ...
                            p->signal->tty = NULL;
                            ...
                            spin_unlock_irq(&p->sighand->siglock);

tty = get_current_tty();
// tty NULL, goto else branch by accident.
if (tty) {
    ...
    put_pid(tty_session);
    put_pid(tty_pgrp);
    ...
} else {
    print msg
}

in task[dumpsys], in disassociate_ctty(), tty is set NULL by task[adbd],
tty_signal_session_leader(), then it goto else branch and lack of
put_pid(), cause memleak.

move spin_unlock(sighand->siglock) after get_current_tty() can avoid
the race and fix the memleak.
Signed-off-by: default avatarZhang Jun <jun.zhang@intel.com>
Signed-off-by: default avatarChen Tingjie <tingjie.chen@intel.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent b24313a8
...@@ -878,9 +878,8 @@ void disassociate_ctty(int on_exit) ...@@ -878,9 +878,8 @@ void disassociate_ctty(int on_exit)
spin_lock_irq(&current->sighand->siglock); spin_lock_irq(&current->sighand->siglock);
put_pid(current->signal->tty_old_pgrp); put_pid(current->signal->tty_old_pgrp);
current->signal->tty_old_pgrp = NULL; current->signal->tty_old_pgrp = NULL;
spin_unlock_irq(&current->sighand->siglock);
tty = get_current_tty(); tty = tty_kref_get(current->signal->tty);
if (tty) { if (tty) {
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&tty->ctrl_lock, flags); spin_lock_irqsave(&tty->ctrl_lock, flags);
...@@ -897,6 +896,7 @@ void disassociate_ctty(int on_exit) ...@@ -897,6 +896,7 @@ void disassociate_ctty(int on_exit)
#endif #endif
} }
spin_unlock_irq(&current->sighand->siglock);
/* Now clear signal->tty under the lock */ /* Now clear signal->tty under the lock */
read_lock(&tasklist_lock); read_lock(&tasklist_lock);
session_clear_tty(task_session(current)); session_clear_tty(task_session(current));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment