Hi,
On 16/01/23 9:14 pm, Greg Kroah-Hartman wrote:
From: Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx>
[ Upstream commit 9181f40fb2952fd59ecb75e7158620c9c669eee3 ]
If rdma receive buffer allocate failed, should call rpcrdma_regbuf_free()
to free the send buffer, otherwise, the buffer data will be leaked.
Fixes: bb93a1ae2bf4 ("xprtrdma: Allocate req's regbufs at xprt create time")
Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx>
Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
net/sunrpc/xprtrdma/verbs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index 0f4d39fdb48f..e13115bbe719 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -1037,6 +1037,7 @@ struct rpcrdma_req *rpcrdma_req_create(struct rpcrdma_xprt *r_xprt, size_t size,
kfree(req->rl_sendbuf);
out3:
kfree(req->rl_rdmabuf);
+ rpcrdma_regbuf_free(req->rl_sendbuf);
I think this introduces a double free in rpcrdma_req_create() [5.4.y]
Copying the function from 5.4.229 post the above patch here.
Comments added with //// marker.
struct rpcrdma_req *rpcrdma_req_create(struct rpcrdma_xprt *r_xprt,
size_t size,
gfp_t flags)
{
struct rpcrdma_buffer *buffer = &r_xprt->rx_buf;
struct rpcrdma_regbuf *rb;
struct rpcrdma_req *req;
size_t maxhdrsize;
req = kzalloc(sizeof(*req), flags);
if (req == NULL)
goto out1;
/* Compute maximum header buffer size in bytes */
maxhdrsize = rpcrdma_fixed_maxsz + 3 +
r_xprt->rx_ia.ri_max_segs * rpcrdma_readchunk_maxsz;
maxhdrsize *= sizeof(__be32);
rb = rpcrdma_regbuf_alloc(__roundup_pow_of_two(maxhdrsize),
DMA_TO_DEVICE, flags);
if (!rb)
goto out2;
req->rl_rdmabuf = rb;
xdr_buf_init(&req->rl_hdrbuf, rdmab_data(rb), rdmab_length(rb));
req->rl_sendbuf = rpcrdma_regbuf_alloc(size, DMA_TO_DEVICE, flags);
if (!req->rl_sendbuf)
goto out3;
req->rl_recvbuf = rpcrdma_regbuf_alloc(size, DMA_NONE, flags);
if (!req->rl_recvbuf)
goto out4; ///// let us say we hit this.
INIT_LIST_HEAD(&req->rl_free_mrs);
INIT_LIST_HEAD(&req->rl_registered);
spin_lock(&buffer->rb_lock);
list_add(&req->rl_all, &buffer->rb_allreqs);
spin_unlock(&buffer->rb_lock);
return req;
out4:
kfree(req->rl_sendbuf);
//// free of (req->rl_sendbuf)
out3:
kfree(req->rl_rdmabuf);
rpcrdma_regbuf_free(req->rl_sendbuf);
//// double free of req->rl_sendbuf, we have a kfree in rpcrdma_regbuf_free.
out2:
kfree(req);
out1:
return NULL;
}
Found using smatch.
I looked at the history of the function, the reason is that we don't
have commit b78de1dca00376aaba7a58bb5fe21c1606524abe in 5.4.y
This problem is only in 5.4.y not seen in newer LTS.
Please correct me if I am missing something here.
Thanks,
Harshit
out2:
kfree(req);
out1:
