Message ID | 20220907071338.56969-1-niejianglei2021@163.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | SUNRPC: Fix potential memory leak in xs_udp_send_request() | expand |
On Wed, 2022-09-07 at 15:13 +0800, Jianglei Nie wrote: > xs_udp_send_request() allocates a memory chunk for xdr->bvec with > xdr_alloc_bvec(). When xprt_sock_sendmsg() finishs, xdr->bvec is not > released, which will lead to a memory leak. > > we should release the xdr->bvec with xdr_free_bvec() after > xprt_sock_sendmsg() like bc_sendto() does. > > Signed-off-by: Jianglei Nie <niejianglei2021@163.com> > --- > net/sunrpc/xprtsock.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c > index e976007f4fd0..298182a3c168 100644 > --- a/net/sunrpc/xprtsock.c > +++ b/net/sunrpc/xprtsock.c > @@ -958,6 +958,7 @@ static int xs_udp_send_request(struct rpc_rqst *req) > return status; > req->rq_xtime = ktime_get(); > status = xprt_sock_sendmsg(transport->sock, &msg, xdr, 0, 0, &sent); > + xdr_free_bvec(xdr); > > dprintk("RPC: xs_udp_send_request(%u) = %d\n", > xdr->len, status); I think you're probably correct here. I was thinking we might have a similar bug in svc_tcp_sendmsg, but it looks like that one gets freed in svc_tcp_sendto. Reviewed-by: Jeff Layton <jlayton@kernel.org>
On Wed, 2022-09-07 at 06:08 -0400, Jeff Layton wrote: > On Wed, 2022-09-07 at 15:13 +0800, Jianglei Nie wrote: > > xs_udp_send_request() allocates a memory chunk for xdr->bvec with > > xdr_alloc_bvec(). When xprt_sock_sendmsg() finishs, xdr->bvec is > > not > > released, which will lead to a memory leak. > > > > we should release the xdr->bvec with xdr_free_bvec() after > > xprt_sock_sendmsg() like bc_sendto() does. > > > > Signed-off-by: Jianglei Nie <niejianglei2021@163.com> > > --- > > net/sunrpc/xprtsock.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c > > index e976007f4fd0..298182a3c168 100644 > > --- a/net/sunrpc/xprtsock.c > > +++ b/net/sunrpc/xprtsock.c > > @@ -958,6 +958,7 @@ static int xs_udp_send_request(struct rpc_rqst > > *req) > > return status; > > req->rq_xtime = ktime_get(); > > status = xprt_sock_sendmsg(transport->sock, &msg, xdr, 0, > > 0, &sent); > > + xdr_free_bvec(xdr); > > > > dprintk("RPC: xs_udp_send_request(%u) = %d\n", > > xdr->len, status); > > I think you're probably correct here. > > I was thinking we might have a similar bug in svc_tcp_sendmsg, but it > looks like that one gets freed in svc_tcp_sendto. > > Reviewed-by: Jeff Layton <jlayton@kernel.org> No, this patch is unnecessary and won't be applied. We already do this for all transports in xprt_request_dequeue_transmit().
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index e976007f4fd0..298182a3c168 100644 --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -958,6 +958,7 @@ static int xs_udp_send_request(struct rpc_rqst *req) return status; req->rq_xtime = ktime_get(); status = xprt_sock_sendmsg(transport->sock, &msg, xdr, 0, 0, &sent); + xdr_free_bvec(xdr); dprintk("RPC: xs_udp_send_request(%u) = %d\n", xdr->len, status);
xs_udp_send_request() allocates a memory chunk for xdr->bvec with xdr_alloc_bvec(). When xprt_sock_sendmsg() finishs, xdr->bvec is not released, which will lead to a memory leak. we should release the xdr->bvec with xdr_free_bvec() after xprt_sock_sendmsg() like bc_sendto() does. Signed-off-by: Jianglei Nie <niejianglei2021@163.com> --- net/sunrpc/xprtsock.c | 1 + 1 file changed, 1 insertion(+)