[PATCH 2/2] ibacm: Incorrect list used for subnet list causes a segfault.

[PATCH 2/2] ibacm: Incorrect list used for subnet list causes a segfault.

[Michael J. Ruhl]

From: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

When using the ibacm from rdma-core commit bf03566e0aa976afc5df,
setting the provider keyword in the ibacm_opts.cfg file to something
other than 'default' will cause ibacm to segfault on startup:

ibacm[32739]: segfault at 302e40 ip 000000000040432d
sp 00007ffe4039e1c0 error 4 in ibacm[400000+c000]

To re-produce the segfault, change the provider keyword in
ibacm_opts.cfg from:

provider ibacmp default

to:

provider ibacmp 0xFE80000000000000

When adding subnets to a provider subnet list, the incorrect list is
used.  The list used is the provider_list (list of all providers)
rather than the (specific) provider subnet list.

This corrupts the provider_list, and causes ibacm to crash with the
above segfault.

Use the correct list when adding subnet information to a provider.

Fixes: 26e05f8304a506 ("ibacm: use ccan/list.h")
Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 ibacm/src/acm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
index 1ccef94..a67001d 100644
--- a/ibacm/src/acm.c
+++ b/ibacm/src/acm.c
@@ -2587,8 +2587,8 @@ static void acm_load_prov_config(void)
 					return;
 				}
 				subnet->subnet_prefix = htobe64(prefix);
-				list_add_after(&provider_list, &prov->entry,
-						&subnet->entry);
+				list_add_tail(&prov->subnet_list,
+					      &subnet->entry);
 			}
 		}
 	}

--
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 2/2] ibacm: Incorrect list used for subnet list causes a segfault.

[Leon Romanovsky]

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

On Fri, Oct 13, 2017 at 02:43:52PM -0400, Michael J. Ruhl wrote:
> From: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> When using the ibacm from rdma-core commit bf03566e0aa976afc5df,

That commit is unrelated to ibacm. I think that we can remove this line.

> setting the provider keyword in the ibacm_opts.cfg file to something
> other than 'default' will cause ibacm to segfault on startup:
>
> ibacm[32739]: segfault at 302e40 ip 000000000040432d
> sp 00007ffe4039e1c0 error 4 in ibacm[400000+c000]
>
> To re-produce the segfault, change the provider keyword in
> ibacm_opts.cfg from:
>
> provider ibacmp default
>
> to:
>
> provider ibacmp 0xFE80000000000000
>
> When adding subnets to a provider subnet list, the incorrect list is
> used.  The list used is the provider_list (list of all providers)
> rather than the (specific) provider subnet list.
>
> This corrupts the provider_list, and causes ibacm to crash with the
> above segfault.
>
> Use the correct list when adding subnet information to a provider.
>
> Fixes: 26e05f8304a506 ("ibacm: use ccan/list.h")
> Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  ibacm/src/acm.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
> index 1ccef94..a67001d 100644
> --- a/ibacm/src/acm.c
> +++ b/ibacm/src/acm.c
> @@ -2587,8 +2587,8 @@ static void acm_load_prov_config(void)
>  					return;
>  				}
>  				subnet->subnet_prefix = htobe64(prefix);
> -				list_add_after(&provider_list, &prov->entry,
> -						&subnet->entry);
> +				list_add_tail(&prov->subnet_list,
> +					      &subnet->entry);
>  			}
>  		}
>  	}
>
> --
> 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 --]

[PATCH 1/2] ibacm: Incorrect usage of BE byte order of MLID attach/detach_mcast()

[Michael J. Ruhl]

From: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

The MLID value passed to ibv_attach/detach_mcast() must be in host
byte order.

acmp.c incorrectly uses the big endian format when doing a multicast
attach/detach (join). Multicast packets are used to do name resolution
by the libibacmp library.

There are two possible results because of this issue.

If a kernel has commit 8561eae60ff9, the attach will fail with an
EINVAL.  ibacm will log this as a failure during the multicast join.

If a kernel does not have commit 8561eae60ff9, the attach will
complete successfully.  Packets sent to this address will be dropped
because the packet dlid value and the multicast address information
given by the attach will not match.

Update MLID usage to use the correct byte order.

Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 ibacm/prov/acmp/src/acmp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ibacm/prov/acmp/src/acmp.c b/ibacm/prov/acmp/src/acmp.c
index aa78416..78d9a29 100644
--- a/ibacm/prov/acmp/src/acmp.c
+++ b/ibacm/prov/acmp/src/acmp.c
@@ -732,7 +732,7 @@ static void acmp_process_join_resp(struct acm_sa_mad *sa_mad)
 			acm_log(0, "ERROR - unable to create ah\n");
 			goto out;
 		}
-		ret = ibv_attach_mcast(ep->qp, &mc_rec->mgid, mc_rec->mlid);
+		ret = ibv_attach_mcast(ep->qp, &dest->mgid, dest->av.dlid);
 		if (ret) {
 			acm_log(0, "ERROR - unable to attach QP to multicast group\n");
 			ibv_destroy_ah(dest->ah);
@@ -1429,7 +1429,7 @@ static void acmp_ep_join(struct acmp_ep *ep)
 
 	if (ep->mc_dest[0].state == ACMP_READY && ep->mc_dest[0].ah) {
 		ibv_detach_mcast(ep->qp, &ep->mc_dest[0].mgid,
-				 be16toh(ep->mc_dest[0].av.dlid));
+				 ep->mc_dest[0].av.dlid);
 		ibv_destroy_ah(ep->mc_dest[0].ah);
 		ep->mc_dest[0].ah = NULL;
 	}

--
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 2/2] ibacm: Incorrect list used for subnet list causes a segfault

[Michael J. Ruhl]

From: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Setting the provider keyword in the ibacm_opts.cfg file to something
other than 'default' will cause ibacm to segfault on startup:

ibacm[32739]: segfault at 302e40 ip 000000000040432d
sp 00007ffe4039e1c0 error 4 in ibacm[400000+c000]

To re-produce the segfault, change the provider keyword in
ibacm_opts.cfg from:

provider ibacmp default

to:

provider ibacmp 0xFE80000000000000

When adding subnets to a provider subnet list, the incorrect list is
used.  The list used is the provider_list (list of all providers)
rather than the (specific) provider subnet list.

This corrupts the provider_list, and causes ibacm to crash with the
above segfault.

Use the correct list when adding subnet information to a provider.

Fixes: 26e05f8304a506 ("ibacm: use ccan/list.h")
Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 ibacm/src/acm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
index 1ccef94..a67001d 100644
--- a/ibacm/src/acm.c
+++ b/ibacm/src/acm.c
@@ -2587,8 +2587,8 @@ static void acm_load_prov_config(void)
 					return;
 				}
 				subnet->subnet_prefix = htobe64(prefix);
-				list_add_after(&provider_list, &prov->entry,
-						&subnet->entry);
+				list_add_tail(&prov->subnet_list,
+					      &subnet->entry);
 			}
 		}
 	}

--
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 2/2] ibacm: Incorrect list used for subnet list causes a segfault

[Ruhl, Michael J]

Missed the versioning information.  Please ignore these two patches.

Resending with the correct information.

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Michael J. Ruhl
> Sent: Monday, October 16, 2017 10:20 AM
> To: Hefty, Sean <sean.hefty@intel.com>
> Cc: linux-rdma@vger.kernel.org; hal@dev.mellanox.co.il
> Subject: [PATCH 2/2] ibacm: Incorrect list used for subnet list causes a
> segfault
> 
> From: Michael J. Ruhl <michael.j.ruhl@intel.com>
> 
> Setting the provider keyword in the ibacm_opts.cfg file to something
> other than 'default' will cause ibacm to segfault on startup:
> 
> ibacm[32739]: segfault at 302e40 ip 000000000040432d
> sp 00007ffe4039e1c0 error 4 in ibacm[400000+c000]
> 
> To re-produce the segfault, change the provider keyword in
> ibacm_opts.cfg from:
> 
> provider ibacmp default
> 
> to:
> 
> provider ibacmp 0xFE80000000000000
> 
> When adding subnets to a provider subnet list, the incorrect list is
> used.  The list used is the provider_list (list of all providers)
> rather than the (specific) provider subnet list.
> 
> This corrupts the provider_list, and causes ibacm to crash with the
> above segfault.
> 
> Use the correct list when adding subnet information to a provider.
> 
> Fixes: 26e05f8304a506 ("ibacm: use ccan/list.h")
> Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  ibacm/src/acm.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
> index 1ccef94..a67001d 100644
> --- a/ibacm/src/acm.c
> +++ b/ibacm/src/acm.c
> @@ -2587,8 +2587,8 @@ static void acm_load_prov_config(void)
>  					return;
>  				}
>  				subnet->subnet_prefix = htobe64(prefix);
> -				list_add_after(&provider_list, &prov->entry,
> -						&subnet->entry);
> +				list_add_tail(&prov->subnet_list,
> +					      &subnet->entry);
>  			}
>  		}
>  	}
> 
> --
> 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

RE: [PATCH 1/2] ibacm: Incorrect usage of BE byte order of MLID attach/detach_mcast()

[Ruhl, Michael J]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2863 bytes --]

Missed the versioning information, please ignore this patch.

Resending with correct info.

Mike


> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Michael J. Ruhl
> Sent: Monday, October 16, 2017 10:20 AM
> To: Hefty, Sean <sean.hefty@intel.com>
> Cc: linux-rdma@vger.kernel.org; hal@dev.mellanox.co.il
> Subject: [PATCH 1/2] ibacm: Incorrect usage of BE byte order of MLID
> attach/detach_mcast()
> 
> From: Michael J. Ruhl <michael.j.ruhl@intel.com>
> 
> The MLID value passed to ibv_attach/detach_mcast() must be in host
> byte order.
> 
> acmp.c incorrectly uses the big endian format when doing a multicast
> attach/detach (join). Multicast packets are used to do name resolution
> by the libibacmp library.
> 
> There are two possible results because of this issue.
> 
> If a kernel has commit 8561eae60ff9, the attach will fail with an
> EINVAL.  ibacm will log this as a failure during the multicast join.
> 
> If a kernel does not have commit 8561eae60ff9, the attach will
> complete successfully.  Packets sent to this address will be dropped
> because the packet dlid value and the multicast address information
> given by the attach will not match.
> 
> Update MLID usage to use the correct byte order.
> 
> Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  ibacm/prov/acmp/src/acmp.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ibacm/prov/acmp/src/acmp.c b/ibacm/prov/acmp/src/acmp.c
> index aa78416..78d9a29 100644
> --- a/ibacm/prov/acmp/src/acmp.c
> +++ b/ibacm/prov/acmp/src/acmp.c
> @@ -732,7 +732,7 @@ static void acmp_process_join_resp(struct
> acm_sa_mad *sa_mad)
>  			acm_log(0, "ERROR - unable to create ah\n");
>  			goto out;
>  		}
> -		ret = ibv_attach_mcast(ep->qp, &mc_rec->mgid, mc_rec-
> >mlid);
> +		ret = ibv_attach_mcast(ep->qp, &dest->mgid, dest->av.dlid);
>  		if (ret) {
>  			acm_log(0, "ERROR - unable to attach QP to multicast
> group\n");
>  			ibv_destroy_ah(dest->ah);
> @@ -1429,7 +1429,7 @@ static void acmp_ep_join(struct acmp_ep *ep)
> 
>  	if (ep->mc_dest[0].state == ACMP_READY && ep->mc_dest[0].ah) {
>  		ibv_detach_mcast(ep->qp, &ep->mc_dest[0].mgid,
> -				 be16toh(ep->mc_dest[0].av.dlid));
> +				 ep->mc_dest[0].av.dlid);
>  		ibv_destroy_ah(ep->mc_dest[0].ah);
>  		ep->mc_dest[0].ah = NULL;
>  	}
> 
> --
> 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
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš