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.
This caused a NULL pointer dereference in mlx5 which removed
the support for LOCAL_DMA_LKEY.
Fixes: bb6c96d72879 ("xprtrdma: Replace global lkey with lkey local to PD")
Signed-off-by: Sagi Grimberg <[email protected]>
---
net/sunrpc/xprtrdma/verbs.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index eb081ad..7efd9ef 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -543,11 +543,8 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
}
if (memreg == RPCRDMA_FRMR) {
- /* Requires both frmr reg and local dma lkey */
- if (((devattr->device_cap_flags &
- (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) !=
- (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) ||
- (devattr->max_fast_reg_page_list_len == 0)) {
+ if (!(devattr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) ||
+ (devattr->max_fast_reg_page_list_len == 0)) {
dprintk("RPC: %s: FRMR registration "
"not supported by HCA\n", __func__);
memreg = RPCRDMA_MTHCAFMR;
--
1.7.1
Hi Sagi-
> On Oct 6, 2015, at 5:48 AM, Sagi Grimberg <[email protected]> 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.
> 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?
> Fixes: bb6c96d72879 ("xprtrdma: Replace global lkey with lkey local to PD")
> Signed-off-by: Sagi Grimberg <[email protected]>
> ---
> net/sunrpc/xprtrdma/verbs.c | 7 ++-----
> 1 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index eb081ad..7efd9ef 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -543,11 +543,8 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
> }
>
> if (memreg == RPCRDMA_FRMR) {
> - /* Requires both frmr reg and local dma lkey */
> - if (((devattr->device_cap_flags &
> - (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) !=
> - (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) ||
> - (devattr->max_fast_reg_page_list_len == 0)) {
> + if (!(devattr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) ||
> + (devattr->max_fast_reg_page_list_len == 0)) {
> dprintk("RPC: %s: FRMR registration "
> "not supported by HCA\n", __func__);
> memreg = RPCRDMA_MTHCAFMR;
Reviewed-by: Chuck Lever <[email protected]>
—
Chuck Lever
On 10/6/2015 6:05 PM, Chuck Lever wrote:
> Hi Sagi-
>
>
>> On Oct 6, 2015, at 5:48 AM, Sagi Grimberg <[email protected]> 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?
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.
> On Oct 6, 2015, at 12:27 PM, Sagi Grimberg <[email protected]> wrote:
>
>> On 10/6/2015 6:05 PM, Chuck Lever wrote:
>> Hi Sagi-
>>
>>
>>> On Oct 6, 2015, at 5:48 AM, Sagi Grimberg <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html