| Message ID | 20190105130648.GC3288@kadam (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] xprtrdma: Fix error code in rpcrdma_buffer_create() | expand |
> On Jan 5, 2019, at 8:06 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > > The clean up is handled by the caller, rpcrdma_buffer_create(), so this > call to rpcrdma_sendctxs_destroy() leads to a double free. True. This fix is adequate, but I'm wondering if rpcrdma_sendctxs_destroy should be made more careful about being called twice. Hm. Reviewed-by: Chuck Lever <chuck.lever@oracle.com> > Fixes: ae72950abf99 ("xprtrdma: Add data structure to manage RDMA Send arguments") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > net/sunrpc/xprtrdma/verbs.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c > index 3dde05892c8e..4994e75945b8 100644 > --- a/net/sunrpc/xprtrdma/verbs.c > +++ b/net/sunrpc/xprtrdma/verbs.c > @@ -845,17 +845,13 @@ static int rpcrdma_sendctxs_create(struct rpcrdma_xprt *r_xprt) > for (i = 0; i <= buf->rb_sc_last; i++) { > sc = rpcrdma_sendctx_create(&r_xprt->rx_ia); > if (!sc) > - goto out_destroy; > + return -ENOMEM; > > sc->sc_xprt = r_xprt; > buf->rb_sc_ctxs[i] = sc; > } > > return 0; > - > -out_destroy: > - rpcrdma_sendctxs_destroy(buf); > - return -ENOMEM; > } > > /* The sendctx queue is not guaranteed to have a size that is a > -- > 2.17.1 > -- Chuck Lever
On Sat, Jan 05, 2019 at 11:24:45AM -0500, Chuck Lever wrote: > > > On Jan 5, 2019, at 8:06 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > The clean up is handled by the caller, rpcrdma_buffer_create(), so this > > call to rpcrdma_sendctxs_destroy() leads to a double free. > > True. This fix is adequate, but I'm wondering if rpcrdma_sendctxs_destroy > should be made more careful about being called twice. Hm. > > Reviewed-by: Chuck Lever <chuck.lever@oracle.com> I'm assuming Trond or Anna will pick this up.--b. > > > > Fixes: ae72950abf99 ("xprtrdma: Add data structure to manage RDMA Send arguments") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > net/sunrpc/xprtrdma/verbs.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c > > index 3dde05892c8e..4994e75945b8 100644 > > --- a/net/sunrpc/xprtrdma/verbs.c > > +++ b/net/sunrpc/xprtrdma/verbs.c > > @@ -845,17 +845,13 @@ static int rpcrdma_sendctxs_create(struct rpcrdma_xprt *r_xprt) > > for (i = 0; i <= buf->rb_sc_last; i++) { > > sc = rpcrdma_sendctx_create(&r_xprt->rx_ia); > > if (!sc) > > - goto out_destroy; > > + return -ENOMEM; > > > > sc->sc_xprt = r_xprt; > > buf->rb_sc_ctxs[i] = sc; > > } > > > > return 0; > > - > > -out_destroy: > > - rpcrdma_sendctxs_destroy(buf); > > - return -ENOMEM; > > } > > > > /* The sendctx queue is not guaranteed to have a size that is a > > -- > > 2.17.1 > > > > -- > Chuck Lever > >
On Sat, Jan 05, 2019 at 11:24:45AM -0500, Chuck Lever wrote: > > > On Jan 5, 2019, at 8:06 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > The clean up is handled by the caller, rpcrdma_buffer_create(), so this > > call to rpcrdma_sendctxs_destroy() leads to a double free. > > True. This fix is adequate, but I'm wondering if rpcrdma_sendctxs_destroy > should be made more careful about being called twice. Hm. > I actually wrote the patch like that originally, but then this way made for an easier patch description so I re-wrote it. Let me send the other patch and you can apply that or both if you want. regards, dan carpenter
On Mon, 2019-01-07 at 12:22 -0500, Bruce Fields wrote: > On Sat, Jan 05, 2019 at 11:24:45AM -0500, Chuck Lever wrote: > > > On Jan 5, 2019, at 8:06 AM, Dan Carpenter <dan.carpenter@oracle.com> > > > wrote: > > > > > > The clean up is handled by the caller, rpcrdma_buffer_create(), so this > > > call to rpcrdma_sendctxs_destroy() leads to a double free. > > > > True. This fix is adequate, but I'm wondering if rpcrdma_sendctxs_destroy > > should be made more careful about being called twice. Hm. > > > > Reviewed-by: Chuck Lever <chuck.lever@oracle.com> > > I'm assuming Trond or Anna will pick this up.--b. Yeah, I'll take this one and 1/2 for a 5.0-rc. I might save the additional cleanup patch Dan sent for 5.1 Anna > > > > > > Fixes: ae72950abf99 ("xprtrdma: Add data structure to manage RDMA Send > > > arguments") > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > --- > > > net/sunrpc/xprtrdma/verbs.c | 6 +----- > > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > > > diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c > > > index 3dde05892c8e..4994e75945b8 100644 > > > --- a/net/sunrpc/xprtrdma/verbs.c > > > +++ b/net/sunrpc/xprtrdma/verbs.c > > > @@ -845,17 +845,13 @@ static int rpcrdma_sendctxs_create(struct > > > rpcrdma_xprt *r_xprt) > > > for (i = 0; i <= buf->rb_sc_last; i++) { > > > sc = rpcrdma_sendctx_create(&r_xprt->rx_ia); > > > if (!sc) > > > - goto out_destroy; > > > + return -ENOMEM; > > > > > > sc->sc_xprt = r_xprt; > > > buf->rb_sc_ctxs[i] = sc; > > > } > > > > > > return 0; > > > - > > > -out_destroy: > > > - rpcrdma_sendctxs_destroy(buf); > > > - return -ENOMEM; > > > } > > > > > > /* The sendctx queue is not guaranteed to have a size that is a > > > -- > > > 2.17.1 > > > > > > > -- > > Chuck Lever > > > >
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c index 3dde05892c8e..4994e75945b8 100644 --- a/net/sunrpc/xprtrdma/verbs.c +++ b/net/sunrpc/xprtrdma/verbs.c @@ -845,17 +845,13 @@ static int rpcrdma_sendctxs_create(struct rpcrdma_xprt *r_xprt) for (i = 0; i <= buf->rb_sc_last; i++) { sc = rpcrdma_sendctx_create(&r_xprt->rx_ia); if (!sc) - goto out_destroy; + return -ENOMEM; sc->sc_xprt = r_xprt; buf->rb_sc_ctxs[i] = sc; } return 0; - -out_destroy: - rpcrdma_sendctxs_destroy(buf); - return -ENOMEM; } /* The sendctx queue is not guaranteed to have a size that is a
The clean up is handled by the caller, rpcrdma_buffer_create(), so this call to rpcrdma_sendctxs_destroy() leads to a double free. Fixes: ae72950abf99 ("xprtrdma: Add data structure to manage RDMA Send arguments") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- net/sunrpc/xprtrdma/verbs.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)