diff mbox

[rdma-rc] xprtrdma: Don't require LOCAL_DMA_LKEY support for fasterg

Message ID 5613F684.3020904@dev.mellanox.co.il (mailing list archive)
State Superseded
Headers show

Commit Message

Sagi Grimberg Oct. 6, 2015, 4:27 p.m. UTC
On 10/6/2015 6:05 PM, Chuck Lever wrote:
> Hi Sagi-
>
>
>> On Oct 6, 2015, at 5:48 AM, Sagi Grimberg <sagig@mellanox.com> wrote:
>>
>> There is no need to require LOCAL_DMA_LKEY support in order to
>> use fast registration as the PD allocation makes sure that there
>> is a local_dma_lkey.
>
> In other words, all devices now have a local DMA lkey, so the
> check is unnecessary.

Right.

>
>
>> This caused a NULL pointer dereference in mlx5 which removed
>> the support for LOCAL_DMA_LKEY.
>
> Where was the bad dereference? in mlx5, or in xprtrdma?

xprtrdma, ia->ri_ops wasn't assigned correctly.

Now that I look at it, one error path in rpcrdma_ia_open misses
an rc assignment. That needs to be fixed too, should it be in the same
patch?

         }
--

the incorrect requirement + missing rc caused the NULL deref of
ia->ri_ops.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Chuck Lever Oct. 6, 2015, 4:35 p.m. UTC | #1
> On Oct 6, 2015, at 12:27 PM, Sagi Grimberg <sagig@dev.mellanox.co.il> wrote:
> 
>> On 10/6/2015 6:05 PM, Chuck Lever wrote:
>> Hi Sagi-
>> 
>> 
>>> On Oct 6, 2015, at 5:48 AM, Sagi Grimberg <sagig@mellanox.com> wrote:
>>> 
>>> There is no need to require LOCAL_DMA_LKEY support in order to
>>> use fast registration as the PD allocation makes sure that there
>>> is a local_dma_lkey.
>> 
>> In other words, all devices now have a local DMA lkey, so the
>> check is unnecessary.
> 
> Right.
> 
>> 
>> 
>>> This caused a NULL pointer dereference in mlx5 which removed
>>> the support for LOCAL_DMA_LKEY.
>> 
>> Where was the bad dereference? in mlx5, or in xprtrdma?
> 
> xprtrdma, ia->ri_ops wasn't assigned correctly.
> 
> Now that I look at it, one error path in rpcrdma_ia_open misses
> an rc assignment. That needs to be fixed too, should it be in the same
> patch?

Go for it.


> 
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index 7efd9ef..81e8d31 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -554,6 +554,7 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
>                if (!ia->ri_device->alloc_fmr) {
>                        dprintk("RPC:       %s: MTHCAFMR registration "
>                                "not supported by HCA\n", __func__);
> +                       rc = -EINVAL;
>                        goto out3;
>                }
>        }
> --
> 
> the incorrect requirement + missing rc caused the NULL deref of
> ia->ri_ops.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index 7efd9ef..81e8d31 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -554,6 +554,7 @@  rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct 
sockaddr *addr, int memreg)
                 if (!ia->ri_device->alloc_fmr) {
                         dprintk("RPC:       %s: MTHCAFMR registration "
                                 "not supported by HCA\n", __func__);
+                       rc = -EINVAL;
                         goto out3;
                 }