On Thu, 2020-06-04 at 22:49 +0800, Zheng Bin wrote:
> If RPC use udp as it's transport protocol, transport->connect_worker
> will call xs_udp_setup_socket.
> xs_udp_setup_socket
> sock = xs_create_sock
> if (IS_ERR(sock))
> goto out;
> out:
> xprt_unlock_connect
> xprt_schedule_autodisconnect
> mod_timer
> internal_add_timer -->insert xprt->timer to base timer
> list
>
> xs_destroy
> cancel_delayed_work_sync(&transport->connect_worker)
> xs_xprt_free(xprt) -->free xprt
>
> Thus use-after-free will happen.
>
> Signed-off-by: Zheng Bin <zhengbin13@xxxxxxxxxx>
> ---
> net/sunrpc/xprtsock.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 845d0be805ec..c796808e7f7a 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1242,6 +1242,7 @@ static void xs_destroy(struct rpc_xprt *xprt)
> dprintk("RPC: xs_destroy xprt %p\n", xprt);
>
> cancel_delayed_work_sync(&transport->connect_worker);
> + del_timer_sync(&xprt->timer);
> xs_close(xprt);
> cancel_work_sync(&transport->recv_worker);
> cancel_work_sync(&transport->error_worker);
> --
> 2.26.0.106.g9fadedd
>
I'm confused. How can this happen given that xprt_destroy() first takes
the XPRT_LOCK, and then deletes xprt->timer?
Right now, the socket code knows nothing about the details of xprt-
>timer and what it is used for. I'd prefer to keep it that way if
possible.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
