[PATCH V3] 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:
[] 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/
RE: [PATCH] [PATCH V2] tty: memleak in alloc_pid
> > Change-Id: Ic960dda039c8f99aad3e0f4d176489a966c62f6a > > > > Why is this line here? > Tingjie, Greg is asking you the sentence of "Change-Id", which is not needed, > please remove it with one new patch. Sorry for mistaken, I will make a new patch for it. -- 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/
RE: [PATCH] [PATCH V2] tty: memleak in alloc_pid
Hello,
The related code in disassociate_ctty() as following:
---
/* sometimes current->signal->tty_old_pgrp is NULL(a stand for
tty_old_pgrp not NULL, !a for tty_old_pgrp is NULL) */
spin_lock_irq(>sighand->siglock);
put_pid(current->signal->tty_old_pgrp);// when a is NULL, will
not actually put_pid() by decrease the count
current->signal->tty_old_pgrp = NULL;
spin_unlock_irq(>sighand->siglock);
/* sometimes current->signal->tty is NULL (b stand for signal->tty, !b
for signal->tty is NULL) */
tty = get_current_tty();
if (tty) {
unsigned long flags;
spin_lock_irqsave(>ctrl_lock, flags);
put_pid(tty->session);
put_pid(tty->pgrp);
tty->session = NULL;
tty->pgrp = NULL;
spin_unlock_irqrestore(>ctrl_lock, flags);
tty_kref_put(tty);
} else {
#ifdef TTY_DEBUG_HANGUP
printk(KERN_DEBUG "error attempted to write to tty [0x%p]"
" = NULL", tty);
#endif
}
Commonly there are 2 normal case flow for the code:
1/ in task: getprop: !a --> b
This case, current->signal->tty_old_pgrp is NULL, but get_current_tty() is
not NULL,
Really put_pid() as following:
put_pid(tty->session);
put_pid(tty->pgrp);
2/ in task: dumpsys: a --> !b
This case, currnet->signal->tty_old_pgrp is not NULL, but get_current_tty()
is NULL,
Really put_pid() as following:
put_pid(current->signal->tty_old_pgrp);
There is one abnormal case:
3/ in task: dumpsys: !a -> !b
This case, because of race, current->signal->tty_old_pgrp and
get_current_tty() are NULL,
Lack of put_pid(), pid->count will not be 0 at last, this pid will be leak.
When read the current->signal->tty_old_pgrp and current->signal->tty,
The 2 operations should be atomic, which protected by spinlock:
current->sighand->siglock.
So the sentence: spin_unlock_irq(>sighand->siglock);
need to move down after tty has process completed.
Thanks,
-Original Message-
From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
Sent: Monday, April 14, 2014 7:47 PM
To: Chen, Tingjie
Cc: Jiri Slaby; linux-kernel@vger.kernel.org; Zhang, Jun
Subject: Re: [PATCH] [PATCH V2] tty: memleak in alloc_pid
On Mon, Apr 14, 2014 at 03:31:15PM +0800, Chen Tingjie wrote:
> 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.
>
> Change-Id: Ic960dda039c8f99aad3e0f4d176489a966c62f6a
Why is this line here?
--
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 V2] 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:
[] 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.
Change-Id: Ic960dda039c8f99aad3e0f4d176489a966c62f6a
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] [PATCH V2] tty: 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:
[] 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.
Change-Id: Ic960dda039c8f99aad3e0f4d176489a966c62f6a
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/
RE: [PATCH] [PATCH V2] tty: memleak in alloc_pid
Hello,
The related code in disassociate_ctty() as following:
---
/* sometimes current-signal-tty_old_pgrp is NULL(a stand for
tty_old_pgrp not NULL, !a for tty_old_pgrp is NULL) */
spin_lock_irq(current-sighand-siglock);
put_pid(current-signal-tty_old_pgrp);// when a is NULL, will
not actually put_pid() by decrease the count
current-signal-tty_old_pgrp = NULL;
spin_unlock_irq(current-sighand-siglock);
/* sometimes current-signal-tty is NULL (b stand for signal-tty, !b
for signal-tty is NULL) */
tty = get_current_tty();
if (tty) {
unsigned long flags;
spin_lock_irqsave(tty-ctrl_lock, flags);
put_pid(tty-session);
put_pid(tty-pgrp);
tty-session = NULL;
tty-pgrp = NULL;
spin_unlock_irqrestore(tty-ctrl_lock, flags);
tty_kref_put(tty);
} else {
#ifdef TTY_DEBUG_HANGUP
printk(KERN_DEBUG error attempted to write to tty [0x%p]
= NULL, tty);
#endif
}
Commonly there are 2 normal case flow for the code:
1/ in task: getprop: !a -- b
This case, current-signal-tty_old_pgrp is NULL, but get_current_tty() is
not NULL,
Really put_pid() as following:
put_pid(tty-session);
put_pid(tty-pgrp);
2/ in task: dumpsys: a -- !b
This case, currnet-signal-tty_old_pgrp is not NULL, but get_current_tty()
is NULL,
Really put_pid() as following:
put_pid(current-signal-tty_old_pgrp);
There is one abnormal case:
3/ in task: dumpsys: !a - !b
This case, because of race, current-signal-tty_old_pgrp and
get_current_tty() are NULL,
Lack of put_pid(), pid-count will not be 0 at last, this pid will be leak.
When read the current-signal-tty_old_pgrp and current-signal-tty,
The 2 operations should be atomic, which protected by spinlock:
current-sighand-siglock.
So the sentence: spin_unlock_irq(current-sighand-siglock);
need to move down after tty has process completed.
Thanks,
-Original Message-
From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
Sent: Monday, April 14, 2014 7:47 PM
To: Chen, Tingjie
Cc: Jiri Slaby; linux-kernel@vger.kernel.org; Zhang, Jun
Subject: Re: [PATCH] [PATCH V2] tty: memleak in alloc_pid
On Mon, Apr 14, 2014 at 03:31:15PM +0800, Chen Tingjie wrote:
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.
Change-Id: Ic960dda039c8f99aad3e0f4d176489a966c62f6a
Why is this line here?
--
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/
RE: [PATCH] [PATCH V2] tty: memleak in alloc_pid
Change-Id: Ic960dda039c8f99aad3e0f4d176489a966c62f6a Why is this line here? Tingjie, Greg is asking you the sentence of Change-Id, which is not needed, please remove it with one new patch. Sorry for mistaken, I will make a new patch for it. -- 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
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/
[PATCH] [PATCH V2] tty: 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
[] 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.
Change-Id: Ic960dda039c8f99aad3e0f4d176489a966c62f6a
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/
[PATCH V2] 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
[] 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.
Change-Id: Ic960dda039c8f99aad3e0f4d176489a966c62f6a
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/
[PATCH] 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:
[] 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--).
Tracking it, in disassociate_ctty(), when both
current->signal->tty_old_pgrp and current->signal->tty are NULL,
it will lack one put_pid() and cause pid leak.
In theory, set p->signal->tty to NULL and p->signal->tty_old_pgrp
should be atomic operation, can be seen in function:
tty_signal_session_leader(), but in disassociate_ctty(), it is not.
This lack of protection will sometimes cause the leak case.
Make sure the atomic for the two operations can fix this leak.
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 d3448a9..3411071 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -878,9 +878,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);
@@ -897,6 +896,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] 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
[] 0x
the leak is due to unreleased pid-count, which execute in function:
get_pid()(pid-count++) and put_pid()(pid-count--).
Tracking it, in disassociate_ctty(), when both
current-signal-tty_old_pgrp and current-signal-tty are NULL,
it will lack one put_pid() and cause pid leak.
In theory, set p-signal-tty to NULL and p-signal-tty_old_pgrp
should be atomic operation, can be seen in function:
tty_signal_session_leader(), but in disassociate_ctty(), it is not.
This lack of protection will sometimes cause the leak case.
Make sure the atomic for the two operations can fix this leak.
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 d3448a9..3411071 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -878,9 +878,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);
@@ -897,6 +896,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/
