On Sun, Jul 26, 2020 at 01:28:04PM +0800, Xin Xiong wrote: > In the loop, every time when p->signal->leader is true, the function > tty_signal_session_leader() will invoke get_pid() and return a > reference of tty->pgrp with increased refcount to the local variable > tty_pgrp or return NULL if it fails. After finishing the loop, the > function invokes put_pid() for only once, decreasing the refcount that > tty_pgrp keeps. > > Refcount leaks may occur when the scenario that p->signal->leader is > true happens more than once. In this assumption, if the above scenario > happens n times in the loop, the function forgets to decrease the > refcount for n-1 times, which causes refcount leaks. > > Fix the issue by decreasing the current refcount of the local variable > tty_pgrp before assigning new objects to it. > > Signed-off-by: Xiyu Yang <xiyuyan...@fudan.edu.cn> > Signed-off-by: Xin Tan <tanxin....@gmail.com> > Signed-off-by: Xin Xiong <xiong...@fudan.edu.cn> > --- > drivers/tty/tty_jobctrl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/tty/tty_jobctrl.c b/drivers/tty/tty_jobctrl.c > index f8ed50a16848..9e6bf693ade1 100644 > --- a/drivers/tty/tty_jobctrl.c > +++ b/drivers/tty/tty_jobctrl.c > @@ -212,6 +212,8 @@ int tty_signal_session_leader(struct tty_struct *tty, int > exit_session) > __group_send_sig_info(SIGCONT, SEND_SIG_PRIV, p); > put_pid(p->signal->tty_old_pgrp); /* A noop */ > spin_lock(&tty->ctrl_lock); > + if (tty_pgrp) > + put_pid(tty_pgrp);
No need to check this before calling it. But, the real question is why is this needed now? Nothing has changed in this area of the kernel for a very long time, so how did things get broken here? How are you triggering this and what is the result when we have that additional reference? thanks, greg k-h
