[PATCH rdma-next 0/5] RDMA fixes for 4.16

[PATCH rdma-next 0/5] RDMA fixes for 4.16

[Leon Romanovsky]

Hi,

This is batch of unrelated fixes and two cleanup patches from Parav.

The fix from Jack is to well-known series 44c58487d51a ("IB/core:
Define 'ib' and 'roce' rdma_ah_attr types") and it is targeted for
stable@.

Thanks

Jack Morgenstein (1):
  IB/umad: Fix use of unprotected device pointer

Leon Romanovsky (2):
  RDMA/mlx5: Avoid memory leak in case of XRCD dealloc failure
  RDMA/cm: Fix access to uninitialized variable

Parav Pandit (2):
  IB/core: Avoid SGID attributes query while converting GID from OPA to
    IB
  RDMA/cma: Use existing netif_is_bond_master function

 drivers/infiniband/core/cm.c       | 6 +-----
 drivers/infiniband/core/cma.c      | 2 +-
 drivers/infiniband/core/user_mad.c | 2 +-
 drivers/infiniband/hw/mlx5/qp.c    | 5 +----
 4 files changed, 4 insertions(+), 11 deletions(-)

--
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 1/5] IB/umad: Fix use of unprotected device pointer

[Leon Romanovsky]

From: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>

The ib_write_umad() is protected by taking the umad file mutex.
However, it accesses file->port->ib_dev -- which is protected only by the
port's mutex (field file_mutex).

The ib_umad_remove_one() calls ib_umad_kill_port() which sets
port->ib_dev to NULL under the port mutex (NOT the file mutex).
It then sets the mad agent to "dead" under the umad file mutex.

This is a race condition -- because there is a window where
port->ib_dev is NULL, while the agent is not "dead".

As a result, we saw stack traces like:

[16490.678059] BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0
[16490.678246] IP: ib_umad_write+0x29c/0xa3a [ib_umad]
[16490.678333] PGD 0 P4D 0
[16490.678404] Oops: 0000 [#1] SMP PTI
[16490.678466] Modules linked in: rdma_ucm(OE) ib_ucm(OE) rdma_cm(OE) iw_cm(OE) ib_ipoib(OE) ib_cm(OE) ib_uverbs(OE) ib_umad(OE) mlx4_en(OE) ptp pps_core mlx4_ib(OE-) ib_core(OE) mlx4_core(OE) mlx_compat
(OE) memtrack(OE) devlink mst_pciconf(OE) mst_pci(OE) netconsole nfsv3 nfs_acl nfs lockd grace fscache cfg80211 rfkill esp6_offload esp6 esp4_offload esp4 sunrpc kvm_intel kvm ppdev parport_pc irqbypass
parport joydev i2c_piix4 virtio_balloon cirrus drm_kms_helper ttm drm e1000 serio_raw virtio_pci virtio_ring virtio ata_generic pata_acpi qemu_fw_cfg [last unloaded: mlxfw]
[16490.679202] CPU: 4 PID: 3115 Comm: sminfo Tainted: G           OE   4.14.13-300.fc27.x86_64 #1
[16490.679339] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu2 04/01/2014
[16490.679477] task: ffff9cf753890000 task.stack: ffffaf70c26b0000
[16490.679571] RIP: 0010:ib_umad_write+0x29c/0xa3a [ib_umad]
[16490.679664] RSP: 0018:ffffaf70c26b3d90 EFLAGS: 00010202
[16490.679747] RAX: 0000000000000010 RBX: ffff9cf75610fd80 RCX: 0000000000000000
[16490.679856] RDX: 0000000000000001 RSI: 00007ffdf2bfd714 RDI: ffff9cf6bb2a9c00

In the above trace, ib_umad_write is trying to dereference the NULL
file->port->ib_dev pointer.

Fix this by using the agent's device pointer (the device field
in struct ib_mad_agent) -- which IS protected by the umad file mutex.

Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v4.11
Fixes: 44c58487d51a ("IB/core: Define 'ib' and 'roce' rdma_ah_attr types")
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/user_mad.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index f0ed883492ec..5e8ffd12c04e 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -511,7 +511,7 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
 	}
 
 	memset(&ah_attr, 0, sizeof ah_attr);
-	ah_attr.type = rdma_ah_find_type(file->port->ib_dev,
+	ah_attr.type = rdma_ah_find_type(agent->device,
 					 file->port->port_num);
 	rdma_ah_set_dlid(&ah_attr, be16_to_cpu(packet->mad.hdr.lid));
 	rdma_ah_set_sl(&ah_attr, packet->mad.hdr.sl);
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 2/5] RDMA/mlx5: Avoid memory leak in case of XRCD dealloc failure

[Leon Romanovsky]

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Failure in XRCD FW deallocation command leaves memory leaked and
returns error to the user which he can't do anything about it.

This patch changes behavior to always free memory and always return
success to the user.

Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters")
Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/qp.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index b06a2f9a8df4..39d24bf694a8 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -5066,13 +5066,10 @@ int mlx5_ib_dealloc_xrcd(struct ib_xrcd *xrcd)
 	int err;

 	err = mlx5_core_xrcd_dealloc(dev->mdev, xrcdn);
-	if (err) {
+	if (err)
 		mlx5_ib_warn(dev, "failed to dealloc xrcdn 0x%x\n", xrcdn);
-		return err;
-	}

 	kfree(xrcd);
-
 	return 0;
 }

--
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 3/5] IB/core: Avoid SGID attributes query while converting GID from OPA to IB

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

SGID attributes are not used during OPA to IB GID conversion.
Therefore don't query it.

Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index b8f8d3128a53..68df93558d80 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1577,15 +1577,13 @@ static void cm_opa_to_ib_sgid(struct cm_work *work,
 			      struct sa_path_rec *path)
 {
 	struct ib_device *dev = work->port->cm_dev->ib_device;
-	struct ib_gid_attr gid_attr;
 	u8 port_num = work->port->port_num;
 
 	if (rdma_cap_opa_ah(dev, port_num) &&
 	    (ib_is_opa_gid(&path->sgid))) {
 		union ib_gid sgid;
 
-		if (ib_get_cached_gid(dev, port_num, 0,
-				      &sgid, &gid_attr)) {
+		if (ib_get_cached_gid(dev, port_num, 0, &sgid, NULL)) {
 			dev_warn(&dev->dev,
 				 "Error updating sgid in CM request\n");
 			return;
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 4/5] RDMA/cma: Use existing netif_is_bond_master function

[Leon Romanovsky]

From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

When checking whatever the current netdev is the bond master interface,
use kernel API netif_is_bond_master() instead of hardcoding the check.
No functionality is changed.

Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/cma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 84812b9dea9b..e66963ca58bd 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -4294,7 +4294,7 @@ static int cma_netdev_callback(struct notifier_block *self, unsigned long event,
 	if (event != NETDEV_BONDING_FAILOVER)
 		return NOTIFY_DONE;
 
-	if (!(ndev->flags & IFF_MASTER) || !(ndev->priv_flags & IFF_BONDING))
+	if (!netif_is_bond_master(ndev))
 		return NOTIFY_DONE;
 
 	mutex_lock(&lock);
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma-next 5/5] RDMA/cm: Fix access to uninitialized variable

[Leon Romanovsky]

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The ndev will be held only for successful ib_get_cached_gid()
and hence no need to use dev_put() in failure path.

Fixes: 16c72e402867 ("IB/cm: Refactor to avoid setting path record software only fields")
Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/cm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 68df93558d80..e6749157fd86 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1901,8 +1901,6 @@ static int cm_req_handler(struct cm_work *work)
 				grh->sgid_index,
 				&gid, &gid_attr);
 	if (ret) {
-		if (gid_attr.ndev)
-			dev_put(gid_attr.ndev);
 		ib_send_cm_rej(cm_id, IB_CM_REJ_UNSUPPORTED, NULL, 0, NULL, 0);
 		goto rejected;
 	}
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 2/5] RDMA/mlx5: Avoid memory leak in case of XRCD dealloc failure

[Yuval Shaia]

On Sun, Jan 28, 2018 at 11:25:30AM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Failure in XRCD FW deallocation command leaves memory leaked and
> returns error to the user which he can't do anything about it.
> 
> This patch changes behavior to always free memory and always return
> success to the user.
> 
> Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters")
> Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/hw/mlx5/qp.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> index b06a2f9a8df4..39d24bf694a8 100644
> --- a/drivers/infiniband/hw/mlx5/qp.c
> +++ b/drivers/infiniband/hw/mlx5/qp.c
> @@ -5066,13 +5066,10 @@ int mlx5_ib_dealloc_xrcd(struct ib_xrcd *xrcd)
>  	int err;
> 
>  	err = mlx5_core_xrcd_dealloc(dev->mdev, xrcdn);
> -	if (err) {
> +	if (err)
>  		mlx5_ib_warn(dev, "failed to dealloc xrcdn 0x%x\n", xrcdn);
> -		return err;
> -	}
> 
>  	kfree(xrcd);
> -
>  	return 0;

In this case suggesting to make the function return void.

>  }
> 
> --
> 2.16.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 3/5] IB/core: Avoid SGID attributes query while converting GID from OPA to IB

[Yuval Shaia]

On Sun, Jan 28, 2018 at 11:25:31AM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> SGID attributes are not used during OPA to IB GID conversion.
> Therefore don't query it.
> 
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/cm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index b8f8d3128a53..68df93558d80 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -1577,15 +1577,13 @@ static void cm_opa_to_ib_sgid(struct cm_work *work,
>  			      struct sa_path_rec *path)
>  {
>  	struct ib_device *dev = work->port->cm_dev->ib_device;
> -	struct ib_gid_attr gid_attr;
>  	u8 port_num = work->port->port_num;
>  
>  	if (rdma_cap_opa_ah(dev, port_num) &&
>  	    (ib_is_opa_gid(&path->sgid))) {
>  		union ib_gid sgid;
>  
> -		if (ib_get_cached_gid(dev, port_num, 0,
> -				      &sgid, &gid_attr)) {
> +		if (ib_get_cached_gid(dev, port_num, 0, &sgid, NULL)) {
>  			dev_warn(&dev->dev,
>  				 "Error updating sgid in CM request\n");
>  			return;

Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

> -- 
> 2.16.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 4/5] RDMA/cma: Use existing netif_is_bond_master function

[Yuval Shaia]

On Sun, Jan 28, 2018 at 11:25:32AM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> When checking whatever the current netdev is the bond master interface,
> use kernel API netif_is_bond_master() instead of hardcoding the check.
> No functionality is changed.
> 
> Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/infiniband/core/cma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 84812b9dea9b..e66963ca58bd 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -4294,7 +4294,7 @@ static int cma_netdev_callback(struct notifier_block *self, unsigned long event,
>  	if (event != NETDEV_BONDING_FAILOVER)
>  		return NOTIFY_DONE;
>  
> -	if (!(ndev->flags & IFF_MASTER) || !(ndev->priv_flags & IFF_BONDING))
> +	if (!netif_is_bond_master(ndev))
>  		return NOTIFY_DONE;

Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

>  
>  	mutex_lock(&lock);
> -- 
> 2.16.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 5/5] RDMA/cm: Fix access to uninitialized variable

[Yuval Shaia]

On Sun, Jan 28, 2018 at 11:25:33AM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The ndev will be held only for successful ib_get_cached_gid()
> and hence no need to use dev_put() in failure path.

More than that - in case of an error from ib_get_cached_gid, gid_attr.ndev
is still not initialized.

> 
> Fixes: 16c72e402867 ("IB/cm: Refactor to avoid setting path record software only fields")
> Signed-off-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/core/cm.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index 68df93558d80..e6749157fd86 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -1901,8 +1901,6 @@ static int cm_req_handler(struct cm_work *work)
>  				grh->sgid_index,
>  				&gid, &gid_attr);
>  	if (ret) {
> -		if (gid_attr.ndev)
> -			dev_put(gid_attr.ndev);
>  		ib_send_cm_rej(cm_id, IB_CM_REJ_UNSUPPORTED, NULL, 0, NULL, 0);
>  		goto rejected;
>  	}

Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

> -- 
> 2.16.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 2/5] RDMA/mlx5: Avoid memory leak in case of XRCD dealloc failure

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1760 bytes --]

On Sun, Jan 28, 2018 at 01:07:18PM +0200, Yuval Shaia wrote:
> On Sun, Jan 28, 2018 at 11:25:30AM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Failure in XRCD FW deallocation command leaves memory leaked and
> > returns error to the user which he can't do anything about it.
> >
> > This patch changes behavior to always free memory and always return
> > success to the user.
> >
> > Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters")
> > Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/infiniband/hw/mlx5/qp.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> > index b06a2f9a8df4..39d24bf694a8 100644
> > --- a/drivers/infiniband/hw/mlx5/qp.c
> > +++ b/drivers/infiniband/hw/mlx5/qp.c
> > @@ -5066,13 +5066,10 @@ int mlx5_ib_dealloc_xrcd(struct ib_xrcd *xrcd)
> >  	int err;
> >
> >  	err = mlx5_core_xrcd_dealloc(dev->mdev, xrcdn);
> > -	if (err) {
> > +	if (err)
> >  		mlx5_ib_warn(dev, "failed to dealloc xrcdn 0x%x\n", xrcdn);
> > -		return err;
> > -	}
> >
> >  	kfree(xrcd);
> > -
> >  	return 0;
>
> In this case suggesting to make the function return void.

It is part of global flow:
ib_uverbs_dealloc_xrcd -> ib_dealloc_xrcd -> xrcd->device->dealloc_xrcd

>
> >  }
> >
> > --
> > 2.16.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 2/5] RDMA/mlx5: Avoid memory leak in case of XRCD dealloc failure

[Yuval Shaia]

On Sun, Jan 28, 2018 at 02:52:08PM +0200, Leon Romanovsky wrote:
> On Sun, Jan 28, 2018 at 01:07:18PM +0200, Yuval Shaia wrote:
> > On Sun, Jan 28, 2018 at 11:25:30AM +0200, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > >
> > > Failure in XRCD FW deallocation command leaves memory leaked and
> > > returns error to the user which he can't do anything about it.
> > >
> > > This patch changes behavior to always free memory and always return
> > > success to the user.
> > >
> > > Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters")
> > > Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > ---
> > >  drivers/infiniband/hw/mlx5/qp.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> > > index b06a2f9a8df4..39d24bf694a8 100644
> > > --- a/drivers/infiniband/hw/mlx5/qp.c
> > > +++ b/drivers/infiniband/hw/mlx5/qp.c
> > > @@ -5066,13 +5066,10 @@ int mlx5_ib_dealloc_xrcd(struct ib_xrcd *xrcd)
> > >  	int err;
> > >
> > >  	err = mlx5_core_xrcd_dealloc(dev->mdev, xrcdn);
> > > -	if (err) {
> > > +	if (err)
> > >  		mlx5_ib_warn(dev, "failed to dealloc xrcdn 0x%x\n", xrcdn);
> > > -		return err;
> > > -	}
> > >
> > >  	kfree(xrcd);
> > > -
> > >  	return 0;
> >
> > In this case suggesting to make the function return void.
> 
> It is part of global flow:
> ib_uverbs_dealloc_xrcd -> ib_dealloc_xrcd -> xrcd->device->dealloc_xrcd

I see, missed that code path.

But for my understanding, as of now - all implementations of this hook
(two) returns 0 always, right?

Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

> 
> >
> > >  }
> > >
> > > --
> > > 2.16.1
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 2/5] RDMA/mlx5: Avoid memory leak in case of XRCD dealloc failure

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2492 bytes --]

On Sun, Jan 28, 2018 at 03:25:58PM +0200, Yuval Shaia wrote:
> On Sun, Jan 28, 2018 at 02:52:08PM +0200, Leon Romanovsky wrote:
> > On Sun, Jan 28, 2018 at 01:07:18PM +0200, Yuval Shaia wrote:
> > > On Sun, Jan 28, 2018 at 11:25:30AM +0200, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > >
> > > > Failure in XRCD FW deallocation command leaves memory leaked and
> > > > returns error to the user which he can't do anything about it.
> > > >
> > > > This patch changes behavior to always free memory and always return
> > > > success to the user.
> > > >
> > > > Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters")
> > > > Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > ---
> > > >  drivers/infiniband/hw/mlx5/qp.c | 5 +----
> > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> > > > index b06a2f9a8df4..39d24bf694a8 100644
> > > > --- a/drivers/infiniband/hw/mlx5/qp.c
> > > > +++ b/drivers/infiniband/hw/mlx5/qp.c
> > > > @@ -5066,13 +5066,10 @@ int mlx5_ib_dealloc_xrcd(struct ib_xrcd *xrcd)
> > > >  	int err;
> > > >
> > > >  	err = mlx5_core_xrcd_dealloc(dev->mdev, xrcdn);
> > > > -	if (err) {
> > > > +	if (err)
> > > >  		mlx5_ib_warn(dev, "failed to dealloc xrcdn 0x%x\n", xrcdn);
> > > > -		return err;
> > > > -	}
> > > >
> > > >  	kfree(xrcd);
> > > > -
> > > >  	return 0;
> > >
> > > In this case suggesting to make the function return void.
> >
> > It is part of global flow:
> > ib_uverbs_dealloc_xrcd -> ib_dealloc_xrcd -> xrcd->device->dealloc_xrcd
>
> I see, missed that code path.
>
> But for my understanding, as of now - all implementations of this hook
> (two) returns 0 always, right?

Right, both mlx4 and mlx5 always return 0, and it is related to mistake
in verbs API - various free and destroy functions return errors without
actual need for it.

>
> Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Thanks

>
> >
> > >
> > > >  }
> > > >
> > > > --
> > > > 2.16.1
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > > > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-next 2/5] RDMA/mlx5: Avoid memory leak in case of XRCD dealloc failure

[Jason Gunthorpe]

On Sun, Jan 28, 2018 at 03:37:55PM +0200, Leon Romanovsky wrote:
> On Sun, Jan 28, 2018 at 03:25:58PM +0200, Yuval Shaia wrote:
> > On Sun, Jan 28, 2018 at 02:52:08PM +0200, Leon Romanovsky wrote:
> > > On Sun, Jan 28, 2018 at 01:07:18PM +0200, Yuval Shaia wrote:
> > > > On Sun, Jan 28, 2018 at 11:25:30AM +0200, Leon Romanovsky wrote:
> > > > > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > >
> > > > > Failure in XRCD FW deallocation command leaves memory leaked and
> > > > > returns error to the user which he can't do anything about it.
> > > > >
> > > > > This patch changes behavior to always free memory and always return
> > > > > success to the user.
> > > > >
> > > > > Fixes: e126ba97dba9 ("mlx5: Add driver for Mellanox Connect-IB adapters")
> > > > > Reviewed-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > > Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > > >  drivers/infiniband/hw/mlx5/qp.c | 5 +----
> > > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> > > > > index b06a2f9a8df4..39d24bf694a8 100644
> > > > > +++ b/drivers/infiniband/hw/mlx5/qp.c
> > > > > @@ -5066,13 +5066,10 @@ int mlx5_ib_dealloc_xrcd(struct ib_xrcd *xrcd)
> > > > >  	int err;
> > > > >
> > > > >  	err = mlx5_core_xrcd_dealloc(dev->mdev, xrcdn);
> > > > > -	if (err) {
> > > > > +	if (err)
> > > > >  		mlx5_ib_warn(dev, "failed to dealloc xrcdn 0x%x\n", xrcdn);
> > > > > -		return err;
> > > > > -	}
> > > > >
> > > > >  	kfree(xrcd);
> > > > > -
> > > > >  	return 0;
> > > >
> > > > In this case suggesting to make the function return void.
> > >
> > > It is part of global flow:
> > > ib_uverbs_dealloc_xrcd -> ib_dealloc_xrcd -> xrcd->device->dealloc_xrcd
> >
> > I see, missed that code path.
> >
> > But for my understanding, as of now - all implementations of this hook
> > (two) returns 0 always, right?
> 
> Right, both mlx4 and mlx5 always return 0, and it is related to mistake
> in verbs API - various free and destroy functions return errors without
> actual need for it.

It would be nice to send another patch to remove the return code.

Long term I want to have all driver dealloc's unfailable..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma-next 0/5] RDMA fixes for 4.16

[Jason Gunthorpe]

On Sun, Jan 28, 2018 at 11:25:28AM +0200, Leon Romanovsky wrote:
> Hi,
> 
> This is batch of unrelated fixes and two cleanup patches from Parav.
> 
> The fix from Jack is to well-known series 44c58487d51a ("IB/core:
> Define 'ib' and 'roce' rdma_ah_attr types") and it is targeted for
> stable@.
> 
> Thanks
> 
> Jack Morgenstein (1):
>   IB/umad: Fix use of unprotected device pointer

FWIW, that would be a -rc patch if another -rc was still expected.

> Leon Romanovsky (2):
>   RDMA/mlx5: Avoid memory leak in case of XRCD dealloc failure
>   RDMA/cm: Fix access to uninitialized variable
> 
> Parav Pandit (2):
>   IB/core: Avoid SGID attributes query while converting GID from OPA to
>     IB
>   RDMA/cma: Use existing netif_is_bond_master function

Applied to for-next thanks

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html