Messages in this thread | | | From | Andy Shevchenko <> | Date | Sun, 26 Jul 2020 11:55:48 +0300 | Subject | Re: [PATCH] tty: fix pid refcount leak in tty_signal_session_leader |
| |
On Sun, Jul 26, 2020 at 8:30 AM Xin Xiong <xiongx18@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 <xiyuyang19@fudan.edu.cn> > Signed-off-by: Xin Tan <tanxin.ctf@gmail.com> > Signed-off-by: Xin Xiong <xiongx18@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
| |