[PATCH V3] tty: fix memleak in alloc_pid

2014-04-14 Thread Chen Tingjie
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:
[] kmemleak_alloc+0x3c/0xa0
[] kmem_cache_alloc+0xc6/0x190
[] alloc_pid+0x1e/0x400
[] copy_process.part.39+0xad4/0x1120
[] do_fork+0x99/0x330
[] sys_fork+0x28/0x30
[] syscall_call+0x7/0xb
[] 0x

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(>sighand->siglock);
put_pid(current->signal->tty_old_pgrp);
current->signal->tty_old_pgrp = NULL;
spin_unlock_irq(>sighand->siglock);

spin_lock_irq(>sighand->siglock);
...
p->signal->tty = NULL;
...
spin_unlock_irq(>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: Zhang Jun 
Signed-off-by: Chen Tingjie 
---
 drivers/tty/tty_io.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 31c4a57..72547a3 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -874,9 +874,8 @@ void disassociate_ctty(int on_exit)
spin_lock_irq(>sighand->siglock);
put_pid(current->signal->tty_old_pgrp);
current->signal->tty_old_pgrp = NULL;
-   spin_unlock_irq(>sighand->siglock);
 
-   tty = get_current_tty();
+   tty = tty_kref_get(current->signal->tty);
if (tty) {
unsigned long flags;
spin_lock_irqsave(>ctrl_lock, flags);
@@ -893,6 +892,7 @@ void disassociate_ctty(int on_exit)
 #endif
}
 
+   spin_unlock_irq(>sighand->siglock);
/* Now clear signal->tty under the lock */
read_lock(_lock);
session_clear_tty(task_session(current));
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH V3] tty: fix memleak in alloc_pid

2014-04-14 Thread Chen Tingjie
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
[] 0x

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: Zhang Jun jun.zh...@intel.com
Signed-off-by: Chen Tingjie tingjie.c...@intel.com
---
 drivers/tty/tty_io.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 31c4a57..72547a3 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -874,9 +874,8 @@ void disassociate_ctty(int on_exit)
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);
 
-   tty = get_current_tty();
+   tty = tty_kref_get(current-signal-tty);
if (tty) {
unsigned long flags;
spin_lock_irqsave(tty-ctrl_lock, flags);
@@ -893,6 +892,7 @@ void disassociate_ctty(int on_exit)
 #endif
}
 
+   spin_unlock_irq(current-sighand-siglock);
/* Now clear signal-tty under the lock */
read_lock(tasklist_lock);
session_clear_tty(task_session(current));
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/