On Sun, Jul 26, 2020 at 8:30 AM Xin Xiong <xiong...@fudan.edu.cn> 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>
This SoB chain is out of order. If you are the author, your SoB should go first, if you are a commiter, the From line should correspond to the first SoB (not yours), if it's a group of authors (funny for one-/twoliner) then you consider to use Co-developed-by. Please, read Submitting Patches document. ... > put_pid(p->signal->tty_old_pgrp); /* A noop */ > spin_lock(&tty->ctrl_lock); > + if (tty_pgrp) > + put_pid(tty_pgrp); > tty_pgrp = get_pid(tty->pgrp); > if (tty->pgrp) > p->signal->tty_old_pgrp = get_pid(tty->pgrp); I guess this patch wasn't thought thru. You see the get_pid for it happens twice in a row. Perhaps you have to get the logic behind all these first? P.S. ...on top of what Greg said. -- With Best Regards, Andy Shevchenko
