Re: [PATCH 2/2] 9p/trans_fd: put worker reqs on destroy

From: Tomas Bortoli
Date: Tue Oct 09 2018 - 09:19:57 EST


Il giorno mar 9 ott 2018 alle ore 06:06 Dominique Martinet
<asmadeus@xxxxxxxxxxxxx> ha scritto:
>
> From: Dominique Martinet <dominique.martinet@xxxxxx>
>
> p9_read_work/p9_write_work might still hold references to a req after
> having been cancelled; make sure we put any of these to avoid potential
> request leak on disconnect.
>
> Fixes: 728356dedeff8 ("9p: Add refcount to p9_req_t")
> Signed-off-by: Dominique Martinet <dominique.martinet@xxxxxx>
> Cc: Eric Van Hensbergen <ericvh@xxxxxxxxx>
> Cc: Latchesar Ionkov <lucho@xxxxxxxxxx>
> Cc: Tomas Bortoli <tomasbortoli@xxxxxxxxx>
Reviewed-by: Tomas Bortoli <tomasbortoli@xxxxxxxxx>
> ---
> Noticed we could leak a ref while looking at the syzbot report,
> this should be safe enough after the work has been cancelled...
> Probably.
>
> net/9p/trans_fd.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
> index a0317d459cde..f868cf6fba79 100644
> --- a/net/9p/trans_fd.c
> +++ b/net/9p/trans_fd.c
> @@ -876,7 +876,15 @@ static void p9_conn_destroy(struct p9_conn *m)
>
> p9_mux_poll_stop(m);
> cancel_work_sync(&m->rq);
> + if (m->rreq) {
> + p9_req_put(m->rreq);
> + m->rreq = NULL;
> + }
> cancel_work_sync(&m->wq);
> + if (m->wreq) {
> + p9_req_put(m->wreq);
> + m->wreq = NULL;
> + }
>
> p9_conn_cancel(m, -ECONNRESET);
>
> --
> 2.19.1
>

LGTM
--
Tomas