| Message ID | 20221119043437.1396270-1-zhangxiaoxu5@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xprtrdma: Fix regbuf data not freed in rpcrdma_req_create() | expand |
> On Nov 18, 2022, at 11:34 PM, Zhang Xiaoxu <zhangxiaoxu5@huawei.com> wrote: > > If rdma receive buffer allocate failed, should call rpcrdma_regbuf_free() > to free the send buffer, otherwise, the buffer data will be leaked. > > Fixes: 8cec3dba76a4 ("xprtrdma: rpcrdma_regbuf alignment") Actually Fixes: bb93a1ae2bf4 ("xprtrdma: Allocate req's regbufs at xprt create time") might be better. bb93a1ae2bf4 is the commit that incorrectly added the kfree() to rpcrdma_req_create(). Even though 8cec3dba76a4 is what split the regbufs into two allocations, your fix isn't applicable to the rpcrdma_req_create() that's in 8cec3dba76a4. > Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> I'm guessing the error path is getting exercised more once f20f18c95630 ("xprtrdma: Prevent memory allocations from driving a reclaim") has been applied. Good catch! Reviewed-by: Chuck Lever <chuck.lever@oracle.com> > --- > net/sunrpc/xprtrdma/verbs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c > index 44b87e4274b4..b098fde373ab 100644 > --- a/net/sunrpc/xprtrdma/verbs.c > +++ b/net/sunrpc/xprtrdma/verbs.c > @@ -831,7 +831,7 @@ struct rpcrdma_req *rpcrdma_req_create(struct rpcrdma_xprt *r_xprt, > return req; > > out3: > - kfree(req->rl_sendbuf); > + rpcrdma_regbuf_free(req->rl_sendbuf); > out2: > kfree(req); > out1: > -- > 2.31.1 > -- Chuck Lever
On 2022/11/20 0:13, Chuck Lever III wrote: > > >> On Nov 18, 2022, at 11:34 PM, Zhang Xiaoxu <zhangxiaoxu5@huawei.com> wrote: >> >> If rdma receive buffer allocate failed, should call rpcrdma_regbuf_free() >> to free the send buffer, otherwise, the buffer data will be leaked. >> >> Fixes: 8cec3dba76a4 ("xprtrdma: rpcrdma_regbuf alignment") > > Actually Fixes: bb93a1ae2bf4 ("xprtrdma: Allocate req's regbufs > at xprt create time") might be better. bb93a1ae2bf4 is the commit > that incorrectly added the kfree() to rpcrdma_req_create(). > Even though 8cec3dba76a4 is what split the regbufs into two > allocations, your fix isn't applicable to the rpcrdma_req_create() > that's in 8cec3dba76a4. >Thanks, I will change the fix tag and add your reviewed-by in v2. > >> Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> > > I'm guessing the error path is getting exercised more once > f20f18c95630 ("xprtrdma: Prevent memory allocations from driving > a reclaim") has been applied. Good catch! > > Reviewed-by: Chuck Lever <chuck.lever@oracle.com> > > >> --- >> net/sunrpc/xprtrdma/verbs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c >> index 44b87e4274b4..b098fde373ab 100644 >> --- a/net/sunrpc/xprtrdma/verbs.c >> +++ b/net/sunrpc/xprtrdma/verbs.c >> @@ -831,7 +831,7 @@ struct rpcrdma_req *rpcrdma_req_create(struct rpcrdma_xprt *r_xprt, >> return req; >> >> out3: >> - kfree(req->rl_sendbuf); >> + rpcrdma_regbuf_free(req->rl_sendbuf); >> out2: >> kfree(req); >> out1: >> -- >> 2.31.1 >> > > -- > Chuck Lever > > > >
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c index 44b87e4274b4..b098fde373ab 100644 --- a/net/sunrpc/xprtrdma/verbs.c +++ b/net/sunrpc/xprtrdma/verbs.c @@ -831,7 +831,7 @@ struct rpcrdma_req *rpcrdma_req_create(struct rpcrdma_xprt *r_xprt, return req; out3: - kfree(req->rl_sendbuf); + rpcrdma_regbuf_free(req->rl_sendbuf); out2: kfree(req); out1:
If rdma receive buffer allocate failed, should call rpcrdma_regbuf_free() to free the send buffer, otherwise, the buffer data will be leaked. Fixes: 8cec3dba76a4 ("xprtrdma: rpcrdma_regbuf alignment") Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> --- net/sunrpc/xprtrdma/verbs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)